- Notifications
You must be signed in to change notification settings - Fork 44
feat(common): replace jersey dependencies with OkHttp (Breaking Change) #133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
hugegraph-common/src/test/java/org/apache/hugegraph/unit/rest/RestClientTest.java Show resolved Hide resolved
hugegraph-common/src/test/java/org/apache/hugegraph/unit/rest/RestClientTest.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestClient.java Show resolved Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@ ## master #133 +/- ## ========================================= Coverage 93.15% 93.15% Complexity 65 65 ========================================= Files 9 9 Lines 263 263 Branches 22 22 ========================================= Hits 245 245 Misses 8 8 Partials 10 10 ☔ View full report in Codecov by Sentry. |
hugegraph-common/src/test/java/org/apache/hugegraph/unit/util/ReflectionUtilTest.java Show resolved Hide resolved
hugegraph-common/src/test/java/org/apache/hugegraph/unit/rest/RestClientTest.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestClient.java Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Show resolved Hide resolved
hugegraph-common/src/test/java/org/apache/hugegraph/unit/util/ReflectionUtilTest.java Show resolved Hide resolved
hugegraph-common/src/test/java/org/apache/hugegraph/unit/rest/RestClientTest.java Outdated Show resolved Hide resolved
| Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label |
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestHeaders.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestHeaders.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestHeaders.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestHeaders.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/OkhttpBasicAuthInterceptor.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestResult.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestResult.java Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/RequestHeaders.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/RequestHeaders.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestResult.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestResult.java Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/OkhttpBasicAuthInterceptor.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/OkHttpConfig.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/OkhttpTokenInterceptor.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/OkhttpTokenInterceptor.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/OkHttpConfig.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestClientConfig.java Outdated Show resolved Hide resolved
hugegraph-common/src/test/java/org/apache/hugegraph/unit/rest/RestClientTest.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/HttpHeadersConstant.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestClientConfig.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestClientConfig.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestClientConfig.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/HttpHeadersConstant.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/HttpHeadersConstant.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java Outdated Show resolved Hide resolved
| private Integer idleTime = 5; | ||
| private TimeUnit idleTimeUnit = TimeUnit.MINUTES; | ||
| private Integer idleTime = 30; | ||
| private TimeUnit idleTimeUnit = TimeUnit.SECONDS; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems don't need to add a var for the unit, prefer to just hardcode it
| return this.headersBuilder.build(); | ||
| } | ||
| | ||
| public static class HttpHeadersConstant { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefer to merge these vars into class RestHeaders
| ConnectionPool connectionPool = new ConnectionPool(config.getMaxIdleConns(), | ||
| config.getIdleTime(), | ||
| TimeUnit.MINUTES); | ||
| config.getIdleTimeUnit()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can just hardcode TimeUnit.SECONDS here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks a lot for the job, I'll publish it to stage maven repo soon
之前在tool-chains提过一个pr,直接修改client的代码将jersey使用okhttp替换
现在尝试直接修改common的代码,不过common的代码中单元测试和jersey是强相关的,在这一块遇到一些问题,然后尝试自己修改单测代码进行跑通。不过由于对单测的代码不太熟悉,对于代码的意图不太了解,麻烦社区同学帮忙看看
Breaking Changes
org.apache.http.HttpStatusis removed now, replace it tojava.net.HttpURLConnection(JDK bundled)jakarta.ws.rs.core.MultivaluedHashMapis removed now, replace it toorg.apache.hugegraph.rest.RestHeadersjakarta.ws.rs.core.Responseis removed now, replace it tookhttp3.Response.org.glassfish.jersey.uri.UriComponentis removed, just not use it. There's no need to encode the query params manual, just remove the codeUriComponent.encode(raw, Type.QUERY_PARAM_SPACE_ENCODED).Other repo/users could refer the info to update/adapt the new version, and comment in this PR if anything missed