diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/PathPatternComparator.java b/spring-web/src/main/java/org/springframework/web/util/pattern/PathPatternComparator.java deleted file mode 100644 index b589bd0163..0000000000 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/PathPatternComparator.java +++ /dev/null @@ -1,72 +0,0 @@ -/* - * Copyright 2002-2017 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.web.util.pattern; - -import java.util.Comparator; - -import org.springframework.lang.Nullable; - -/** - * {@link PathPattern} comparator that takes account of a specified - * path and sorts anything that exactly matches it to be first. - * - *

Patterns that have the same specificity are then compared - * using their String representation, in order to avoid - * considering them as "duplicates" when sorting them - * in {@link java.util.Set} collections. - * - * @author Brian Clozel - * @since 5.0 - */ -public class PathPatternComparator implements Comparator { - - private final String path; - - - public PathPatternComparator(String path) { - this.path = path; - } - - - @Override - public int compare(@Nullable PathPattern o1, @Nullable PathPattern o2) { - // Nulls get sorted to the end - if (o1 == null) { - return (o2 == null ? 0 : +1); - } - else if (o2 == null) { - return -1; - } - - // exact matches get sorted first - if (o1.getPatternString().equals(path)) { - return (o2.getPatternString().equals(path)) ? 0 : -1; - } - else if (o2.getPatternString().equals(path)) { - return +1; - } - - // compare pattern specificity - int result = o1.compareTo(o2); - // if equal specificity, sort using pattern string - if (result == 0) { - return o1.getPatternString().compareTo(o2.getPatternString()); - } - return result; - } - -} 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 99be68ea19..7cbfd5f197 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 @@ -28,7 +28,6 @@ import java.util.stream.Collectors; import org.springframework.lang.Nullable; import org.springframework.util.StringUtils; import org.springframework.web.util.pattern.PathPattern; -import org.springframework.web.util.pattern.PathPatternComparator; import org.springframework.web.util.pattern.PathPatternParser; /** @@ -101,10 +100,10 @@ public class PathPatternRegistry { * @param lookupPath the URL lookup path to be matched against */ public Optional> findFirstMatch(String lookupPath) { - PathPatternComparator comparator = new PathPatternComparator(lookupPath); return this.patternsMap.entrySet().stream() .filter(entry -> entry.getKey().matches(lookupPath)) - .reduce((e1, e2) -> comparator.compare(e1.getKey(), e2.getKey()) < 0 ? e1 : e2) + .sorted(Comparator.comparing(Map.Entry::getKey)) + .findFirst() .map(entry -> new PathMatchResult<>(entry.getKey(), entry.getValue())); } 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 aa0a87208d..2e630dca99 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 @@ -26,12 +26,13 @@ import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.springframework.http.server.reactive.PathContainer; import org.springframework.lang.Nullable; import org.springframework.util.StringUtils; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.util.pattern.PathPattern; -import org.springframework.web.util.pattern.PathPatternComparator; import org.springframework.web.util.pattern.PathPatternParser; /** @@ -44,17 +45,18 @@ import org.springframework.web.util.pattern.PathPatternParser; */ public final class PatternsRequestCondition extends AbstractRequestCondition { - private final List patterns; + private final Set patterns; private final PathPatternParser parser; + /** * Creates a new instance with the given URL patterns. * Each pattern is prepended with "/" if not already. * @param patterns 0 or more URL patterns; if 0 the condition will match to every request. */ public PatternsRequestCondition(String... patterns) { - this(asList(patterns), null); + this(patterns, null); } /** @@ -64,44 +66,39 @@ public final class PatternsRequestCondition extends AbstractRequestCondition patterns, PathPatternParser patternParser) { - this.parser = patternParser != null ? patternParser : new PathPatternParser(); - this.patterns = new ArrayList<>(); - patterns.forEach(pattern -> { - if (StringUtils.hasText(pattern) && !pattern.startsWith("/")) { - pattern = "/" + pattern; - } - this.patterns.add(this.parser.parse(pattern)); - }); + private PatternsRequestCondition(Collection patterns, PathPatternParser parser) { + this.parser = (parser != null ? parser : new PathPatternParser()); + this.patterns = toSortedSet(patterns.stream().map(pattern -> parse(pattern, this.parser))); + } + + private static PathPattern parse(String pattern, PathPatternParser parser) { + if (StringUtils.hasText(pattern) && !pattern.startsWith("/")) { + pattern = "/" + pattern; + } + return parser.parse(pattern); + } + + private static Set toSortedSet(Stream stream) { + Set result = stream.sorted().collect(Collectors.toCollection(TreeSet::new)); + return Collections.unmodifiableSet(result); } /** * Private constructor accepting a list of path patterns. */ private PatternsRequestCondition(List patterns, PathPatternParser patternParser) { - this.patterns = patterns; + this.patterns = toSortedSet(patterns.stream()); this.parser = patternParser; } - - private static List asList(String... patterns) { - return (patterns != null ? Arrays.asList(patterns) : Collections.emptyList()); - } - public Set getPatterns() { - return new TreeSet<>(this.patterns); - } - - public Set getPatternStrings() { - return this.patterns.stream() - .map(PathPattern::toString).collect(Collectors.toSet()); + return this.patterns; } @Override @@ -143,14 +140,12 @@ public final class PatternsRequestCondition extends AbstractRequestCondition matches = getMatchingPatterns(lookupPath); - return matches.isEmpty() ? null : new PatternsRequestCondition( - new ArrayList(matches), this.parser); + SortedSet matches = getMatchingPatterns(exchange); + return matches.isEmpty() ? null : + new PatternsRequestCondition(new ArrayList(matches), this.parser); } /** @@ -175,20 +168,19 @@ public final class PatternsRequestCondition extends AbstractRequestCondition getMatchingPatterns(String lookupPath) { + private SortedSet getMatchingPatterns(ServerWebExchange exchange) { + PathContainer lookupPath = exchange.getRequest().getPath().pathWithinApplication(); return patterns.stream() .filter(pattern -> pattern.matches(lookupPath)) - .collect(Collectors.toCollection(() -> - new TreeSet<>(new PathPatternComparator(lookupPath)))); + .collect(Collectors.toCollection(TreeSet::new)); } /** * Compare the two conditions based on the URL patterns they contain. - * Patterns are compared one at a time, from top to bottom via - * {@link PathPatternComparator}. If all compared + * Patterns are compared one at a time, from top to bottom. If all compared * patterns match equally, but one instance has more patterns, it is * considered a closer match. *

It is assumed that both instances have been obtained via @@ -198,14 +190,10 @@ public final class PatternsRequestCondition extends AbstractRequestCondition iterator = this.patterns.stream() - .sorted(comparator).collect(Collectors.toList()).iterator(); - Iterator iteratorOther = other.getPatterns().stream() - .sorted(comparator).collect(Collectors.toList()).iterator(); + Iterator iterator = this.patterns.iterator(); + Iterator iteratorOther = other.getPatterns().iterator(); while (iterator.hasNext() && iteratorOther.hasNext()) { - int result = comparator.compare(iterator.next(), iteratorOther.next()); + int result = iterator.next().compareTo(iteratorOther.next()); if (result != 0) { return result; } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java index b075544191..aeccd29802 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java @@ -292,7 +292,7 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap * @param lookupPath the lookup path within the current mapping * @param exchange the current exchange * @return the best-matching handler method, or {@code null} if no match - * @see #handleMatch(Object, String, ServerWebExchange) + * @see #handleMatch(Object, HandlerMethod, String, ServerWebExchange) * @see #handleNoMatch(Set, String, ServerWebExchange) */ @Nullable @@ -400,11 +400,6 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap @Nullable protected abstract T getMappingForMethod(Method method, Class handlerType); - /** - * Extract and return the URL paths contained in a mapping. - */ - protected abstract Set getMappingPathPatterns(T mapping); - /** * Check if a mapping matches the current request and return a (potentially * new) mapping with conditions relevant to the current request. diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMapping.java index 32b962d77f..00b3d7d7ae 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMapping.java @@ -66,14 +66,6 @@ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMe } - /** - * Get the URL path patterns associated with this {@link RequestMappingInfo}. - */ - @Override - protected Set getMappingPathPatterns(RequestMappingInfo info) { - return info.getPatternsCondition().getPatternStrings(); - } - /** * Check if the given RequestMappingInfo matches the current request and * return a (potentially new) instance with conditions that match the diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/HandlerMethodMappingTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/HandlerMethodMappingTests.java index 53df01de07..ef429c90e1 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/HandlerMethodMappingTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/HandlerMethodMappingTests.java @@ -17,9 +17,7 @@ package org.springframework.web.reactive.result.method; import java.lang.reflect.Method; -import java.util.Collections; import java.util.Comparator; -import java.util.Set; import org.hamcrest.Matchers; import org.junit.Before; @@ -35,7 +33,10 @@ import org.springframework.web.method.HandlerMethod; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.util.pattern.ParsingPathMatcher; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; /** * Unit tests for {@link AbstractHandlerMethodMapping}. @@ -149,11 +150,6 @@ public class HandlerMethodMappingTests { return methodName.startsWith("handler") ? methodName : null; } - @Override - protected Set getMappingPathPatterns(String key) { - return (this.pathMatcher.isPattern(key) ? Collections.emptySet() : Collections.singleton(key)); - } - @Override protected String getMatchingMapping(String pattern, ServerWebExchange exchange) { String lookupPath = exchange.getRequest().getURI().getPath(); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java index a8c7ea8280..4a81a1cf78 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java @@ -21,7 +21,6 @@ import java.net.URI; import java.util.Arrays; import java.util.Collections; import java.util.EnumSet; -import java.util.HashSet; import java.util.Map; import java.util.Set; import java.util.function.Consumer; @@ -94,15 +93,6 @@ public class RequestMappingInfoHandlerMappingTests { } - @Test - public void getMappingPathPatterns() throws Exception { - String[] patterns = {"/foo/*", "/foo", "/bar/*", "/bar"}; - RequestMappingInfo info = paths(patterns).build(); - Set actual = this.handlerMapping.getMappingPathPatterns(info); - - assertEquals(new HashSet<>(Arrays.asList(patterns)), actual); - } - @Test public void getHandlerDirectMatch() throws Exception { Method expected = on(TestController.class).annot(getMapping("/foo").params()).resolveMethod(); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMappingTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMappingTests.java index e0dd6e6b62..eb1c6ca7d8 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMappingTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMappingTests.java @@ -39,6 +39,7 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.context.support.StaticWebApplicationContext; import org.springframework.web.reactive.result.method.RequestMappingInfo; +import org.springframework.web.util.pattern.PathPattern; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; @@ -133,9 +134,9 @@ public class RequestMappingHandlerMappingTests { assertNotNull(info); - Set paths = info.getPatternsCondition().getPatternStrings(); + Set paths = info.getPatternsCondition().getPatterns(); assertEquals(1, paths.size()); - assertEquals(path, paths.iterator().next()); + assertEquals(path, paths.iterator().next().getPatternString()); Set methods = info.getMethodsCondition().getMethods(); assertEquals(1, methods.size());