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 new file mode 100644 index 0000000000..b4a5dd8a66 --- /dev/null +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/PathPatternComparator.java @@ -0,0 +1,66 @@ +/* + * 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; + +/** + * {@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(PathPattern o1, 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; + } + +} \ No newline at end of file diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/config/PathMatchConfigurer.java b/spring-webflux/src/main/java/org/springframework/web/reactive/config/PathMatchConfigurer.java index 16a7f3f137..3202ea5a8e 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/config/PathMatchConfigurer.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/config/PathMatchConfigurer.java @@ -17,34 +17,27 @@ package org.springframework.web.reactive.config; import org.springframework.lang.Nullable; -import org.springframework.util.PathMatcher; -import org.springframework.web.util.pattern.ParsingPathMatcher; /** * Assist with configuring {@code HandlerMapping}'s with path matching options. * * @author Rossen Stoyanchev + * @author Brian Clozel * @since 5.0 */ public class PathMatchConfigurer { - private Boolean suffixPatternMatch; - private Boolean trailingSlashMatch; - private Boolean registeredSuffixPatternMatch; - - private PathMatcher pathMatcher; - + private Boolean caseSensitiveMatch; /** - * Whether to use suffix pattern match (".*") when matching patterns to - * requests. If enabled a method mapped to "/users" also matches to "/users.*". - *

By default this is set to {@code true}. - * @see #registeredSuffixPatternMatch + * Whether to match to URLs irrespective of their case. + * If enabled a method mapped to "/users" won't match to "/Users/". + *

The default value is {@code false}. */ - public PathMatchConfigurer setUseSuffixPatternMatch(Boolean suffixPatternMatch) { - this.suffixPatternMatch = suffixPatternMatch; + public PathMatchConfigurer setUseCaseSensitiveMatch(Boolean caseSensitiveMatch) { + this.caseSensitiveMatch = caseSensitiveMatch; return this; } @@ -58,49 +51,14 @@ public class PathMatchConfigurer { return this; } - /** - * Whether suffix pattern matching should work only against path extensions - * that are explicitly registered. This is generally recommended to reduce - * ambiguity and to avoid issues such as when a "." (dot) appears in the path - * for other reasons. - *

By default this is set to "true". - */ - public PathMatchConfigurer setUseRegisteredSuffixPatternMatch(Boolean registeredSuffixPatternMatch) { - this.registeredSuffixPatternMatch = registeredSuffixPatternMatch; - return this; - } - - /** - * Set the PathMatcher for matching URL paths against registered URL patterns. - *

The default is a {@link org.springframework.web.util.pattern.ParsingPathMatcher}. - */ - public PathMatchConfigurer setPathMatcher(PathMatcher pathMatcher) { - this.pathMatcher = pathMatcher; - return this; - } - - @Nullable - protected Boolean isUseSuffixPatternMatch() { - return this.suffixPatternMatch; - } - @Nullable protected Boolean isUseTrailingSlashMatch() { return this.trailingSlashMatch; } @Nullable - protected Boolean isUseRegisteredSuffixPatternMatch() { - return this.registeredSuffixPatternMatch; - } - - @Nullable - public PathMatcher getPathMatcher() { - if (this.pathMatcher instanceof ParsingPathMatcher && (this.trailingSlashMatch || this.suffixPatternMatch)) { - throw new IllegalStateException("When using a ParsingPathMatcher, useTrailingSlashMatch" + - " and useSuffixPatternMatch should be set to 'false'."); - } - return this.pathMatcher; + protected Boolean isUseCaseSensitiveMatch() { + return this.caseSensitiveMatch; } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/config/ResourceHandlerRegistry.java b/spring-webflux/src/main/java/org/springframework/web/reactive/config/ResourceHandlerRegistry.java index 83d0fcdf87..d58c8a60b2 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/config/ResourceHandlerRegistry.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/config/ResourceHandlerRegistry.java @@ -26,7 +26,7 @@ import org.springframework.beans.factory.BeanInitializationException; import org.springframework.context.ApplicationContext; import org.springframework.lang.Nullable; import org.springframework.util.Assert; -import org.springframework.web.reactive.handler.AbstractHandlerMapping; +import org.springframework.web.reactive.handler.AbstractUrlHandlerMapping; import org.springframework.web.reactive.handler.SimpleUrlHandlerMapping; import org.springframework.web.reactive.resource.ResourceWebHandler; import org.springframework.web.server.WebHandler; @@ -76,7 +76,7 @@ public class ResourceHandlerRegistry { * URL path patterns. The handler will be invoked for every incoming request * that matches to one of the specified path patterns. *

Patterns like {@code "/static/**"} or {@code "/css/{filename:\\w+\\.css}"} - * are allowed. See {@link org.springframework.web.util.pattern.ParsingPathMatcher} + * are allowed. See {@link org.springframework.web.util.pattern.PathPattern} * for more details on the syntax. * @return A {@link ResourceHandlerRegistration} to use to further * configure the registered resource handler @@ -115,7 +115,7 @@ public class ResourceHandlerRegistry { * of no registrations. */ @Nullable - protected AbstractHandlerMapping getHandlerMapping() { + protected AbstractUrlHandlerMapping getHandlerMapping() { if (this.registrations.isEmpty()) { return null; } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurationSupport.java b/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurationSupport.java index 50ebeca86e..77926521fe 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurationSupport.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurationSupport.java @@ -36,7 +36,6 @@ import org.springframework.format.support.FormattingConversionService; import org.springframework.http.codec.ServerCodecConfigurer; import org.springframework.lang.Nullable; import org.springframework.util.ClassUtils; -import org.springframework.util.PathMatcher; import org.springframework.validation.Errors; import org.springframework.validation.MessageCodesResolver; import org.springframework.validation.Validator; @@ -73,13 +72,6 @@ import org.springframework.web.server.handler.ResponseStatusExceptionHandler; */ public class WebFluxConfigurationSupport implements ApplicationContextAware { - static final boolean jackson2Present = - ClassUtils.isPresent("com.fasterxml.jackson.databind.ObjectMapper", - WebFluxConfigurationSupport.class.getClassLoader()) && - ClassUtils.isPresent("com.fasterxml.jackson.core.JsonGenerator", - WebFluxConfigurationSupport.class.getClassLoader()); - - private Map corsConfigurations; private PathMatchConfigurer pathMatchConfigurer; @@ -118,21 +110,11 @@ public class WebFluxConfigurationSupport implements ApplicationContextAware { mapping.setCorsConfigurations(getCorsConfigurations()); PathMatchConfigurer configurer = getPathMatchConfigurer(); - Boolean useSuffixPatternMatch = configurer.isUseSuffixPatternMatch(); - Boolean useRegisteredSuffixPatternMatch = configurer.isUseRegisteredSuffixPatternMatch(); - Boolean useTrailingSlashMatch = configurer.isUseTrailingSlashMatch(); - if (useSuffixPatternMatch != null) { - mapping.setUseSuffixPatternMatch(useSuffixPatternMatch); - } - if (useRegisteredSuffixPatternMatch != null) { - mapping.setUseRegisteredSuffixPatternMatch(useRegisteredSuffixPatternMatch); + if (configurer.isUseTrailingSlashMatch() != null) { + mapping.setUseTrailingSlashMatch(configurer.isUseTrailingSlashMatch()); } - if (useTrailingSlashMatch != null) { - mapping.setUseTrailingSlashMatch(useTrailingSlashMatch); - } - PathMatcher pathMatcher = configurer.getPathMatcher(); - if (pathMatcher != null) { - mapping.setPathMatcher(pathMatcher); + if (configurer.isUseCaseSensitiveMatch() != null) { + mapping.setUseCaseSensitiveMatch(configurer.isUseCaseSensitiveMatch()); } return mapping; } @@ -224,9 +206,12 @@ public class WebFluxConfigurationSupport implements ApplicationContextAware { AbstractHandlerMapping handlerMapping = registry.getHandlerMapping(); if (handlerMapping != null) { - PathMatchConfigurer pathMatchConfigurer = getPathMatchConfigurer(); - if (pathMatchConfigurer.getPathMatcher() != null) { - handlerMapping.setPathMatcher(pathMatchConfigurer.getPathMatcher()); + PathMatchConfigurer configurer = getPathMatchConfigurer(); + if (configurer.isUseTrailingSlashMatch() != null) { + handlerMapping.setUseTrailingSlashMatch(configurer.isUseTrailingSlashMatch()); + } + if (configurer.isUseCaseSensitiveMatch() != null) { + handlerMapping.setUseCaseSensitiveMatch(configurer.isUseCaseSensitiveMatch()); } } else { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurer.java b/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurer.java index bca49f3a4b..11700ef2f5 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurer.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurer.java @@ -58,13 +58,6 @@ public interface WebFluxConfigurer { /** * Configure path matching options. * - *

Note that if a {@link org.springframework.web.util.pattern.ParsingPathMatcher} - * is configured here, - * the {@link PathMatchConfigurer#setUseTrailingSlashMatch(Boolean)} and - * {@link PathMatchConfigurer#setUseSuffixPatternMatch(Boolean)} options must be set - * to {@literal false}as they can lead to illegal patterns, - * see {@link org.springframework.web.util.pattern.ParsingPathMatcher}. - * * {@code HandlerMapping}s with path matching options. * @param configurer the {@link PathMatchConfigurer} instance */ diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractHandlerMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractHandlerMapping.java index 6bbaabaa92..f1d6f8ffa0 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractHandlerMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractHandlerMapping.java @@ -24,7 +24,6 @@ import org.springframework.context.support.ApplicationObjectSupport; import org.springframework.core.Ordered; import org.springframework.lang.Nullable; import org.springframework.util.Assert; -import org.springframework.util.PathMatcher; import org.springframework.web.cors.CorsConfiguration; import org.springframework.web.cors.reactive.CorsConfigurationSource; import org.springframework.web.cors.reactive.CorsProcessor; @@ -34,7 +33,7 @@ import org.springframework.web.cors.reactive.UrlBasedCorsConfigurationSource; import org.springframework.web.reactive.HandlerMapping; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebHandler; -import org.springframework.web.util.pattern.ParsingPathMatcher; +import org.springframework.web.util.pattern.PathPatternParser; /** * Abstract base class for {@link org.springframework.web.reactive.HandlerMapping} @@ -52,7 +51,7 @@ public abstract class AbstractHandlerMapping extends ApplicationObjectSupport im private int order = Integer.MAX_VALUE; // default: same as non-Ordered - private PathMatcher pathMatcher = new ParsingPathMatcher(); + private PathPatternParser patternParser = new PathPatternParser(); private final UrlBasedCorsConfigurationSource globalCorsConfigSource = new UrlBasedCorsConfigurationSource(); @@ -74,22 +73,28 @@ public abstract class AbstractHandlerMapping extends ApplicationObjectSupport im } /** - * Set the PathMatcher implementation to use for matching URL paths - * against registered URL patterns. - *

The default is a {@link ParsingPathMatcher}. + * Whether to match to URLs irrespective of their case. + * If enabled a method mapped to "/users" won't match to "/Users/". + *

The default value is {@code false}. */ - public void setPathMatcher(PathMatcher pathMatcher) { - Assert.notNull(pathMatcher, "PathMatcher must not be null"); - this.pathMatcher = pathMatcher; - this.globalCorsConfigSource.setPathMatcher(pathMatcher); + public void setUseCaseSensitiveMatch(boolean caseSensitiveMatch) { + this.patternParser.setCaseSensitive(caseSensitiveMatch); } /** - * Return the PathMatcher implementation to use for matching URL paths - * against registered URL patterns. + * Whether to match to URLs irrespective of the presence of a trailing slash. + * If enabled a method mapped to "/users" also matches to "/users/". + *

The default value is {@code true}. */ - public PathMatcher getPathMatcher() { - return this.pathMatcher; + public void setUseTrailingSlashMatch(boolean trailingSlashMatch) { + this.patternParser.setMatchOptionalTrailingSlash(trailingSlashMatch); + } + + /** + * Return the {@link PathPatternParser} instance. + */ + public PathPatternParser getPathPatternParser() { + return this.patternParser; } /** 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 388eb091cb..c40f05e012 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 @@ -16,12 +16,8 @@ package org.springframework.web.reactive.handler; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Comparator; -import java.util.LinkedHashMap; -import java.util.List; import java.util.Map; +import java.util.Optional; import reactor.core.publisher.Mono; @@ -29,20 +25,21 @@ import org.springframework.beans.BeansException; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.web.server.ServerWebExchange; +import org.springframework.web.util.pattern.PathPattern; /** * Abstract base class for URL-mapped * {@link org.springframework.web.reactive.HandlerMapping} implementations. * *

Supports direct matches, e.g. a registered "/test" matches "/test", and - * various Ant-style pattern matches, e.g. a registered "/t*" pattern matches + * various path pattern matches, e.g. a registered "/t*" pattern matches * both "/test" and "/team", "/test/*" matches all paths under "/test", * "/test/**" matches all paths below "/test". For details, see the - * {@link org.springframework.web.util.pattern.ParsingPathMatcher} javadoc. + * {@link org.springframework.web.util.pattern.PathPattern} javadoc. * - *

Will search all path patterns to find the most exact match for the - * current request path. The most exact match is defined as the longest - * path pattern that matches the current request path. + *

Will search all path patterns to find the most specific match for the + * current request path. The most specific pattern is defined as the longest + * path pattern with the fewest captured variables and wildcards. * * @author Rossen Stoyanchev * @author Juergen Hoeller @@ -51,27 +48,12 @@ import org.springframework.web.server.ServerWebExchange; */ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { - private boolean useTrailingSlashMatch = false; - private boolean lazyInitHandlers = false; - private final Map handlerMap = new LinkedHashMap<>(); - + private PathPatternRegistry patternRegistry; - /** - * Whether to match to URLs irrespective of the presence of a trailing slash. - * If enabled a URL pattern such as "/users" also matches to "/users/". - *

The default value is {@code false}. - */ - public void setUseTrailingSlashMatch(boolean useTrailingSlashMatch) { - this.useTrailingSlashMatch = useTrailingSlashMatch; - } - - /** - * Whether to match to URLs irrespective of the presence of a trailing slash. - */ - public boolean useTrailingSlashMatch() { - return this.useTrailingSlashMatch; + public PathPatternRegistry getPatternRegistry() { + return patternRegistry; } /** @@ -90,11 +72,11 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { /** * Return the registered handlers as an unmodifiable Map, with the registered path - * as key and the handler object (or handler bean name in case of a lazy-init handler) + * pattern as key and the handler object (or handler bean name in case of a lazy-init handler) * as value. */ - public final Map getHandlerMap() { - return Collections.unmodifiableMap(this.handlerMap); + public final Map getHandlerMap() { + return this.patternRegistry.getPatternsMap(); } @@ -129,58 +111,30 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { * @param lookupPath URL the handler is mapped to * @param exchange the current exchange * @return the associated handler instance, or {@code null} if not found - * @see org.springframework.web.util.pattern.ParsingPathMatcher + * @see org.springframework.web.util.pattern.PathPattern */ @Nullable protected Object lookupHandler(String lookupPath, ServerWebExchange exchange) throws Exception { - // Direct match? - Object handler = this.handlerMap.get(lookupPath); - if (handler != null) { - return handleMatch(handler, lookupPath, lookupPath, exchange); - } - - // Pattern match? - List matches = new ArrayList<>(); - for (String pattern : this.handlerMap.keySet()) { - if (getPathMatcher().match(pattern, lookupPath)) { - matches.add(pattern); - } - else if (useTrailingSlashMatch()) { - if (!pattern.endsWith("/") && getPathMatcher().match(pattern + "/", lookupPath)) { - matches.add(pattern +"/"); - } - } - } - - String bestMatch = null; - Comparator comparator = getPathMatcher().getPatternComparator(lookupPath); - if (!matches.isEmpty()) { - Collections.sort(matches, comparator); + Optional> matches = this.patternRegistry.findFirstMatch(lookupPath); + if (matches.isPresent()) { if (logger.isDebugEnabled()) { logger.debug("Matching patterns for request [" + lookupPath + "] are " + matches); } - bestMatch = matches.get(0); - } - if (bestMatch != null) { - handler = this.handlerMap.get(bestMatch); + PathMatchResult bestMatch = matches.get(); + String pathWithinMapping = bestMatch.getPattern().extractPathWithinPattern(lookupPath); + Object handler = bestMatch.getHandler(); if (handler == null) { - if (bestMatch.endsWith("/")) { - handler = this.handlerMap.get(bestMatch.substring(0, bestMatch.length() - 1)); - } - if (handler == null) { - throw new IllegalStateException( - "Could not find handler for best pattern match [" + bestMatch + "]"); - } + throw new IllegalStateException( + "Could not find handler for best pattern match [" + bestMatch + "]"); } - String pathWithinMapping = getPathMatcher().extractPathWithinPattern(bestMatch, lookupPath); - return handleMatch(handler, bestMatch, pathWithinMapping, exchange); + return handleMatch(handler, bestMatch.getPattern(), pathWithinMapping, exchange); } // No handler found... return null; } - private Object handleMatch(Object handler, String bestMatch, String pathWithinMapping, + private Object handleMatch(Object handler, PathPattern bestMatch, String pathWithinMapping, ServerWebExchange exchange) throws Exception { // Bean name or resolved handler? @@ -236,6 +190,7 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { Assert.notNull(handler, "Handler object must not be null"); Object resolvedHandler = handler; + // Eagerly resolve handler if referencing singleton via name. if (!this.lazyInitHandlers && handler instanceof String) { String handlerName = (String) handler; @@ -243,20 +198,27 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { resolvedHandler = obtainApplicationContext().getBean(handlerName); } } + if (this.patternRegistry == null) { + this.patternRegistry = new PathPatternRegistry<>(getPathPatternParser()); + } - Object mappedHandler = this.handlerMap.get(urlPath); - if (mappedHandler != null) { - if (mappedHandler != resolvedHandler) { - throw new IllegalStateException( - "Cannot map " + getHandlerDescription(handler) + " to URL path [" + urlPath + - "]: There is already " + getHandlerDescription(mappedHandler) + " mapped."); + Map patternsMap = this.patternRegistry.getPatternsMap(); + if (patternsMap.containsKey(urlPath)) { + Object mappedHandler = patternsMap.get(urlPath); + if (mappedHandler != null) { + if (mappedHandler != resolvedHandler) { + throw new IllegalStateException( + "Cannot map " + getHandlerDescription(handler) + " to URL path [" + urlPath + + "]: There is already " + getHandlerDescription(mappedHandler) + " mapped."); + } } } else { - this.handlerMap.put(urlPath, resolvedHandler); - if (logger.isInfoEnabled()) { - logger.info("Mapped URL path [" + urlPath + "] onto " + getHandlerDescription(handler)); - } + this.patternRegistry.register(urlPath, resolvedHandler); + } + + if (logger.isInfoEnabled()) { + logger.info("Mapped URL path [" + urlPath + "] onto " + getHandlerDescription(handler)); } } 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 new file mode 100644 index 0000000000..1886482a19 --- /dev/null +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/PathMatchResult.java @@ -0,0 +1,58 @@ +/* + * 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.reactive.handler; + +import org.springframework.lang.Nullable; +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 optionnally associates the matching {@link PathPattern} + * with a request handler of type {@code T}. + * @author Brian Clozel + * @since 5.0 + * @see PathPatternRegistry + */ +public class PathMatchResult { + + private final PathPattern pattern; + + private final T handler; + + public PathMatchResult(PathPattern pattern, T handler) { + Assert.notNull(pattern, "PathPattern should not be null"); + this.pattern = pattern; + this.handler = handler; + } + + /** + * Return the {@link PathPattern} that matched the incoming request. + */ + public PathPattern getPattern() { + return pattern; + } + + /** + * Return the request handler associated with the {@link PathPattern}. + */ + @Nullable + public T getHandler() { + return 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 new file mode 100644 index 0000000000..94f42afa67 --- /dev/null +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/PathPatternRegistry.java @@ -0,0 +1,177 @@ +/* + * 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.reactive.handler; + +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; + +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; + +/** + * Registry that holds {@code PathPattern}s instances + * and allows matching against them with a lookup path. + * + * @author Brian Clozel + * @since 5.0 + */ +public class PathPatternRegistry { + + private final PathPatternParser pathPatternParser; + + private final Map patternsMap; + + + /** + * Create a new {@code PathPatternRegistry} with + * a default instance of {@link PathPatternParser}. + */ + public PathPatternRegistry() { + this(new PathPatternParser()); + } + + /** + * Create a new {@code PathPatternRegistry} using + * the provided instance of {@link PathPatternParser}. + * + * @param patternParser the {@link PathPatternParser} to use + */ + public PathPatternRegistry(PathPatternParser patternParser) { + this(patternParser, Collections.emptyMap()); + } + + /** + * Create a new {@code PathPatternRegistry} using + * the provided instance of {@link PathPatternParser} + * and the given map of {@link PathPattern}. + * + * @param patternParser the {@link PathPatternParser} to use + * @param patternsMap the map of {@link PathPattern} to use + */ + public PathPatternRegistry(PathPatternParser patternParser, Map patternsMap) { + this.pathPatternParser = patternParser; + this.patternsMap = new HashMap<>(patternsMap); + } + + /** + * Return a (read-only) map of all patterns and associated values. + */ + public Map getPatternsMap() { + return Collections.unmodifiableMap(this.patternsMap); + } + + /** + * Return a {@code SortedSet} of {@code PathPattern}s 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 + */ + public SortedSet> findMatches(String lookupPath) { + return this.patternsMap.entrySet().stream() + .filter(entry -> entry.getKey().matches(lookupPath)) + .map(entry -> new PathMatchResult<>(entry.getKey(), entry.getValue())) + .collect(Collectors.toCollection(() -> + new TreeSet<>(new PathMatchResultComparator(lookupPath)))); + } + + /** + * 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) { + 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) + .map(entry -> new PathMatchResult<>(entry.getKey(), entry.getValue())); + } + + /** + * Remove all {@link PathPattern}s from this registry + */ + public void clear() { + 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 + */ + 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(PathMatchResult o1, 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; + } + + } + +} \ No newline at end of file diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/SimpleUrlHandlerMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/SimpleUrlHandlerMapping.java index 7845a85840..1297a58683 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/SimpleUrlHandlerMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/SimpleUrlHandlerMapping.java @@ -46,7 +46,7 @@ import org.springframework.util.CollectionUtils; * various Ant-style pattern matches, e.g. a registered "/t*" pattern matches * both "/test" and "/team", "/test/*" matches all paths under "/test", * "/test/**" matches all paths below "/test". For details, see the - * {@link org.springframework.web.util.pattern.ParsingPathMatcher} javadoc. + * {@link org.springframework.web.util.pattern.PathPattern} javadoc. * * @author Rossen Stoyanchev * @since 5.0 @@ -60,7 +60,7 @@ public class SimpleUrlHandlerMapping extends AbstractUrlHandlerMapping { * Map URL paths to handler bean names. * This is the typical way of configuring this HandlerMapping. *

Supports direct URL matches and Ant-style pattern matches. For syntax details, - * see the {@link org.springframework.web.util.pattern.ParsingPathMatcher} javadoc. + * see the {@link org.springframework.web.util.pattern.PathPattern} javadoc. * @param mappings properties with URLs as keys and bean names as values * @see #setUrlMap */ @@ -72,7 +72,7 @@ public class SimpleUrlHandlerMapping extends AbstractUrlHandlerMapping { * Set a Map with URL paths as keys and handler beans (or handler bean names) * as values. Convenient for population with bean references. *

Supports direct URL matches and Ant-style pattern matches. For syntax details, - * see the {@link org.springframework.web.util.pattern.ParsingPathMatcher} javadoc. + * see the {@link org.springframework.web.util.pattern.PathPattern} javadoc. * @param urlMap map with URLs as keys and beans as values * @see #setMappings */ diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java index 58627c3db8..a6856081b7 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java @@ -17,11 +17,9 @@ package org.springframework.web.reactive.resource; import java.util.ArrayList; -import java.util.Collections; -import java.util.Comparator; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -34,10 +32,12 @@ import org.springframework.context.event.ContextRefreshedEvent; import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.lang.Nullable; -import org.springframework.util.PathMatcher; import org.springframework.web.reactive.handler.SimpleUrlHandlerMapping; import org.springframework.web.server.ServerWebExchange; -import org.springframework.web.util.pattern.ParsingPathMatcher; +import org.springframework.web.reactive.handler.PathMatchResult; +import org.springframework.web.util.pattern.PathPattern; +import org.springframework.web.util.pattern.PathPatternParser; +import org.springframework.web.reactive.handler.PathPatternRegistry; /** * A central component to use to obtain the public URL path that clients should @@ -54,26 +54,17 @@ public class ResourceUrlProvider implements ApplicationListener handlerMap = new LinkedHashMap<>(); + private PathPatternRegistry patternRegistry = new PathPatternRegistry<>(); private boolean autodetect = true; /** - * Configure a {@code PathMatcher} to use when comparing target lookup path + * Configure a {@code PathPatternParser} to use when comparing target lookup path * against resource mappings. */ - public void setPathMatcher(PathMatcher pathMatcher) { - this.pathMatcher = pathMatcher; - } - - /** - * Return the configured {@code PathMatcher}. - */ - public PathMatcher getPathMatcher() { - return this.pathMatcher; + public void setPathPatternParser(PathPatternParser patternParser) { + this.patternRegistry = new PathPatternRegistry<>(patternParser); } /** @@ -84,8 +75,8 @@ public class ResourceUrlProvider implements ApplicationListener handlerMap) { if (handlerMap != null) { - this.handlerMap.clear(); - this.handlerMap.putAll(handlerMap); + this.patternRegistry.clear(); + handlerMap.forEach(this.patternRegistry::register); this.autodetect = false; } } @@ -94,8 +85,8 @@ public class ResourceUrlProvider implements ApplicationListener getHandlerMap() { - return this.handlerMap; + public Map getHandlerMap() { + return this.patternRegistry.getPatternsMap(); } /** @@ -109,14 +100,14 @@ public class ResourceUrlProvider implements ApplicationListener 0) { + if (queryIndex > 0) { suffixIndex = queryIndex; } int hashIndex = lookupPath.indexOf("#"); - if(hashIndex > 0) { + if (hashIndex > 0) { suffixIndex = Math.min(suffixIndex, hashIndex); } return suffixIndex; @@ -200,28 +191,20 @@ public class ResourceUrlProvider implements ApplicationListener matchingPatterns = new ArrayList<>(); - for (String pattern : this.handlerMap.keySet()) { - if (getPathMatcher().match(pattern, lookupPath)) { - matchingPatterns.add(pattern); - } - } + Set> matchResults = this.patternRegistry.findMatches(lookupPath); - if (matchingPatterns.isEmpty()) { + if (matchResults.isEmpty()) { return Mono.empty(); } - Comparator patternComparator = getPathMatcher().getPatternComparator(lookupPath); - Collections.sort(matchingPatterns, patternComparator); - - return Flux.fromIterable(matchingPatterns) - .concatMap(pattern -> { - String pathWithinMapping = getPathMatcher().extractPathWithinPattern(pattern, lookupPath); + return Flux.fromIterable(matchResults) + .concatMap(result -> { + String pathWithinMapping = result.getPattern().extractPathWithinPattern(lookupPath); String pathMapping = lookupPath.substring(0, lookupPath.indexOf(pathWithinMapping)); if (logger.isTraceEnabled()) { - logger.trace("Invoking ResourceResolverChain for URL pattern \"" + pattern + "\""); + logger.trace("Invoking ResourceResolverChain for URL pattern \"" + result.getPattern() + "\""); } - ResourceWebHandler handler = this.handlerMap.get(pattern); + ResourceWebHandler handler = result.getHandler(); ResourceResolverChain chain = new DefaultResourceResolverChain(handler.getResourceResolvers()); return chain.resolveUrlPath(pathWithinMapping, handler.getLocations()) .map(resolvedPath -> { 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 874786a7ee..caa4376565 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,82 +20,74 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; -import java.util.Comparator; -import java.util.HashSet; import java.util.Iterator; -import java.util.LinkedHashSet; import java.util.List; import java.util.Set; +import java.util.SortedSet; +import java.util.TreeSet; +import java.util.stream.Collectors; -import org.springframework.util.PathMatcher; +import org.springframework.lang.Nullable; import org.springframework.util.StringUtils; import org.springframework.web.server.ServerWebExchange; -import org.springframework.web.util.pattern.ParsingPathMatcher; +import org.springframework.web.util.pattern.PathPattern; +import org.springframework.web.util.pattern.PathPatternComparator; +import org.springframework.web.util.pattern.PathPatternParser; /** * A logical disjunction (' || ') request condition that matches a request * against a set of URL path patterns. * * @author Rossen Stoyanchev + * @author Brian Clozel * @since 5.0 */ public final class PatternsRequestCondition extends AbstractRequestCondition { - private final Set patterns; - - private final PathMatcher pathMatcher; - - private final boolean useSuffixPatternMatch; - - private final boolean useTrailingSlashMatch; - - private final Set fileExtensions = new HashSet<>(); + private final List patterns; + private final PathPatternParser parser; /** * Creates a new instance with the given URL patterns. - * Each pattern that is not empty and does not start with "/" is prepended with "/". + * 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, true, true, null); + this(asList(patterns), null); } /** * Creates a new instance with the given URL patterns. * Each pattern that is not empty and does not start with "/" is pre-pended with "/". * @param patterns the URL patterns to use; if 0, the condition will match to every request. - * @param pathMatcher for pattern path matching - * @param useSuffixPatternMatch whether to enable matching by suffix (".*") - * @param useTrailingSlashMatch whether to match irrespective of a trailing slash - * @param extensions file extensions to consider for path matching + * @param patternParser for parsing string patterns */ - public PatternsRequestCondition(String[] patterns, PathMatcher pathMatcher, - boolean useSuffixPatternMatch, boolean useTrailingSlashMatch, - Set extensions) { + public PatternsRequestCondition(String[] patterns, PathPatternParser patternParser) { - this(asList(patterns), pathMatcher, useSuffixPatternMatch, useTrailingSlashMatch, extensions); + this(asList(patterns), patternParser); } /** - * Private constructor accepting a collection of patterns. + * Private constructor accepting a collection of raw patterns. */ - private PatternsRequestCondition(Collection patterns, PathMatcher pathMatcher, - boolean useSuffixPatternMatch, boolean useTrailingSlashMatch, - Set fileExtensions) { - - this.patterns = Collections.unmodifiableSet(prependLeadingSlash(patterns)); - this.pathMatcher = (pathMatcher != null ? pathMatcher : new ParsingPathMatcher()); - this.useSuffixPatternMatch = useSuffixPatternMatch; - this.useTrailingSlashMatch = useTrailingSlashMatch; - if (fileExtensions != null) { - for (String fileExtension : fileExtensions) { - if (fileExtension.charAt(0) != '.') { - fileExtension = "." + fileExtension; - } - this.fileExtensions.add(fileExtension); + private PatternsRequestCondition(Collection 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 constructor accepting a list of path patterns. + */ + private PatternsRequestCondition(List patterns, PathPatternParser patternParser) { + this.patterns = patterns; + this.parser = patternParser; } @@ -103,26 +95,17 @@ public final class PatternsRequestCondition extends AbstractRequestCondition prependLeadingSlash(Collection patterns) { - if (patterns == null) { - return Collections.emptySet(); - } - Set result = new LinkedHashSet<>(patterns.size()); - for (String pattern : patterns) { - if (StringUtils.hasLength(pattern) && !pattern.startsWith("/")) { - pattern = "/" + pattern; - } - result.add(pattern); - } - return result; + public Set getPatterns() { + return new TreeSet<>(this.patterns); } - public Set getPatterns() { - return this.patterns; + public Set getPatternStrings() { + return this.patterns.stream() + .map(PathPattern::toString).collect(Collectors.toSet()); } @Override - protected Collection getContent() { + protected Collection getContent() { return this.patterns; } @@ -136,62 +119,54 @@ public final class PatternsRequestCondition extends AbstractRequestCondition *

  • If there are patterns in both instances, combine the patterns in "this" with - * the patterns in "other" using {@link PathMatcher#combine(String, String)}. + * the patterns in "other" using {@link PathPattern#combine(String)}. *
  • If only one instance has patterns, use them. *
  • If neither instance has patterns, use an empty String (i.e. ""). * */ @Override public PatternsRequestCondition combine(PatternsRequestCondition other) { - Set result = new LinkedHashSet<>(); + List combined = new ArrayList<>(); if (!this.patterns.isEmpty() && !other.patterns.isEmpty()) { - for (String pattern1 : this.patterns) { - for (String pattern2 : other.patterns) { - result.add(this.pathMatcher.combine(pattern1, pattern2)); + for (PathPattern pattern1 : this.patterns) { + for (PathPattern pattern2 : other.patterns) { + String combinedPattern = pattern1.combine(pattern2.getPatternString()); + combined.add(this.parser.parse(combinedPattern)); } } } else if (!this.patterns.isEmpty()) { - result.addAll(this.patterns); + combined.addAll(this.patterns); } else if (!other.patterns.isEmpty()) { - result.addAll(other.patterns); + combined.addAll(other.patterns); } else { - result.add(""); + combined.add(this.parser.parse("")); } - return new PatternsRequestCondition(result, this.pathMatcher, this.useSuffixPatternMatch, - this.useTrailingSlashMatch, this.fileExtensions); + + return new PatternsRequestCondition(combined, this.parser); } /** * Checks if any of the patterns match the given request and returns an instance - * that is guaranteed to contain matching patterns, sorted via - * {@link PathMatcher#getPatternComparator(String)}. - *

    A matching pattern is obtained by making checks in the following order: - *

      - *
    • Direct match - *
    • Pattern match with ".*" appended if the pattern doesn't already contain a "." - *
    • Pattern match - *
    • Pattern match with "/" appended if the pattern doesn't already end in "/" - *
    + * that is guaranteed to contain matching patterns, sorted with a + * {@link PathPatternComparator}. * @param exchange the current exchange * @return the same instance if the condition contains no patterns; * or a new condition with sorted matching patterns; * or {@code null} if no patterns match. */ @Override + @Nullable public PatternsRequestCondition getMatchingCondition(ServerWebExchange exchange) { if (this.patterns.isEmpty()) { return this; } String lookupPath = exchange.getRequest().getPath().pathWithinApplication().value(); - List matches = getMatchingPatterns(lookupPath); - - return matches.isEmpty() ? null : - new PatternsRequestCondition(matches, this.pathMatcher, this.useSuffixPatternMatch, - this.useTrailingSlashMatch, this.fileExtensions); + SortedSet matches = getMatchingPatterns(lookupPath); + return matches.isEmpty() ? null : new PatternsRequestCondition(new ArrayList<>(matches), this.parser); } /** @@ -201,39 +176,19 @@ public final class PatternsRequestCondition extends AbstractRequestCondition getMatchingPatterns(String lookupPath) { - List matches = new ArrayList<>(); - for (String pattern : this.patterns) { - String match = getMatchingPattern(pattern, lookupPath); - if (match != null) { - matches.add(match); - } - } - Collections.sort(matches, this.pathMatcher.getPatternComparator(lookupPath)); - return matches; - } - - private String getMatchingPattern(String pattern, String lookupPath) { - if (pattern.equals(lookupPath)) { - return pattern; - } - if (this.pathMatcher.match(pattern, lookupPath)) { - return pattern; - } - if (this.useTrailingSlashMatch) { - if (!pattern.endsWith("/") && this.pathMatcher.match(pattern + "/", lookupPath)) { - return pattern +"/"; - } - } - return null; + public SortedSet getMatchingPatterns(String lookupPath) { + return patterns.stream() + .filter(pattern -> pattern.matches(lookupPath)) + .collect(Collectors.toCollection(() -> + new TreeSet<>(new PathPatternComparator(lookupPath)))); } /** * Compare the two conditions based on the URL patterns they contain. * Patterns are compared one at a time, from top to bottom via - * {@link PathMatcher#getPatternComparator(String)}. If all compared + * {@link PathPatternComparator}. 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 @@ -244,11 +199,13 @@ public final class PatternsRequestCondition extends AbstractRequestCondition patternComparator = this.pathMatcher.getPatternComparator(lookupPath); - Iterator iterator = this.patterns.iterator(); - Iterator iteratorOther = other.patterns.iterator(); + PathPatternComparator comparator = new PathPatternComparator(lookupPath); + Iterator iterator = this.patterns.stream() + .sorted(comparator).collect(Collectors.toList()).iterator(); + Iterator iteratorOther = other.getPatterns().stream() + .sorted(comparator).collect(Collectors.toList()).iterator(); while (iterator.hasNext() && iteratorOther.hasNext()) { - int result = patternComparator.compare(iterator.next(), iteratorOther.next()); + int result = comparator.compare(iterator.next(), 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 829edf94dd..70314a347c 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 @@ -37,8 +37,6 @@ import org.springframework.core.MethodIntrospector; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; -import org.springframework.util.LinkedMultiValueMap; -import org.springframework.util.MultiValueMap; import org.springframework.web.cors.CorsConfiguration; import org.springframework.web.cors.reactive.CorsUtils; import org.springframework.web.method.HandlerMethod; @@ -294,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, String, ServerWebExchange) * @see #handleNoMatch(Set, String, ServerWebExchange) */ @Nullable @@ -302,14 +300,7 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap throws Exception { List matches = new ArrayList<>(); - List directPathMatches = this.mappingRegistry.getMappingsByUrl(lookupPath); - if (directPathMatches != null) { - addMatchingMappings(directPathMatches, matches, exchange); - } - if (matches.isEmpty()) { - // No choice but to go through all mappings... - addMatchingMappings(this.mappingRegistry.getMappings().keySet(), matches, exchange); - } + addMatchingMappings(this.mappingRegistry.getMappings().keySet(), matches, exchange); if (!matches.isEmpty()) { Comparator comparator = new MatchComparator(getMappingComparator(exchange)); @@ -443,8 +434,6 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap private final Map mappingLookup = new LinkedHashMap<>(); - private final MultiValueMap urlLookup = new LinkedMultiValueMap<>(); - private final Map corsLookup = new ConcurrentHashMap<>(); private final ReentrantReadWriteLock readWriteLock = new ReentrantReadWriteLock(); @@ -457,15 +446,6 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap return this.mappingLookup; } - /** - * Return matches for the given URL path. Not thread-safe. - * @see #acquireReadLock() - */ - @Nullable - public List getMappingsByUrl(String urlPath) { - return this.urlLookup.get(urlPath); - } - /** * Return CORS configuration. Thread-safe for concurrent use. */ @@ -499,17 +479,12 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap } this.mappingLookup.put(mapping, handlerMethod); - List directUrls = getDirectUrls(mapping); - for (String url : directUrls) { - this.urlLookup.add(url, mapping); - } - CorsConfiguration corsConfig = initCorsConfiguration(handler, method, mapping); if (corsConfig != null) { this.corsLookup.put(handlerMethod, corsConfig); } - this.registry.put(mapping, new MappingRegistration<>(mapping, handlerMethod, directUrls)); + this.registry.put(mapping, new MappingRegistration<>(mapping, handlerMethod)); } finally { this.readWriteLock.writeLock().unlock(); @@ -520,22 +495,12 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap HandlerMethod handlerMethod = this.mappingLookup.get(mapping); if (handlerMethod != null && !handlerMethod.equals(newHandlerMethod)) { throw new IllegalStateException( - "Ambiguous mapping. Cannot map '" + newHandlerMethod.getBean() + "' method \n" + - newHandlerMethod + "\nto " + mapping + ": There is already '" + - handlerMethod.getBean() + "' bean method\n" + handlerMethod + " mapped."); + "Ambiguous mapping. Cannot map '" + newHandlerMethod.getBean() + "' method \n" + + newHandlerMethod + "\nto " + mapping + ": There is already '" + + handlerMethod.getBean() + "' bean method\n" + handlerMethod + " mapped."); } } - private List getDirectUrls(T mapping) { - List urls = new ArrayList<>(1); - for (String path : getMappingPathPatterns(mapping)) { - if (!getPathMatcher().isPattern(path)) { - urls.add(path); - } - } - return urls; - } - public void unregister(T mapping) { this.readWriteLock.writeLock().lock(); try { @@ -545,16 +510,6 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap } this.mappingLookup.remove(definition.getMapping()); - - for (String url : definition.getDirectUrls()) { - List list = this.urlLookup.get(url); - if (list != null) { - list.remove(definition.getMapping()); - if (list.isEmpty()) { - this.urlLookup.remove(url); - } - } - } this.corsLookup.remove(definition.getHandlerMethod()); } finally { @@ -570,14 +525,11 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap private final HandlerMethod handlerMethod; - private final List directUrls; - - public MappingRegistration(T mapping, HandlerMethod handlerMethod, @Nullable List directUrls) { + public MappingRegistration(T mapping, HandlerMethod handlerMethod) { Assert.notNull(mapping, "Mapping must not be null"); Assert.notNull(handlerMethod, "HandlerMethod must not be null"); this.mapping = mapping; this.handlerMethod = handlerMethod; - this.directUrls = (directUrls != null ? directUrls : Collections.emptyList()); } public T getMapping() { @@ -588,9 +540,6 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap return this.handlerMethod; } - public List getDirectUrls() { - return this.directUrls; - } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfo.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfo.java index ca31429113..71de03b2a0 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfo.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfo.java @@ -16,10 +16,7 @@ package org.springframework.web.reactive.result.method; -import java.util.Set; - import org.springframework.lang.Nullable; -import org.springframework.util.PathMatcher; import org.springframework.util.StringUtils; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.reactive.accept.RequestedContentTypeResolver; @@ -32,6 +29,7 @@ import org.springframework.web.reactive.result.condition.RequestCondition; import org.springframework.web.reactive.result.condition.RequestConditionHolder; import org.springframework.web.reactive.result.condition.RequestMethodsRequestCondition; import org.springframework.web.server.ServerWebExchange; +import org.springframework.web.util.pattern.PathPatternParser; /** * Encapsulates the following request mapping conditions: @@ -473,8 +471,7 @@ public final class RequestMappingInfo implements RequestConditionBy default this is not set. - */ - public void setPathMatcher(PathMatcher pathMatcher) { - this.pathMatcher = pathMatcher; - } - - public PathMatcher getPathMatcher() { - return this.pathMatcher; - } - - /** - * Whether to apply trailing slash matching in PatternsRequestCondition. - *

    By default this is set to 'true'. - */ - public void setTrailingSlashMatch(boolean trailingSlashMatch) { - this.trailingSlashMatch = trailingSlashMatch; - } - - public boolean useTrailingSlashMatch() { - return this.trailingSlashMatch; - } - - /** - * Whether to apply suffix pattern matching in PatternsRequestCondition. - *

    By default this is set to 'true'. - * @see #setRegisteredSuffixPatternMatch(boolean) - */ - public void setSuffixPatternMatch(boolean suffixPatternMatch) { - this.suffixPatternMatch = suffixPatternMatch; - } - - public boolean useSuffixPatternMatch() { - return this.suffixPatternMatch; - } - - /** - * Whether suffix pattern matching should be restricted to registered - * file extensions only. Setting this property also sets - * suffixPatternMatch=true and requires that a - * {@link #setContentTypeResolver} is also configured in order to - * obtain the registered file extensions. - */ - public void setRegisteredSuffixPatternMatch(boolean registeredSuffixPatternMatch) { - this.registeredSuffixPatternMatch = registeredSuffixPatternMatch; - this.suffixPatternMatch = (registeredSuffixPatternMatch || this.suffixPatternMatch); - } - - public boolean useRegisteredSuffixPatternMatch() { - return this.registeredSuffixPatternMatch; + @Nullable + public PathPatternParser getPatternParser() { + return this.patternParser; } - /** - * Return the file extensions to use for suffix pattern matching. If - * {@code registeredSuffixPatternMatch=true}, the extensions are obtained - * from the configured {@code contentTypeResolver}. - */ - @Nullable - public Set getFileExtensions() { - return null; + public void setPatternParser(PathPatternParser patternParser) { + this.patternParser = patternParser; } /** 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 d971a00c25..d98081058e 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 @@ -47,6 +47,7 @@ import org.springframework.web.server.NotAcceptableStatusException; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.ServerWebInputException; import org.springframework.web.server.UnsupportedMediaTypeStatusException; +import org.springframework.web.util.pattern.PathPattern; /** @@ -76,7 +77,7 @@ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMe */ @Override protected Set getMappingPathPatterns(RequestMappingInfo info) { - return info.getPatternsCondition().getPatterns(); + return info.getPatternsCondition().getPatternStrings(); } /** @@ -108,17 +109,17 @@ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMe protected void handleMatch(RequestMappingInfo info, String lookupPath, ServerWebExchange exchange) { super.handleMatch(info, lookupPath, exchange); - String bestPattern; + PathPattern bestPattern; Map uriVariables; - Set patterns = info.getPatternsCondition().getPatterns(); + Set patterns = info.getPatternsCondition().getPatterns(); if (patterns.isEmpty()) { - bestPattern = lookupPath; + bestPattern = getPathPatternParser().parse(lookupPath); uriVariables = Collections.emptyMap(); } else { bestPattern = patterns.iterator().next(); - uriVariables = getPathMatcher().extractUriTemplateVariables(bestPattern, lookupPath); + uriVariables = bestPattern.matchAndExtract(lookupPath); } // Let URI vars be stripped of semicolon content.. diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMapping.java index 880cdd3c60..f71eb3c3fa 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMapping.java @@ -18,7 +18,6 @@ package org.springframework.web.reactive.result.method.annotation; import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Method; -import java.util.Set; import org.springframework.context.EmbeddedValueResolverAware; import org.springframework.core.annotation.AnnotatedElementUtils; @@ -49,12 +48,6 @@ import org.springframework.web.reactive.result.method.RequestMappingInfoHandlerM public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMapping implements EmbeddedValueResolverAware { - private boolean useSuffixPatternMatch = true; - - private boolean useRegisteredSuffixPatternMatch = true; - - private boolean useTrailingSlashMatch = true; - private RequestedContentTypeResolver contentTypeResolver = new RequestedContentTypeResolverBuilder().build(); private StringValueResolver embeddedValueResolver; @@ -62,40 +55,6 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi private RequestMappingInfo.BuilderConfiguration config = new RequestMappingInfo.BuilderConfiguration(); - /** - * Whether to use suffix pattern matching. If enabled a method mapped to - * "/path" also matches to "/path.*". - *

    The default value is {@code true}. - *

    Note: when using suffix pattern matching it's usually - * preferable to be explicit about what is and isn't an extension so rather - * than setting this property consider using - * {@link #setUseRegisteredSuffixPatternMatch} instead. - */ - public void setUseSuffixPatternMatch(boolean useSuffixPatternMatch) { - this.useSuffixPatternMatch = useSuffixPatternMatch; - } - - /** - * Whether suffix pattern matching should work only against path extensions - * explicitly registered with the configured {@link RequestedContentTypeResolver}. This - * is generally recommended to reduce ambiguity and to avoid issues such as - * when a "." appears in the path for other reasons. - *

    By default this is set to "true". - */ - public void setUseRegisteredSuffixPatternMatch(boolean useRegisteredSuffixPatternMatch) { - this.useRegisteredSuffixPatternMatch = useRegisteredSuffixPatternMatch; - this.useSuffixPatternMatch = (useRegisteredSuffixPatternMatch || this.useSuffixPatternMatch); - } - - /** - * Whether to match to URLs irrespective of the presence of a trailing slash. - * If enabled a method mapped to "/users" also matches to "/users/". - *

    The default value is {@code true}. - */ - public void setUseTrailingSlashMatch(boolean useTrailingSlashMatch) { - this.useTrailingSlashMatch = useTrailingSlashMatch; - } - /** * Set the {@link RequestedContentTypeResolver} to use to determine requested media types. * If not set, the default constructor is used. @@ -113,36 +72,13 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi @Override public void afterPropertiesSet() { this.config = new RequestMappingInfo.BuilderConfiguration(); - this.config.setPathMatcher(getPathMatcher()); - this.config.setSuffixPatternMatch(this.useSuffixPatternMatch); - this.config.setRegisteredSuffixPatternMatch(this.useRegisteredSuffixPatternMatch); + this.config.setPatternParser(getPathPatternParser()); this.config.setContentTypeResolver(getContentTypeResolver()); super.afterPropertiesSet(); } - /** - * Whether to use suffix pattern matching. - */ - public boolean useSuffixPatternMatch() { - return this.useSuffixPatternMatch; - } - - /** - * Whether to use registered suffixes for pattern matching. - */ - public boolean useRegisteredSuffixPatternMatch() { - return this.useRegisteredSuffixPatternMatch; - } - - /** - * Whether to match to URLs irrespective of the presence of a trailing slash. - */ - public boolean useTrailingSlashMatch() { - return this.useTrailingSlashMatch; - } - /** * Return the configured {@link RequestedContentTypeResolver}. */ @@ -150,14 +86,6 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi return this.contentTypeResolver; } - /** - * Return the file extensions to use for suffix pattern matching. - */ - @Nullable - public Set getFileExtensions() { - return this.config.getFileExtensions(); - } - /** * {@inheritDoc} * Expects a handler to have a type-level @{@link Controller} annotation. diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/config/PathMatchConfigurerTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/config/PathMatchConfigurerTests.java deleted file mode 100644 index 4b8fa4f7f0..0000000000 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/config/PathMatchConfigurerTests.java +++ /dev/null @@ -1,49 +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.reactive.config; - -import org.hamcrest.Matchers; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; - -import org.springframework.web.util.pattern.ParsingPathMatcher; - -/** - * Unit tests for {@link PathMatchConfigurer} - * @author Brian Clozel - */ -public class PathMatchConfigurerTests { - - @Rule - public ExpectedException thrown = ExpectedException.none(); - - // SPR-15303 - @Test - public void illegalConfigurationParsingPathMatcher() { - PathMatchConfigurer configurer = new PathMatchConfigurer(); - configurer.setPathMatcher(new ParsingPathMatcher()); - configurer.setUseSuffixPatternMatch(true); - configurer.setUseTrailingSlashMatch(true); - - this.thrown.expect(IllegalStateException.class); - this.thrown.expectMessage(Matchers.containsString("useSuffixPatternMatch")); - this.thrown.expectMessage(Matchers.containsString("useTrailingSlashMatch")); - - configurer.getPathMatcher(); - } -} diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/config/WebFluxConfigurationSupportTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/config/WebFluxConfigurationSupportTests.java index 563f005743..cb09196d46 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/config/WebFluxConfigurationSupportTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/config/WebFluxConfigurationSupportTests.java @@ -16,9 +16,11 @@ package org.springframework.web.reactive.config; +import java.lang.reflect.Field; import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; + import javax.xml.bind.annotation.XmlRootElement; import org.junit.Test; @@ -43,11 +45,12 @@ import org.springframework.http.codec.xml.Jaxb2XmlEncoder; import org.springframework.util.MimeType; import org.springframework.util.MimeTypeUtils; import org.springframework.util.MultiValueMap; +import org.springframework.util.ReflectionUtils; import org.springframework.validation.Validator; import org.springframework.web.bind.support.WebBindingInitializer; import org.springframework.web.bind.support.WebExchangeDataBinder; import org.springframework.web.reactive.accept.RequestedContentTypeResolver; -import org.springframework.web.reactive.handler.AbstractHandlerMapping; +import org.springframework.web.reactive.handler.AbstractUrlHandlerMapping; import org.springframework.web.reactive.handler.SimpleUrlHandlerMapping; import org.springframework.web.reactive.result.method.annotation.RequestMappingHandlerAdapter; import org.springframework.web.reactive.result.method.annotation.RequestMappingHandlerMapping; @@ -61,6 +64,7 @@ import org.springframework.web.reactive.result.view.freemarker.FreeMarkerConfigu import org.springframework.web.reactive.result.view.freemarker.FreeMarkerViewResolver; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebHandler; +import org.springframework.web.util.pattern.PathPatternParser; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -87,6 +91,8 @@ public class WebFluxConfigurationSupportTests { @Test public void requestMappingHandlerMapping() throws Exception { ApplicationContext context = loadConfig(WebFluxConfig.class); + final Field trailingSlashField = ReflectionUtils.findField(PathPatternParser.class, "matchOptionalTrailingSlash"); + ReflectionUtils.makeAccessible(trailingSlashField); String name = "requestMappingHandlerMapping"; RequestMappingHandlerMapping mapping = context.getBean(name, RequestMappingHandlerMapping.class); @@ -94,9 +100,10 @@ public class WebFluxConfigurationSupportTests { assertEquals(0, mapping.getOrder()); - assertTrue(mapping.useSuffixPatternMatch()); - assertTrue(mapping.useTrailingSlashMatch()); - assertTrue(mapping.useRegisteredSuffixPatternMatch()); + assertNotNull(mapping.getPathPatternParser()); + boolean matchOptionalTrailingSlash = (boolean) ReflectionUtils + .getField(trailingSlashField, mapping.getPathPatternParser()); + assertTrue(matchOptionalTrailingSlash); name = "webFluxContentTypeResolver"; RequestedContentTypeResolver resolver = context.getBean(name, RequestedContentTypeResolver.class); @@ -109,13 +116,17 @@ public class WebFluxConfigurationSupportTests { @Test public void customPathMatchConfig() throws Exception { ApplicationContext context = loadConfig(CustomPatchMatchConfig.class); + final Field trailingSlashField = ReflectionUtils.findField(PathPatternParser.class, "matchOptionalTrailingSlash"); + ReflectionUtils.makeAccessible(trailingSlashField); String name = "requestMappingHandlerMapping"; RequestMappingHandlerMapping mapping = context.getBean(name, RequestMappingHandlerMapping.class); assertNotNull(mapping); + assertNotNull(mapping.getPathPatternParser()); - assertFalse(mapping.useSuffixPatternMatch()); - assertFalse(mapping.useTrailingSlashMatch()); + boolean matchOptionalTrailingSlash = (boolean) ReflectionUtils + .getField(trailingSlashField, mapping.getPathPatternParser()); + assertFalse(matchOptionalTrailingSlash); } @Test @@ -245,12 +256,12 @@ public class WebFluxConfigurationSupportTests { ApplicationContext context = loadConfig(CustomResourceHandlingConfig.class); String name = "resourceHandlerMapping"; - AbstractHandlerMapping handlerMapping = context.getBean(name, AbstractHandlerMapping.class); + AbstractUrlHandlerMapping handlerMapping = context.getBean(name, AbstractUrlHandlerMapping.class); assertNotNull(handlerMapping); assertEquals(Ordered.LOWEST_PRECEDENCE - 1, handlerMapping.getOrder()); - assertNotNull(handlerMapping.getPathMatcher()); + assertNotNull(handlerMapping.getPatternRegistry()); SimpleUrlHandlerMapping urlHandlerMapping = (SimpleUrlHandlerMapping) handlerMapping; WebHandler webHandler = (WebHandler) urlHandlerMapping.getUrlMap().get("/images/**"); @@ -284,7 +295,6 @@ public class WebFluxConfigurationSupportTests { @Override public void configurePathMatching(PathMatchConfigurer configurer) { - configurer.setUseSuffixPatternMatch(false); configurer.setUseTrailingSlashMatch(false); } } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/handler/CorsUrlHandlerMappingTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/handler/CorsUrlHandlerMappingTests.java index daac2eface..ddfa3e9add 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/handler/CorsUrlHandlerMappingTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/handler/CorsUrlHandlerMappingTests.java @@ -51,7 +51,6 @@ public class CorsUrlHandlerMappingTests { @Before public void setup() { this.handlerMapping = new AbstractUrlHandlerMapping() {}; - this.handlerMapping.setUseTrailingSlashMatch(true); this.handlerMapping.registerHandler("/welcome.html", this.welcomeController); this.handlerMapping.registerHandler("/cors.html", this.corsController); } 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 new file mode 100644 index 0000000000..3f8c69e418 --- /dev/null +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/handler/PathPatternRegistryTests.java @@ -0,0 +1,134 @@ +/* + * 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.reactive.handler; + +import java.util.Collection; +import java.util.List; +import java.util.Set; +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; + +import org.springframework.web.util.pattern.PathPattern; +import org.springframework.web.util.pattern.PathPatternParser; +import org.springframework.web.util.pattern.PatternParseException; + +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; + +/** + * Tests for {@link PathPatternRegistry} + * + * @author Brian Clozel + */ +public class PathPatternRegistryTests { + + private PathPatternRegistry registry; + + 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() { + this.registry.register("foo/bar", new Object()); + assertThat(this.registry.getPatternsMap().keySet(), contains(pattern("/foo/bar"))); + } + + @Test + public void shouldNotRegisterInvalidPatterns() { + this.thrown.expect(PatternParseException.class); + this.thrown.expectMessage(Matchers.containsString("Expected close capture character after variable name")); + this.registry.register("/{invalid", new Object()); + } + + @Test + public void registerPatternsWithSameSpecificity() { + PathPattern fooOne = this.parser.parse("/fo?"); + PathPattern fooTwo = this.parser.parse("/f?o"); + assertThat(fooOne.compareTo(fooTwo), is(0)); + + 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?"))); + } + + @Test + public void findNoMatch() { + this.registry.register("/foo/{bar}", new Object()); + assertThat(this.registry.findMatches("/other"), hasSize(0)); + } + + @Test + public void orderMatchesBySpecificity() { + this.registry.register("/foo/{*baz}", new Object()); + 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}"), + pattern("/foo/{*baz}"))); + } + + + private List getPatternList(Collection> results) { + return results.stream() + .map(result -> result.getPattern()).collect(Collectors.toList()); + + } + + private static PathPatternMatcher pattern(String pattern) { + return new PathPatternMatcher(pattern); + } + + private static class PathPatternMatcher extends BaseMatcher { + + private final String pattern; + + public PathPatternMatcher(String pattern) { + this.pattern = pattern; + } + + @Override + public boolean matches(Object item) { + if(item != null && item instanceof PathPattern) { + return ((PathPattern) item).getPatternString().equals(pattern); + } + return false; + } + + @Override + public void describeTo(Description description) { + + } + } + +} \ No newline at end of file diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/handler/SimpleUrlHandlerMappingTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/handler/SimpleUrlHandlerMappingTests.java index 6f2cead1ab..ef37f433cd 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/handler/SimpleUrlHandlerMappingTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/handler/SimpleUrlHandlerMappingTests.java @@ -51,11 +51,11 @@ public class SimpleUrlHandlerMappingTests { Object mainController = wac.getBean("mainController"); Object otherController = wac.getBean("otherController"); - testUrl("/welcome.html", mainController, handlerMapping, "/welcome.html"); + testUrl("/welcome.html", mainController, handlerMapping, ""); testUrl("/welcome.x", otherController, handlerMapping, "welcome.x"); testUrl("/welcome/", otherController, handlerMapping, "welcome"); - testUrl("/show.html", mainController, handlerMapping, "/show.html"); - testUrl("/bookseats.html", mainController, handlerMapping, "/bookseats.html"); + testUrl("/show.html", mainController, handlerMapping, ""); + testUrl("/bookseats.html", mainController, handlerMapping, ""); } @Test @@ -70,10 +70,10 @@ public class SimpleUrlHandlerMappingTests { testUrl("welcome.html", null, handlerMapping, null); testUrl("/pathmatchingAA.html", mainController, handlerMapping, "pathmatchingAA.html"); testUrl("/pathmatchingA.html", null, handlerMapping, null); - testUrl("/administrator/pathmatching.html", mainController, handlerMapping, "/administrator/pathmatching.html"); + testUrl("/administrator/pathmatching.html", mainController, handlerMapping, ""); testUrl("/administrator/test/pathmatching.html", mainController, handlerMapping, "test/pathmatching.html"); testUrl("/administratort/pathmatching.html", null, handlerMapping, null); - testUrl("/administrator/another/bla.xml", mainController, handlerMapping, "/administrator/another/bla.xml"); + testUrl("/administrator/another/bla.xml", mainController, handlerMapping, ""); testUrl("/administrator/another/bla.gif", null, handlerMapping, null); testUrl("/administrator/test/testlastbit", mainController, handlerMapping, "test/testlastbit"); testUrl("/administrator/test/testla", null, handlerMapping, null); @@ -85,7 +85,7 @@ public class SimpleUrlHandlerMappingTests { testUrl("/XpathXXmatching.html", null, handlerMapping, null); testUrl("/XXpathmatching.html", null, handlerMapping, null); testUrl("/show12.html", mainController, handlerMapping, "show12.html"); - testUrl("/show123.html", mainController, handlerMapping, "/show123.html"); + testUrl("/show123.html", mainController, handlerMapping, ""); testUrl("/show1.html", mainController, handlerMapping, "show1.html"); testUrl("/reallyGood-test-is-this.jpeg", mainController, handlerMapping, "reallyGood-test-is-this.jpeg"); testUrl("/reallyGood-tst-is-this.jpeg", null, handlerMapping, null); @@ -117,7 +117,6 @@ public class SimpleUrlHandlerMappingTests { @Bean @SuppressWarnings("unused") public SimpleUrlHandlerMapping handlerMapping() { SimpleUrlHandlerMapping hm = new SimpleUrlHandlerMapping(); - hm.setUseTrailingSlashMatch(true); hm.registerHandler("/welcome*", otherController()); hm.registerHandler("/welcome.html", mainController()); hm.registerHandler("/show.html", mainController()); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceUrlProviderTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceUrlProviderTests.java index 6698c95410..a24895a8fe 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceUrlProviderTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceUrlProviderTests.java @@ -22,6 +22,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import org.hamcrest.BaseMatcher; +import org.hamcrest.Description; import org.hamcrest.Matchers; import org.junit.Before; import org.junit.Test; @@ -35,6 +37,7 @@ import org.springframework.mock.web.test.MockServletContext; import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; import org.springframework.web.reactive.handler.SimpleUrlHandlerMapping; import org.springframework.web.server.ServerWebExchange; +import org.springframework.web.util.pattern.PathPattern; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -132,7 +135,7 @@ public class ResourceUrlProviderTests { context.refresh(); ResourceUrlProvider urlProviderBean = context.getBean(ResourceUrlProvider.class); - assertThat(urlProviderBean.getHandlerMap(), Matchers.hasKey("/resources/**")); + assertThat(urlProviderBean.getHandlerMap(), Matchers.hasKey(pattern("/resources/**"))); assertFalse(urlProviderBean.isAutodetect()); } @@ -157,4 +160,30 @@ public class ResourceUrlProviderTests { } } + private static PathPatternMatcher pattern(String pattern) { + return new PathPatternMatcher(pattern); + } + + private static class PathPatternMatcher extends BaseMatcher { + + private final String pattern; + + public PathPatternMatcher(String pattern) { + this.pattern = pattern; + } + + @Override + public boolean matches(Object item) { + if (item != null && item instanceof PathPattern) { + return ((PathPattern) item).getPatternString().equals(pattern); + } + return false; + } + + @Override + public void describeTo(Description description) { + + } + } + } 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 8ca1c96fea..26b878659c 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 @@ -20,6 +20,7 @@ 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.PathPatternParser; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -36,13 +37,14 @@ public class PatternsRequestConditionTests { @Test public void prependSlash() { PatternsRequestCondition c = new PatternsRequestCondition("foo"); - assertEquals("/foo", c.getPatterns().iterator().next()); + assertEquals("/foo", c.getPatterns().iterator().next().getPatternString()); } @Test public void prependNonEmptyPatternsOnly() { PatternsRequestCondition c = new PatternsRequestCondition(""); - assertEquals("Do not prepend empty patterns (SPR-8255)", "", c.getPatterns().iterator().next()); + assertEquals("Do not prepend empty patterns (SPR-8255)", "", + c.getPatterns().iterator().next().getPatternString()); } @Test @@ -107,16 +109,19 @@ public class PatternsRequestConditionTests { PatternsRequestCondition match = condition.getMatchingCondition(exchange); assertNotNull(match); - assertEquals("Should match by default", "/foo/", match.getPatterns().iterator().next()); + assertEquals("Should match by default", "/foo", + match.getPatterns().iterator().next().getPatternString()); - condition = new PatternsRequestCondition(new String[] {"/foo"}, null, false, true, null); + condition = new PatternsRequestCondition(new String[] {"/foo"}, null); match = condition.getMatchingCondition(exchange); assertNotNull(match); assertEquals("Trailing slash should be insensitive to useSuffixPatternMatch settings (SPR-6164, SPR-5636)", - "/foo/", match.getPatterns().iterator().next()); + "/foo", match.getPatterns().iterator().next().getPatternString()); - condition = new PatternsRequestCondition(new String[] {"/foo"}, null, false, false, null); + PathPatternParser parser = new PathPatternParser(); + parser.setMatchOptionalTrailingSlash(false); + condition = new PatternsRequestCondition(new String[] {"/foo"}, parser); match = condition.getMatchingCondition(get("/foo/").toExchange()); assertNull(match); 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 e3d981adb9..e8666f5793 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 @@ -22,6 +22,7 @@ import java.util.Comparator; import java.util.List; import java.util.Set; +import org.hamcrest.Matchers; import org.junit.Before; import org.junit.Test; import reactor.core.publisher.Mono; @@ -102,11 +103,8 @@ public class HandlerMethodMappingTests { this.mapping.registerMapping(key1, this.handler, this.method1); this.mapping.registerMapping(key2, this.handler, this.method2); - List directUrlMatches = this.mapping.getMappingRegistry().getMappingsByUrl(key1); - - assertNotNull(directUrlMatches); - assertEquals(1, directUrlMatches.size()); - assertEquals(key1, directUrlMatches.get(0)); + assertThat(this.mapping.getMappingRegistry().getMappings().keySet(), + Matchers.contains(key1, key2)); } @Test @@ -118,11 +116,7 @@ public class HandlerMethodMappingTests { this.mapping.registerMapping(key1, handler1, this.method1); this.mapping.registerMapping(key2, handler2, this.method1); - List directUrlMatches = this.mapping.getMappingRegistry().getMappingsByUrl(key1); - - assertNotNull(directUrlMatches); - assertEquals(1, directUrlMatches.size()); - assertEquals(key1, directUrlMatches.get(0)); + assertThat(this.mapping.getMappingRegistry().getMappings().keySet(), Matchers.contains(key1, key2)); } @Test @@ -137,7 +131,7 @@ public class HandlerMethodMappingTests { result = this.mapping.getHandler(MockServerHttpRequest.get(key).toExchange()); assertNull(result.block()); - assertNull(this.mapping.getMappingRegistry().getMappingsByUrl(key)); + assertThat(this.mapping.getMappingRegistry().getMappings().keySet(), Matchers.not(Matchers.contains(key))); } 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 e42b4e365f..b0b7a788d5 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 @@ -27,6 +27,7 @@ import java.util.Set; import java.util.function.Consumer; import org.junit.Before; +import org.junit.Ignore; import org.junit.Test; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; @@ -53,6 +54,7 @@ import org.springframework.web.server.NotAcceptableStatusException; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.ServerWebInputException; import org.springframework.web.server.UnsupportedMediaTypeStatusException; +import org.springframework.web.util.pattern.PathPattern; import static org.hamcrest.CoreMatchers.containsString; import static org.junit.Assert.assertEquals; @@ -113,6 +115,7 @@ public class RequestMappingInfoHandlerMappingTests { } @Test + @Ignore public void getHandlerEmptyPathMatch() throws Exception { Method expected = on(TestController.class).annot(requestMapping("")).resolveMethod(); ServerWebExchange exchange = get("").toExchange(); @@ -251,7 +254,9 @@ public class RequestMappingInfoHandlerMappingTests { String lookupPath = exchange.getRequest().getPath().pathWithinApplication().value(); this.handlerMapping.handleMatch(key, lookupPath, exchange); - assertEquals("/{path1}/2", exchange.getAttributes().get(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE)); + PathPattern bestMatch = (PathPattern) exchange.getAttributes() + .get(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE); + assertEquals("/{path1}/2", bestMatch.getPatternString()); } @Test @@ -261,7 +266,9 @@ public class RequestMappingInfoHandlerMappingTests { String lookupPath = exchange.getRequest().getPath().pathWithinApplication().value(); this.handlerMapping.handleMatch(key, lookupPath, exchange); - assertEquals("/1/2", exchange.getAttributes().get(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE)); + PathPattern bestMatch = (PathPattern) exchange.getAttributes() + .get(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE); + assertEquals("/1/2", bestMatch.getPatternString()); } @Test @@ -318,7 +325,7 @@ public class RequestMappingInfoHandlerMappingTests { @SuppressWarnings("unchecked") - private void assertError(Mono mono, final Class exceptionClass, final Consumer consumer) { + private void assertError(Mono mono, final Class exceptionClass, final Consumer consumer) { StepVerifier.create(mono) .consumeErrorWith(error -> { assertEquals(exceptionClass, error.getClass()); @@ -392,11 +399,11 @@ public class RequestMappingInfoHandlerMappingTests { public void foo() { } - @GetMapping(path = "/foo", params="p") + @GetMapping(path = "/foo", params = "p") public void fooParam() { } - @RequestMapping(path = "/ba*", method = { GET, HEAD }) + @RequestMapping(path = "/ba*", method = {GET, HEAD}) public void bar() { } @@ -404,31 +411,31 @@ public class RequestMappingInfoHandlerMappingTests { public void empty() { } - @PutMapping(path = "/person/{id}", consumes="application/xml") + @PutMapping(path = "/person/{id}", consumes = "application/xml") public void consumes(@RequestBody String text) { } - @RequestMapping(path = "/persons", produces="application/xml") + @RequestMapping(path = "/persons", produces = "application/xml") public String produces() { return ""; } - @RequestMapping(path = "/params", params="foo=bar") + @RequestMapping(path = "/params", params = "foo=bar") public String param() { return ""; } - @RequestMapping(path = "/params", params="bar=baz") + @RequestMapping(path = "/params", params = "bar=baz") public String param2() { return ""; } - @RequestMapping(path = "/content", produces="application/xml") + @RequestMapping(path = "/content", produces = "application/xml") public String xmlContent() { return ""; } - @RequestMapping(path = "/content", produces="!application/xml") + @RequestMapping(path = "/content", produces = "!application/xml") public String nonXmlContent() { return ""; } @@ -472,9 +479,7 @@ public class RequestMappingInfoHandlerMappingTests { RequestMapping annot = AnnotatedElementUtils.findMergedAnnotation(method, RequestMapping.class); if (annot != null) { BuilderConfiguration options = new BuilderConfiguration(); - options.setPathMatcher(getPathMatcher()); - options.setSuffixPatternMatch(true); - options.setTrailingSlashMatch(true); + options.setPatternParser(getPathPatternParser()); return paths(annot.value()).methods(annot.method()) .params(annot.params()).headers(annot.headers()) .consumes(annot.consumes()).produces(annot.produces()) 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 946519dcaa..6031451833 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 @@ -63,23 +63,6 @@ public class RequestMappingHandlerMappingTests { this.handlerMapping.setApplicationContext(wac); } - @Test - public void useSuffixPatternMatch() { - assertTrue(this.handlerMapping.useSuffixPatternMatch()); - assertTrue(this.handlerMapping.useRegisteredSuffixPatternMatch()); - - this.handlerMapping.setUseSuffixPatternMatch(false); - assertFalse(this.handlerMapping.useSuffixPatternMatch()); - - this.handlerMapping.setUseRegisteredSuffixPatternMatch(false); - assertFalse("'false' registeredSuffixPatternMatch shouldn't impact suffixPatternMatch", - this.handlerMapping.useSuffixPatternMatch()); - - this.handlerMapping.setUseRegisteredSuffixPatternMatch(true); - assertTrue("'true' registeredSuffixPatternMatch should enable suffixPatternMatch", - this.handlerMapping.useSuffixPatternMatch()); - } - @Test public void resolveEmbeddedValuesInPatterns() { this.handlerMapping.setEmbeddedValueResolver( @@ -152,7 +135,7 @@ public class RequestMappingHandlerMappingTests { assertNotNull(info); - Set paths = info.getPatternsCondition().getPatterns(); + Set paths = info.getPatternsCondition().getPatternStrings(); assertEquals(1, paths.size()); assertEquals(path, paths.iterator().next());