You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shenyu.apache.org by GitBox <gi...@apache.org> on 2022/12/25 09:10:33 UTC

[GitHub] [shenyu] RayayChung opened a new pull request, #4283: [ISSUE #4276] remove redundant cookie setting

RayayChung opened a new pull request, #4283:
URL: https://github.com/apache/shenyu/pull/4283

   fix: #4276 
   Make sure that:
   
   - [ ] You have read the [contribution guidelines](https://shenyu.apache.org/community/contributor-guide).
   - [ ] You submit test cases (unit or integration tests) that back your changes.
   - [ ] Your local test passed `./mvnw clean install -Dmaven.javadoc.skip=true`.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shenyu] RayayChung commented on a diff in pull request #4283: [ISSUE #4276] remove redundant cookie setting

Posted by GitBox <gi...@apache.org>.
RayayChung commented on code in PR #4283:
URL: https://github.com/apache/shenyu/pull/4283#discussion_r1056960923


##########
shenyu-plugin/shenyu-plugin-httpclient/src/main/java/org/apache/shenyu/plugin/httpclient/WebClientPlugin.java:
##########
@@ -63,8 +63,7 @@ protected Mono<ClientResponse> doRequest(final ServerWebExchange exchange, final
                         .flatMap(bytes -> Mono.fromCallable(() -> Optional.ofNullable(bytes))).defaultIfEmpty(Optional.empty())
                         .flatMap(option -> {
                             final ClientResponse.Builder builder = ClientResponse.create(response.statusCode())
-                                    .headers(headers -> headers.addAll(response.headers().asHttpHeaders()))
-                                    .cookies(cookies -> cookies.addAll(response.cookies()));

Review Comment:
   Setting cookies is just part of setting headers. So the previous version codes may lead setting cookies twice.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shenyu] RayayChung commented on a diff in pull request #4283: [ISSUE #4276] remove redundant cookie setting

Posted by GitBox <gi...@apache.org>.
RayayChung commented on code in PR #4283:
URL: https://github.com/apache/shenyu/pull/4283#discussion_r1057095482


##########
shenyu-plugin/shenyu-plugin-httpclient/src/main/java/org/apache/shenyu/plugin/httpclient/WebClientPlugin.java:
##########
@@ -63,8 +63,7 @@ protected Mono<ClientResponse> doRequest(final ServerWebExchange exchange, final
                         .flatMap(bytes -> Mono.fromCallable(() -> Optional.ofNullable(bytes))).defaultIfEmpty(Optional.empty())
                         .flatMap(option -> {
                             final ClientResponse.Builder builder = ClientResponse.create(response.statusCode())
-                                    .headers(headers -> headers.addAll(response.headers().asHttpHeaders()))
-                                    .cookies(cookies -> cookies.addAll(response.cookies()));

Review Comment:
   the data of `response.cookies()` is the subset of `response.headers().asHttpHeaders()`, therefore setting headers is also setting cookies in the sam time. I have never met the case that cookies is not present in headers. Is this case exists?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shenyu] yu199195 merged pull request #4283: [ISSUE #4276] remove redundant cookie setting

Posted by GitBox <gi...@apache.org>.
yu199195 merged PR #4283:
URL: https://github.com/apache/shenyu/pull/4283


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shenyu] codecov-commenter commented on pull request #4283: [ISSUE #4276] remove redundant cookie setting

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #4283:
URL: https://github.com/apache/shenyu/pull/4283#issuecomment-1364652118

   # [Codecov](https://codecov.io/gh/apache/shenyu/pull/4283?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#4283](https://codecov.io/gh/apache/shenyu/pull/4283?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c951ede) into [master](https://codecov.io/gh/apache/shenyu/commit/f1aaa20c3cb8810bd877dba4cb340aa1959b3fc9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f1aaa20) will **decrease** coverage by `0.07%`.
   > The diff coverage is `100.00%`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #4283      +/-   ##
   ============================================
   - Coverage     68.97%   68.90%   -0.08%     
   + Complexity     7335     7333       -2     
   ============================================
     Files           994      994              
     Lines         28013    28003      -10     
     Branches       2483     2480       -3     
   ============================================
   - Hits          19323    19296      -27     
   - Misses         7213     7225      +12     
   - Partials       1477     1482       +5     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shenyu/pull/4283?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ache/shenyu/plugin/httpclient/WebClientPlugin.java](https://codecov.io/gh/apache/shenyu/pull/4283/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hlbnl1LXBsdWdpbi9zaGVueXUtcGx1Z2luLWh0dHBjbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoZW55dS9wbHVnaW4vaHR0cGNsaWVudC9XZWJDbGllbnRQbHVnaW4uamF2YQ==) | `92.00% <100.00%> (+34.30%)` | :arrow_up: |
   | [...ache/shenyu/plugin/grpc/cache/GrpcClientCache.java](https://codecov.io/gh/apache/shenyu/pull/4283/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hlbnl1LXBsdWdpbi9zaGVueXUtcGx1Z2luLWdycGMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoZW55dS9wbHVnaW4vZ3JwYy9jYWNoZS9HcnBjQ2xpZW50Q2FjaGUuamF2YQ==) | `68.75% <0.00%> (-31.25%)` | :arrow_down: |
   | [...yu/sync/data/http/refresh/AbstractDataRefresh.java](https://codecov.io/gh/apache/shenyu/pull/4283/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hlbnl1LXN5bmMtZGF0YS1jZW50ZXIvc2hlbnl1LXN5bmMtZGF0YS1odHRwL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGVueXUvc3luYy9kYXRhL2h0dHAvcmVmcmVzaC9BYnN0cmFjdERhdGFSZWZyZXNoLmphdmE=) | `68.96% <0.00%> (-13.80%)` | :arrow_down: |
   | [...henyu/plugin/grpc/resolver/ShenyuNameResolver.java](https://codecov.io/gh/apache/shenyu/pull/4283/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hlbnl1LXBsdWdpbi9zaGVueXUtcGx1Z2luLWdycGMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoZW55dS9wbHVnaW4vZ3JwYy9yZXNvbHZlci9TaGVueXVOYW1lUmVzb2x2ZXIuamF2YQ==) | `44.68% <0.00%> (-8.52%)` | :arrow_down: |
   | [...a/org/apache/shenyu/common/utils/VersionUtils.java](https://codecov.io/gh/apache/shenyu/pull/4283/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hlbnl1LWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hlbnl1L2NvbW1vbi91dGlscy9WZXJzaW9uVXRpbHMuamF2YQ==) | `21.27% <0.00%> (-6.39%)` | :arrow_down: |
   | [...che/shenyu/sync/data/http/HttpSyncDataService.java](https://codecov.io/gh/apache/shenyu/pull/4283/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hlbnl1LXN5bmMtZGF0YS1jZW50ZXIvc2hlbnl1LXN5bmMtZGF0YS1odHRwL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGVueXUvc3luYy9kYXRhL2h0dHAvSHR0cFN5bmNEYXRhU2VydmljZS5qYXZh) | `81.63% <0.00%> (-4.09%)` | :arrow_down: |
   | [...he/shenyu/plugin/grpc/client/ShenyuGrpcClient.java](https://codecov.io/gh/apache/shenyu/pull/4283/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hlbnl1LXBsdWdpbi9zaGVueXUtcGx1Z2luLWdycGMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoZW55dS9wbHVnaW4vZ3JwYy9jbGllbnQvU2hlbnl1R3JwY0NsaWVudC5qYXZh) | `8.00% <0.00%> (-4.00%)` | :arrow_down: |
   | [...yu/common/dto/convert/rule/Resilience4JHandle.java](https://codecov.io/gh/apache/shenyu/pull/4283/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hlbnl1LWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hlbnl1L2NvbW1vbi9kdG8vY29udmVydC9ydWxlL1Jlc2lsaWVuY2U0SkhhbmRsZS5qYXZh) | `71.60% <0.00%> (-3.71%)` | :arrow_down: |
   | [...rg/apache/shenyu/plugin/hystrix/HystrixPlugin.java](https://codecov.io/gh/apache/shenyu/pull/4283/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hlbnl1LXBsdWdpbi9zaGVueXUtcGx1Z2luLWh5c3RyaXgvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoZW55dS9wbHVnaW4vaHlzdHJpeC9IeXN0cml4UGx1Z2luLmphdmE=) | `75.86% <0.00%> (-0.81%)` | :arrow_down: |
   | [...yu/admin/service/impl/PluginHandleServiceImpl.java](https://codecov.io/gh/apache/shenyu/pull/4283/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hlbnl1LWFkbWluL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGVueXUvYWRtaW4vc2VydmljZS9pbXBsL1BsdWdpbkhhbmRsZVNlcnZpY2VJbXBsLmphdmE=) | `75.47% <0.00%> (-0.46%)` | :arrow_down: |
   | ... and [8 more](https://codecov.io/gh/apache/shenyu/pull/4283/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shenyu] yu199195 commented on a diff in pull request #4283: [ISSUE #4276] remove redundant cookie setting

Posted by GitBox <gi...@apache.org>.
yu199195 commented on code in PR #4283:
URL: https://github.com/apache/shenyu/pull/4283#discussion_r1056959599


##########
shenyu-plugin/shenyu-plugin-httpclient/src/main/java/org/apache/shenyu/plugin/httpclient/WebClientPlugin.java:
##########
@@ -63,8 +63,7 @@ protected Mono<ClientResponse> doRequest(final ServerWebExchange exchange, final
                         .flatMap(bytes -> Mono.fromCallable(() -> Optional.ofNullable(bytes))).defaultIfEmpty(Optional.empty())
                         .flatMap(option -> {
                             final ClientResponse.Builder builder = ClientResponse.create(response.statusCode())
-                                    .headers(headers -> headers.addAll(response.headers().asHttpHeaders()))
-                                    .cookies(cookies -> cookies.addAll(response.cookies()));

Review Comment:
   why remove cookies setting？



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [shenyu] yu199195 commented on a diff in pull request #4283: [ISSUE #4276] remove redundant cookie setting

Posted by GitBox <gi...@apache.org>.
yu199195 commented on code in PR #4283:
URL: https://github.com/apache/shenyu/pull/4283#discussion_r1057066768


##########
shenyu-plugin/shenyu-plugin-httpclient/src/main/java/org/apache/shenyu/plugin/httpclient/WebClientPlugin.java:
##########
@@ -63,8 +63,7 @@ protected Mono<ClientResponse> doRequest(final ServerWebExchange exchange, final
                         .flatMap(bytes -> Mono.fromCallable(() -> Optional.ofNullable(bytes))).defaultIfEmpty(Optional.empty())
                         .flatMap(option -> {
                             final ClientResponse.Builder builder = ClientResponse.create(response.statusCode())
-                                    .headers(headers -> headers.addAll(response.headers().asHttpHeaders()))
-                                    .cookies(cookies -> cookies.addAll(response.cookies()));

Review Comment:
   Is there a time when cookies is not passed from the header?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org