Skip to content
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

Replace String.format() with newly created class OpenApiPathBuilder #3982

Merged
merged 2 commits into from
Sep 14, 2021

Conversation

WillardHu
Copy link
Contributor

Signed-off-by: WillardHu wei.hu@daocloud.io

What's the purpose of this PR

OpenApiPathBuilder has better readability, maintainability, and performance. Before modifying String.format (..) will be used twice to complete url concatenation. String.format(..) uses regular expressions to assemble strings, performance is slow.

Which issue(s) this PR fixes:

Fixes #

Brief changelog

Replace String.format() with newly created class OpenApiPathBuilder

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

  • Read the Contributing Guide before making this pull request.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit tests to verify the code.
  • Run mvn clean test to make sure this pull request doesn't break anything.
  • Update the CHANGES log.

@WillardHu WillardHu force-pushed the fix/openapi-path-builder branch from b2ec7f1 to d3bafae Compare September 14, 2021 03:07
@codecov-commenter
Copy link

codecov-commenter commented Sep 14, 2021

Codecov Report

Merging #3982 (60e2fe2) into master (0b5d227) will increase coverage by 0.27%.
The diff coverage is 92.45%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3982      +/-   ##
============================================
+ Coverage     50.76%   51.04%   +0.27%     
- Complexity     2468     2479      +11     
============================================
  Files           475      476       +1     
  Lines         14600    14681      +81     
  Branches       1521     1521              
============================================
+ Hits           7412     7494      +82     
+ Misses         6661     6657       -4     
- Partials        527      530       +3     
Impacted Files Coverage Δ
...ctrip/framework/apollo/core/utils/StringUtils.java 82.35% <ø> (+17.53%) ⬆️
...enapi/client/exception/ApolloOpenApiException.java 100.00% <ø> (ø)
...ollo/openapi/client/service/AppOpenApiService.java 48.27% <36.36%> (+0.27%) ⬆️
.../apollo/openapi/client/url/OpenApiPathBuilder.java 90.74% <90.74%> (ø)
...openapi/client/service/AbstractOpenApiService.java 92.85% <100.00%> (-0.90%) ⬇️
.../openapi/client/service/ClusterOpenApiService.java 93.10% <100.00%> (+1.43%) ⬆️
...llo/openapi/client/service/ItemOpenApiService.java 74.28% <100.00%> (+5.68%) ⬆️
...penapi/client/service/NamespaceOpenApiService.java 83.07% <100.00%> (+3.83%) ⬆️
.../openapi/client/service/ReleaseOpenApiService.java 82.00% <100.00%> (+5.07%) ⬆️
...rk/apollo/spring/property/SpringValueRegistry.java 83.78% <0.00%> (-5.41%) ⬇️
... and 3 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 d8b685e...60e2fe2. Read the comment docs.

@WillardHu WillardHu force-pushed the fix/openapi-path-builder branch from d3bafae to 6e2d3bc Compare September 14, 2021 03:12
@WillardHu WillardHu force-pushed the fix/openapi-path-builder branch from 6e2d3bc to 85acf67 Compare September 14, 2021 04:55
@WillardHu WillardHu force-pushed the fix/openapi-path-builder branch from 85acf67 to 744f242 Compare September 14, 2021 05:36
kezhenxu94
kezhenxu94 previously approved these changes Sep 14, 2021
Copy link
Member

@kezhenxu94 kezhenxu94 left a comment

Choose a reason for hiding this comment

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

LGTM, just one nit

@WillardHu WillardHu force-pushed the fix/openapi-path-builder branch from 110aaef to 260c228 Compare September 14, 2021 07:13
Copy link
Member

@nobodyiam nobodyiam left a comment

Choose a reason for hiding this comment

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

LGTM

@nobodyiam nobodyiam merged commit d593178 into apolloconfig:master Sep 14, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Sep 14, 2021
@WillardHu WillardHu deleted the fix/openapi-path-builder branch September 14, 2021 13:29
@nobodyiam nobodyiam added this to the 1.10.0 milestone Oct 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants