feat(common): replace jersey dependencies with OkHttp (Breaking Change) by zhenyuT · Pull Request #133 · apache/hugegraph-commons

zhenyuT · 2023-08-27T17:38:38Z

之前在tool-chains提过一个pr，直接修改client的代码将jersey使用okhttp替换

feat(client): replace jersey with okhttp in hugegraph-client (Draft PR) hugegraph-toolchain#508

现在尝试直接修改common的代码，不过common的代码中单元测试和jersey是强相关的，在这一块遇到一些问题，然后尝试自己修改单测代码进行跑通。不过由于对单测的代码不太熟悉，对于代码的意图不太了解，麻烦社区同学帮忙看看

Breaking Changes

org.apache.http.HttpStatus is removed now, replace it to java.net.HttpURLConnection (JDK bundled)

jakarta.ws.rs.core.MultivaluedHashMap is removed now, replace it to org.apache.hugegraph.rest.RestHeaders

   MultivaluedHashMap<String, Object> headers = new MultivaluedHashMap<>();
   headers.putSingle("Content-Encoding", BATCH_ENCODING);

For example,replace the upon code to:

 RestHeaders headers = new RestHeaders().add("Content-Encoding", BATCH_ENCODING);

jakarta.ws.rs.core.Response is removed now, replace it to okhttp3.Response.
org.glassfish.jersey.uri.UriComponent is removed, just not use it. There's no need to encode the query params manual, just remove the code UriComponent.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

workflow_dispatch

hugegraph-common/src/test/java/org/apache/hugegraph/unit/rest/RestClientTest.java

hugegraph-common/src/main/java/org/apache/hugegraph/util/JsonUtil.java

hugegraph-common/src/test/java/org/apache/hugegraph/unit/rest/RestClientTest.java

hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestClient.java

codecov · 2023-08-28T07:14:51Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b49be05) 93.15% compared to head (81aa7ca) 93.15%.

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.
📢 Have feedback on the report? Share it here.

hugegraph-common/src/test/java/org/apache/hugegraph/unit/util/ReflectionUtilTest.java

hugegraph-common/src/test/java/org/apache/hugegraph/unit/rest/RestClientTest.java

hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java

hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestClient.java

hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java

hugegraph-common/src/test/java/org/apache/hugegraph/unit/util/ReflectionUtilTest.java

hugegraph-common/src/test/java/org/apache/hugegraph/unit/rest/RestClientTest.java

github-actions · 2023-10-03T21:01:25Z

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

…se-removejersey

hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java

hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestHeaders.java

hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java

hugegraph-common/src/main/java/org/apache/hugegraph/rest/OkhttpBasicAuthInterceptor.java

hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestResult.java

hugegraph-common/src/main/java/org/apache/hugegraph/rest/RequestHeaders.java

hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestResult.java

hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java

hugegraph-common/src/main/java/org/apache/hugegraph/rest/OkhttpBasicAuthInterceptor.java

hugegraph-common/src/main/java/org/apache/hugegraph/rest/OkHttpConfig.java

hugegraph-common/src/main/java/org/apache/hugegraph/rest/OkhttpTokenInterceptor.java

hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java

hugegraph-common/src/main/java/org/apache/hugegraph/rest/OkHttpConfig.java

hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestClientConfig.java

hugegraph-common/src/test/java/org/apache/hugegraph/unit/rest/RestClientTest.java

hugegraph-common/src/main/java/org/apache/hugegraph/rest/HttpHeadersConstant.java

hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java

hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestClientConfig.java

hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java

hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestClientConfig.java

hugegraph-common/src/main/java/org/apache/hugegraph/rest/HttpHeadersConstant.java

hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java

javeme · 2023-11-21T10:37:03Z

hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestClientConfig.java

-    private Integer idleTime = 5;
-    private TimeUnit idleTimeUnit = TimeUnit.MINUTES;
+    private Integer idleTime = 30;
+    private TimeUnit idleTimeUnit = TimeUnit.SECONDS;


seems don't need to add a var for the unit, prefer to just hardcode it

javeme · 2023-11-21T10:37:54Z

hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestHeaders.java

        return this.headersBuilder.build();
    }
+
+    public static class HttpHeadersConstant {


prefer to merge these vars into class RestHeaders

javeme · 2023-11-21T10:38:27Z

hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java

            ConnectionPool connectionPool = new ConnectionPool(config.getMaxIdleConns(),
                                                               config.getIdleTime(),
-                                                               TimeUnit.MINUTES);
+                                                               config.getIdleTimeUnit());


we can just hardcode TimeUnit.SECONDS here

javeme

LGTM

imbajin

thanks a lot for the job, I'll publish it to stage maven repo soon

zhenyuT added 8 commits August 28, 2023 00:16

remove jersey

749c177

add workflow_dispatch

79b04cc

add workflow_dispatch

ebb0331

Empty-Commit

c79ac13

Empty-Commit

befd4da

Update ci.yml

ddadd35

workflow_dispatch

fix error

0d44db7

fix error

b77745b

zhenyuT commented Aug 27, 2023

View reviewed changes

hugegraph-common/src/test/java/org/apache/hugegraph/unit/rest/RestClientTest.java Show resolved Hide resolved

zhenyuT commented Aug 27, 2023

View reviewed changes

hugegraph-common/src/main/java/org/apache/hugegraph/util/JsonUtil.java Show resolved Hide resolved

zhenyuT commented Aug 27, 2023

View reviewed changes

hugegraph-common/src/test/java/org/apache/hugegraph/unit/rest/RestClientTest.java Outdated Show resolved Hide resolved

zhenyuT commented Aug 27, 2023

View reviewed changes

hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestClient.java Show resolved Hide resolved

imbajin requested review from javeme, z7658329 and zyxxoo August 28, 2023 07:12

imbajin mentioned this pull request Aug 7, 2023

[Feature] Replace jersey in hugegraph-client/common to avoid SpringBoot/Jarkata conflicts apache/hugegraph-toolchain#505

Closed

2 tasks

zhenyuT commented Aug 30, 2023

View reviewed changes

hugegraph-common/src/test/java/org/apache/hugegraph/unit/util/ReflectionUtilTest.java Show resolved Hide resolved

zhenyuT commented Aug 30, 2023

View reviewed changes

hugegraph-common/src/test/java/org/apache/hugegraph/unit/rest/RestClientTest.java Outdated Show resolved Hide resolved

javeme reviewed Aug 30, 2023

View reviewed changes

chore: reformat code

5198676

javeme mentioned this pull request Sep 10, 2023

feat(client): replace jersey with okhttp in hugegraph-client (Draft PR) apache/hugegraph-toolchain#508

Closed

github-actions bot added the inactive label Oct 3, 2023

javeme removed the inactive label Oct 4, 2023

imbajin and others added 5 commits October 7, 2023 16:07

Merge branch 'master' into release-removejersey

ae8605b

refact: replace params okhttp3.Headers to internal RestHeaders

7945718

Merge remote-tracking branch 'origin/release-removejersey' into relea…

193b5d3

…se-removejersey

fix: licence dependency

1ea7905

Merge branch 'apache:master' into master

74c681c

javeme reviewed Oct 28, 2023

View reviewed changes

zhenyuT added 2 commits October 29, 2023 22:00

fix: code issue

945005a

fix: code issue

24a7dbf

imbajin requested a review from javeme October 31, 2023 08:57

imbajin added this to the 1.2.0 milestone Oct 31, 2023

javeme reviewed Nov 4, 2023

View reviewed changes

fix: code issue

2cce7f4

javeme reviewed Nov 5, 2023

View reviewed changes

hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestResult.java Show resolved Hide resolved

zhenyuT added 6 commits November 5, 2023 19:39

fix: code issue

cfb8daa

fix: code issue

e662b64

fix: code issue

e8fb6f3

version 1.2.0

ff91e8a

fix: version error

a6b22fd

fix: version error

d717f6d

javeme reviewed Nov 11, 2023

View reviewed changes

fix: code issue

bbc5ec1

javeme reviewed Nov 12, 2023

View reviewed changes

imbajin and others added 2 commits November 15, 2023 15:50

tiny improve

e851694

fix: code issue

5aecb31

javeme reviewed Nov 18, 2023

View reviewed changes

fix: code issue

1a1435e

javeme reviewed Nov 21, 2023

View reviewed changes

fix: code issue

81aa7ca

javeme approved these changes Nov 21, 2023

View reviewed changes

imbajin approved these changes Nov 22, 2023

View reviewed changes

imbajin merged commit 5ad55fb into apache:master Nov 22, 2023

This was referenced Nov 22, 2023

refact(core): adaptor for common 1.2 & fix a string of possible CI problem apache/hugegraph-computer#286

Merged

chore(ci): add stage profile settings apache/hugegraph-toolchain#536

Merged

Zony7 mentioned this pull request Nov 23, 2023

chore(ci): add stage profile settings apache/hugegraph#2361

Merged

11 tasks

zhenyuT mentioned this pull request Nov 23, 2023

feat: update common version and remove jersey code apache/hugegraph-toolchain#538

Merged

11 tasks

Comments

Conversation

zhenyuT commented Aug 27, 2023 • edited by imbajin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Breaking Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Aug 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Oct 3, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

javeme Nov 21, 2023

Choose a reason for hiding this comment

Uh oh!

javeme Nov 21, 2023

Choose a reason for hiding this comment

zhenyuT commented Aug 27, 2023 •

edited by imbajin

Loading

codecov bot commented Aug 28, 2023 •

edited

Loading