diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractUrlHandlerMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractUrlHandlerMapping.java index 5dedc9d240..e48c5aed0d 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractUrlHandlerMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractUrlHandlerMapping.java @@ -18,7 +18,6 @@ package org.springframework.web.reactive.handler; import java.util.Collections; import java.util.Map; -import java.util.Optional; import reactor.core.publisher.Mono; @@ -113,12 +112,11 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { @Nullable protected Object lookupHandler(String lookupPath, ServerWebExchange exchange) throws Exception { if (this.patternRegistry != null) { - Optional> matches = this.patternRegistry.findFirstMatch(lookupPath); - if (matches.isPresent()) { + PathMatchResult bestMatch = this.patternRegistry.findFirstMatch(lookupPath); + if (bestMatch != null) { if (logger.isDebugEnabled()) { - logger.debug("Matching patterns for request [" + lookupPath + "] are " + matches); + logger.debug("Matching patterns for request [" + lookupPath + "] are " + bestMatch); } - PathMatchResult bestMatch = matches.get(); String pathWithinMapping = bestMatch.getPattern().extractPathWithinPattern(lookupPath); Object handler = bestMatch.getHandler(); return handleMatch(handler, bestMatch.getPattern(), pathWithinMapping, exchange); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/PathMatchResult.java b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/PathMatchResult.java index 7fc98e64bd..5479cfb88b 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/PathMatchResult.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/PathMatchResult.java @@ -16,20 +16,20 @@ package org.springframework.web.reactive.handler; +import org.jetbrains.annotations.NotNull; + import org.springframework.util.Assert; import org.springframework.web.util.pattern.PathPattern; /** - * Result of matching an request lookup path against {@link PathPattern} instances. - * - *

Each result optionally associates the matching {@link PathPattern} - * with a request handler of type {@code T}. + * Result of path match performed through a {@link PathPatternRegistry}. + * Each result contains the matched pattern and handler of type {@code T}. * * @author Brian Clozel * @since 5.0 * @see PathPatternRegistry */ -public class PathMatchResult { +public class PathMatchResult implements Comparable> { private final PathPattern pattern; @@ -58,4 +58,20 @@ public class PathMatchResult { return this.handler; } + + @Override + public int compareTo(@NotNull PathMatchResult other) { + int index = this.pattern.compareTo(other.pattern); + return (index != 0 ? index : this.getPatternString().compareTo(other.getPatternString())); + } + + private String getPatternString() { + return this.pattern.getPatternString(); + } + + @Override + public String toString() { + return "PathMatchResult{pattern=" + this.pattern + ", handler=" + this.handler + "]"; + } + } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/PathPatternRegistry.java b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/PathPatternRegistry.java index 7cbfd5f197..30753394b1 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/PathPatternRegistry.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/PathPatternRegistry.java @@ -20,7 +20,6 @@ import java.util.Collections; import java.util.Comparator; import java.util.HashMap; import java.util.Map; -import java.util.Optional; import java.util.SortedSet; import java.util.TreeSet; import java.util.stream.Collectors; @@ -81,8 +80,30 @@ public class PathPatternRegistry { return Collections.unmodifiableMap(this.patternsMap); } + /** - * Return a {@code SortedSet} of {@code PathPattern}s matching the given {@code lookupPath}. + * Parse the given {@code rawPattern} and adds it to this registry. + * @param rawPattern raw path pattern to parse and register + * @param handler the associated handler object + */ + public void register(String rawPattern, T handler) { + String fixedPattern = prependLeadingSlash(rawPattern); + PathPattern newPattern = this.pathPatternParser.parse(fixedPattern); + this.patternsMap.put(newPattern, handler); + } + + private static String prependLeadingSlash(String pattern) { + if (StringUtils.hasLength(pattern) && !pattern.startsWith("/")) { + return "/" + pattern; + } + else { + return pattern; + } + } + + + /** + * Return patterns matching the given {@code lookupPath}. *

The returned set sorted with the most specific * patterns first, according to the given {@code lookupPath}. * @param lookupPath the URL lookup path to be matched against @@ -90,21 +111,23 @@ public class PathPatternRegistry { public SortedSet> findMatches(String lookupPath) { return this.patternsMap.entrySet().stream() .filter(entry -> entry.getKey().matches(lookupPath)) + .sorted(Comparator.comparing(Map.Entry::getKey)) .map(entry -> new PathMatchResult<>(entry.getKey(), entry.getValue())) - .collect(Collectors.toCollection(() -> - new TreeSet<>(new PathMatchResultComparator(lookupPath)))); + .collect(Collectors.toCollection(TreeSet::new)); } /** * Return, if any, the most specific {@code PathPattern} matching the given {@code lookupPath}. * @param lookupPath the URL lookup path to be matched against */ - public Optional> findFirstMatch(String lookupPath) { + @Nullable + public PathMatchResult findFirstMatch(String lookupPath) { return this.patternsMap.entrySet().stream() .filter(entry -> entry.getKey().matches(lookupPath)) .sorted(Comparator.comparing(Map.Entry::getKey)) .findFirst() - .map(entry -> new PathMatchResult<>(entry.getKey(), entry.getValue())); + .map(entry -> new PathMatchResult<>(entry.getKey(), entry.getValue())) + .orElse(null); } /** @@ -114,61 +137,4 @@ public class PathPatternRegistry { this.patternsMap.clear(); } - /** - * Parse the given {@code rawPattern} and adds it to this registry. - * @param rawPattern raw path pattern to parse and register - * @param handler the associated handler object - */ - public void register(String rawPattern, T handler) { - String fixedPattern = prependLeadingSlash(rawPattern); - PathPattern newPattern = this.pathPatternParser.parse(fixedPattern); - this.patternsMap.put(newPattern, handler); - } - - private String prependLeadingSlash(String pattern) { - if (StringUtils.hasLength(pattern) && !pattern.startsWith("/")) { - return "/" + pattern; - } - else { - return pattern; - } - } - - - private class PathMatchResultComparator implements Comparator> { - - private final String path; - - public PathMatchResultComparator(String path) { - this.path = path; - } - - @Override - public int compare(@Nullable PathMatchResult o1, @Nullable PathMatchResult o2) { - // Nulls get sorted to the end - if (o1 == null) { - return (o2 == null ? 0 : +1); - } - else if (o2 == null) { - return -1; - } - PathPattern p1 = o1.getPattern(); - PathPattern p2 = o2.getPattern(); - // exact matches get sorted first - if (p1.getPatternString().equals(path)) { - return (p2.getPatternString().equals(path)) ? 0 : -1; - } - else if (p2.getPatternString().equals(path)) { - return +1; - } - // compare pattern specificity - int result = p1.compareTo(p2); - // if equal specificity, sort using pattern string - if (result == 0) { - return p1.getPatternString().compareTo(p2.getPatternString()); - } - return result; - } - } - } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/PatternsRequestCondition.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/PatternsRequestCondition.java index 2e630dca99..30798265f8 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/PatternsRequestCondition.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/PatternsRequestCondition.java @@ -20,6 +20,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.Comparator; import java.util.Iterator; import java.util.List; import java.util.Set; @@ -85,8 +86,15 @@ public final class PatternsRequestCondition extends AbstractRequestCondition toSortedSet(Stream stream) { - Set result = stream.sorted().collect(Collectors.toCollection(TreeSet::new)); - return Collections.unmodifiableSet(result); + return Collections.unmodifiableSet(stream.sorted() + .collect(Collectors.toCollection(() -> new TreeSet<>(getPatternComparator())))); + } + + private static Comparator getPatternComparator() { + return (p1, p2) -> { + int index = p1.compareTo(p2); + return (index != 0 ? index : p1.getPatternString().compareTo(p2.getPatternString())); + }; } /** diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/handler/PathPatternRegistryTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/handler/PathPatternRegistryTests.java index 3f8c69e418..69de1467cd 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/handler/PathPatternRegistryTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/handler/PathPatternRegistryTests.java @@ -24,7 +24,6 @@ import java.util.stream.Collectors; import org.hamcrest.BaseMatcher; import org.hamcrest.Description; import org.hamcrest.Matchers; -import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -45,17 +44,13 @@ import static org.junit.Assert.assertThat; */ public class PathPatternRegistryTests { - private PathPatternRegistry registry; + private final PathPatternRegistry registry = new PathPatternRegistry(); private final PathPatternParser parser = new PathPatternParser(); @Rule public ExpectedException thrown = ExpectedException.none(); - @Before - public void setUp() throws Exception { - this.registry = new PathPatternRegistry(); - } @Test public void shouldPrependPatternsWithSlash() { @@ -79,7 +74,7 @@ public class PathPatternRegistryTests { this.registry.register("/fo?", new Object()); this.registry.register("/f?o", new Object()); Set> matches = this.registry.findMatches("/foo"); - assertThat(getPatternList(matches), contains(pattern("/f?o"), pattern("/fo?"))); + assertThat(toPatterns(matches), contains(pattern("/f?o"), pattern("/fo?"))); } @Test @@ -94,15 +89,13 @@ public class PathPatternRegistryTests { this.registry.register("/foo/bar/baz", new Object()); this.registry.register("/foo/bar/{baz}", new Object()); Set> matches = this.registry.findMatches("/foo/bar/baz"); - assertThat(getPatternList(matches), contains(pattern("/foo/bar/baz"), pattern("/foo/bar/{baz}"), + assertThat(toPatterns(matches), contains(pattern("/foo/bar/baz"), pattern("/foo/bar/{baz}"), pattern("/foo/{*baz}"))); } - private List getPatternList(Collection> results) { - return results.stream() - .map(result -> result.getPattern()).collect(Collectors.toList()); - + private List toPatterns(Collection> results) { + return results.stream().map(PathMatchResult::getPattern).collect(Collectors.toList()); } private static PathPatternMatcher pattern(String pattern) { diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/PatternsRequestConditionTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/PatternsRequestConditionTests.java index 26b878659c..19bb9a208b 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/PatternsRequestConditionTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/PatternsRequestConditionTests.java @@ -16,10 +16,14 @@ package org.springframework.web.reactive.result.condition; +import java.util.Arrays; +import java.util.Iterator; + import org.junit.Test; import org.springframework.mock.http.server.reactive.test.MockServerWebExchange; import org.springframework.web.server.ServerWebExchange; +import org.springframework.web.util.pattern.PathPattern; import org.springframework.web.util.pattern.PathPatternParser; import static org.junit.Assert.assertEquals; @@ -143,6 +147,15 @@ public class PatternsRequestConditionTests { assertEquals(0, c1.compareTo(c2, get("/foo").toExchange())); } + @Test + public void equallyMatchingPatternsAreBothPresent() throws Exception { + PatternsRequestCondition c = new PatternsRequestCondition("/a", "/b"); + assertEquals(2, c.getPatterns().size()); + Iterator itr = c.getPatterns().iterator(); + assertEquals("/a", itr.next().getPatternString()); + assertEquals("/b", itr.next().getPatternString()); + } + @Test public void comparePatternSpecificity() throws Exception { PatternsRequestCondition c1 = new PatternsRequestCondition("/fo*");