Skip to content

fix repeat adding parameters to the URL#7498

Merged
AlbumenJ merged 3 commits intoapache:masterfrom
BurningCN:fix_duplicate_parameters
Apr 14, 2021
Merged

fix repeat adding parameters to the URL#7498
AlbumenJ merged 3 commits intoapache:masterfrom
BurningCN:fix_duplicate_parameters

Conversation

@BurningCN
Copy link
Copy Markdown
Member

@BurningCN BurningCN commented Apr 1, 2021 •
edited
Loading

What is the purpose of the change

ConfigParser#toParameterString and ConfigParser#toParameterString#appItemToUrls ，when after the toParameterString method call, the category parameter is added again

Brief changelog

XXXXX

Verifying this change

XXXXX

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Make sure there is a GITHUB_issue field for the change (usually before you start working on it). Trivial changes like typos do not require a GITHUB issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
  • Format the pull request title like [Dubbo-XXX] Fix UnknownException when host config not exist #XXX. Each commit in the pull request should have a meaningful subject line and body.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add sample in dubbo samples project.
  • Run mvn clean install -DskipTests=false & mvn clean test-compile failsafe:integration-test to make sure unit-test and integration-test pass.
  • If this contribution is large, please follow the Software Donation Guide.
    image

@BurningCN BurningCN mentioned this pull request Apr 2, 2021
Closed
zonghaishang
zonghaishang approved these changes Apr 2, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@zonghaishang zonghaishang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zonghaishang zonghaishang added the type/bug Bugs to being fixed label Apr 2, 2021
@chickenlj chickenlj modified the milestones: 2.7.10, 2.7.11 Apr 6, 2021
chickenlj
chickenlj requested changes Apr 6, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@chickenlj chickenlj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the review comment i left above

@codecov-io
Copy link
Copy Markdown

codecov-io commented Apr 6, 2021 •
edited
Loading

Codecov Report

Merging #7498 (43ab0a8) into master (eeac3a0) will decrease coverage by 0.52%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #7498      +/-   ##
============================================
- Coverage     59.38%   58.86%   -0.53%     
  Complexity      502      502              
============================================
  Files          1079     1079              
  Lines         43329    43384      +55     
  Branches       6318     6329      +11     
============================================
- Hits          25733    25540     -193     
- Misses        14759    14998     +239     
- Partials       2837     2846       +9
Impacted Files Coverage Δ Complexity Δ
.../rpc/cluster/configurator/parser/ConfigParser.java 85.84% <ø> (+0.38%) 0.00 <0.00> (ø)
...a/org/apache/dubbo/rpc/filter/AccessLogFilter.java 26.86% <0.00%> (-52.24%) 0.00% <0.00%> (ø%)
...gcenter/wrapper/CompositeDynamicConfiguration.java 0.00% <0.00%> (-50.00%) 0.00% <0.00%> (ø%)
...va/org/apache/dubbo/rpc/support/AccessLogData.java 53.16% <0.00%> (-37.98%) 0.00% <0.00%> (ø%)
...ng/transport/dispatcher/all/AllChannelHandler.java 62.06% <0.00%> (-34.49%) 0.00% <0.00%> (ø%)
...gistry/client/migration/MigrationRuleListener.java 55.88% <0.00%> (-17.65%) 0.00% <0.00%> (ø%)
...dubbo/common/status/support/LoadStatusChecker.java 46.15% <0.00%> (-15.39%) 0.00% <0.00%> (ø%)
...rg/apache/dubbo/remoting/utils/PayloadDropper.java 76.92% <0.00%> (-15.39%) 0.00% <0.00%> (ø%)
...o/rpc/cluster/support/migration/MigrationRule.java 46.42% <0.00%> (-12.20%) 0.00% <0.00%> (ø%)
...t/migration/DefaultMigrationAddressComparator.java 15.38% <0.00%> (-11.54%) 0.00% <0.00%> (ø%)
... and 53 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eeac3a0...43ab0a8. Read the comment docs.

xiaoheng1
xiaoheng1 approved these changes Apr 13, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@xiaoheng1 xiaoheng1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

AlbumenJ
AlbumenJ approved these changes Apr 14, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@AlbumenJ AlbumenJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@AlbumenJ AlbumenJ merged commit 74a91d6 into apache:master Apr 14, 2021
@BurningCN BurningCN deleted the fix_duplicate_parameters branch April 19, 2021 12:07
AlbumenJ added a commit to AlbumenJ/dubbo that referenced this pull request May 28, 2021
@AlbumenJ
Pick from apache#7498 74a91d6
01f6b31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chickenlj chickenlj chickenlj requested changes

@AlbumenJ AlbumenJ AlbumenJ approved these changes

@xiaoheng1 xiaoheng1 xiaoheng1 approved these changes

@zonghaishang zonghaishang zonghaishang approved these changes

Assignees

No one assigned

Labels

type/bug Bugs to being fixed

Projects

None yet

Milestone

2.7.11

Development

Successfully merging this pull request may close these issues.

6 participants

@BurningCN @codecov-io @AlbumenJ @xiaoheng1 @zonghaishang @chickenlj