Code cleanup for io.micronaut.http.uri #10857
Merged
+99
−75
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -44,7 +43,7 @@ class DefaultUriBuilder implements UriBuilder { | |||
| private static final String STRING_PATTERN_SCHEME = "([^:/?#]+):"; | |||
| private static final String STRING_PATTERN_USER_INFO = "([^@\\[/?#]*)"; | |||
| private static final String STRING_PATTERN_HOST_IPV4 = "[^\\[{/?#:]*"; | |||
| private static final String STRING_PATTERN_HOST_IPV6 = "\\[[\\p{XDigit}\\:\\.]*[%\\p{Alnum}]*\\]"; | |||
| private static final String STRING_PATTERN_HOST_IPV6 = "\\[[\\p{XDigit}:.]*[%\\p{Alnum}]*]"; | |||
There was a problem hiding this comment.
can you explain the changes to these patterns?
There was a problem hiding this comment.
Just redundant characters escape. This is just a simplification of the expression, removed unnecessary escapes
altro3
force-pushed
the
code-cleanup-url
branch
2 times, most recently
from
d7c9556 to
9ab6c80
Compare
|
Also added minor optimization for decoding parameters from URI: |
altro3
commented
May 31, 2024
| @@ -123,7 +123,7 @@ final class QueryStringDecoder { | |||
| QueryStringDecoder(String uri, Charset charset, boolean hasPath, int maxParams) { | |||
| this.uri = Objects.requireNonNull(uri, "uri"); | |||
| this.charset = Objects.requireNonNull(charset, "charset"); | |||
| this.maxParams = Objects.requireNonNull(maxParams, "maxParams"); | |||
| this.maxParams = maxParams; | |||
There was a problem hiding this comment.
don't need to check primitive type to non null value
| String scheme = this.scheme; | ||
| String host = this.host; | ||
| if (StringUtils.isNotEmpty(scheme)) { | ||
| if (isTemplate(scheme, values)) { | ||
| scheme = UriTemplate.of(scheme).expand(values); | ||
| } | ||
| builder.append(scheme) | ||
| .append(":"); | ||
| .append(':'); |
There was a problem hiding this comment.
Use chars better than Strings
| @@ -148,7 +147,7 @@ class DefaultUriBuilder implements UriBuilder { | |||
|
|
|||
| this.path = new StringBuilder(path); | |||
| if (query != null) { | |||
| final Map parameters = new QueryStringDecoder(uri.toString()).parameters(); | |||
| final Map parameters = QueryStringDecoder.decodeParams(uri.toString()); | |||
There was a problem hiding this comment.
Added new method to decode params without creating QueryStringDecoder instance
| @@ -120,7 +119,7 @@ class DefaultUriBuilder implements UriBuilder { | |||
| this.host = host; | |||
| } | |||
| if (port != null) { | |||
| this.port = Integer.valueOf(port); | |||
| this.port = Integer.parseInt(port); | |||
There was a problem hiding this comment.
don't need to boxing/ unboxing
| private static final String STRING_PATTERN_PATH = "([^#?]*)"; | ||
| private static final String STRING_PATTERN_QUERY = "([^#]*)"; | ||
| private static final String STRING_PATTERN_REMAINING = "(.*)"; | ||
|
|
||
| // Regex patterns that matches URIs. See RFC 3986, appendix B | ||
| private static final Pattern PATTERN_SCHEME = Pattern.compile("^" + STRING_PATTERN_SCHEME + "//.*"); | ||
| private static final Pattern PATTERN_FULL_PATH = Pattern.compile("^([^#\\?]*)(\\?([^#]*))?(\\#(.*))?$"); | ||
| private static final Pattern PATTERN_FULL_PATH = Pattern.compile("^([^#?]*)(\\?([^#]*))?(#(.*))?$"); |
There was a problem hiding this comment.
Just remove redundant escpape characters
|
Added some clarification to my changes |
dstepanov
approved these changes
Jul 1, 2024
|
@graemerocher ping |
graemerocher
approved these changes
Jul 19, 2024
|
Thanks for the contribution! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.