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

[UNDERTOW-1821] Path template matched parameters should keep order #983

Merged

Conversation

seregamorph
Copy link
Contributor

@seregamorph seregamorph commented Oct 26, 2020

Jira: https://issues.redhat.com/browse/UNDERTOW-1821

Replace HashMap to LinkedHashMap in PathTemplateMatcher. Motivation: ordered matched paths look more natural e.g. in debugging:
Screen Shot 2020-10-26 at 10 18 51

Also in my case I needed to write a simple custom matcher: not Map (expected) equals Map (actual), but a List of matched values (cannot use Set, because values can be repeated).

Reference implementation: spring-core org.springframework.util.AntPathMatcher keeps original order:

AntPathMatcher antPathMatcher = new AntPathMatcher();
Map<String, String> pathVariables = antPathMatcher.extractUriTemplateVariables(
        "/{context}/{version}/{controller}/{id}",
        "/api/v3/users/1"
);
// prints "{context=api, version=v3, controller=users, id=1}"

Cons: in theory, updated impl will take more nanoseconds in runtime and more nanobytes in memory, but I believe if there was an intention to micro-optimise it, these should be a custom Map implementation (like Netty HttpHeaders)

@seregamorph
Copy link
Contributor Author

@stuartwdouglas what do you think?

@fl4via fl4via changed the title Path template matched parameters should keep order [UNDERTOW-1821] Path template matched parameters should keep order Dec 3, 2020
@fl4via fl4via added enhancement Enhances existing behaviour or code next release This PR will be merged before next release or has already been merged (for payload double check) labels Dec 3, 2020
@fl4via fl4via force-pushed the feature/ordered-path-variables branch from 452ff65 to 843ecc3 Compare December 3, 2020 07:18
@fl4via fl4via force-pushed the feature/ordered-path-variables branch from 843ecc3 to d6ee3b2 Compare December 3, 2020 07:19
@fl4via fl4via added the waiting CI check Ready to be merged but waiting for CI check label Dec 3, 2020
@fl4via
Copy link
Member

fl4via commented Dec 3, 2020

@seregamorph we are approving this change. The advantage of keeping parameters in order outweights the added overhead, specially because hte matcher's map is small and is not used every where in the code.

@seregamorph
Copy link
Contributor Author

Thanks @fl4via

@fl4via fl4via removed the waiting CI check Ready to be merged but waiting for CI check label Dec 3, 2020
@fl4via fl4via merged commit baf02f5 into undertow-io:master Dec 3, 2020
@fl4via fl4via removed the next release This PR will be merged before next release or has already been merged (for payload double check) label Dec 7, 2020
@seregamorph seregamorph deleted the feature/ordered-path-variables branch May 28, 2023 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhances existing behaviour or code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants