From fa4202f1bd2b1df5364c50723e99571df0db6605 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Fri, 10 Feb 2017 18:08:43 +0100 Subject: [PATCH] Revert Path Pattern changes in WebFlux Issue: SPR-14544 --- .../UrlBasedCorsConfigurationSource.java | 40 ++- .../util/patterns/PathPatternRegistry.java | 336 ------------------ .../UrlBasedCorsConfigurationSourceTests.java | 4 +- .../patterns/PathPatternRegistryTests.java | 177 --------- .../reactive/config/PathMatchConfigurer.java | 40 ++- .../config/WebFluxConfigurationSupport.java | 25 +- .../handler/AbstractHandlerMapping.java | 33 +- .../handler/AbstractUrlHandlerMapping.java | 96 +++-- .../resource/ResourceUrlProvider.java | 66 ++-- .../condition/PatternsRequestCondition.java | 188 ++++++++-- .../method/AbstractHandlerMethodMapping.java | 256 +++++++------ .../result/method/RequestMappingInfo.java | 87 +++-- .../RequestMappingInfoHandlerMapping.java | 21 +- .../RequestMappingHandlerMapping.java | 79 +++- .../WebFluxConfigurationSupportTests.java | 25 +- .../handler/SimpleUrlHandlerMappingTests.java | 15 +- .../CssLinkResourceTransformerTests.java | 1 - .../resource/ResourceUrlProviderTests.java | 8 +- .../PatternsRequestConditionTests.java | 59 +-- .../method/HandlerMethodMappingTests.java | 57 ++- ...RequestMappingInfoHandlerMappingTests.java | 51 +-- .../RequestMappingHandlerMappingTests.java | 77 +++- 22 files changed, 788 insertions(+), 953 deletions(-) delete mode 100644 spring-web/src/main/java/org/springframework/web/util/patterns/PathPatternRegistry.java delete mode 100644 spring-web/src/test/java/org/springframework/web/util/patterns/PathPatternRegistryTests.java diff --git a/spring-web/src/main/java/org/springframework/web/cors/reactive/UrlBasedCorsConfigurationSource.java b/spring-web/src/main/java/org/springframework/web/cors/reactive/UrlBasedCorsConfigurationSource.java index a7d9067e669..32a04851e37 100644 --- a/spring-web/src/main/java/org/springframework/web/cors/reactive/UrlBasedCorsConfigurationSource.java +++ b/spring-web/src/main/java/org/springframework/web/cors/reactive/UrlBasedCorsConfigurationSource.java @@ -18,16 +18,14 @@ package org.springframework.web.cors.reactive; import java.util.Collections; import java.util.LinkedHashMap; -import java.util.List; import java.util.Map; -import java.util.SortedSet; +import org.springframework.util.AntPathMatcher; import org.springframework.util.Assert; +import org.springframework.util.PathMatcher; import org.springframework.web.cors.CorsConfiguration; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.support.HttpRequestPathHelper; -import org.springframework.web.util.patterns.PathPattern; -import org.springframework.web.util.patterns.PathPatternRegistry; /** * Provide a per reactive request {@link CorsConfiguration} instance based on a @@ -37,18 +35,27 @@ import org.springframework.web.util.patterns.PathPatternRegistry; * as well as Ant-style path patterns (such as {@code "/admin/**"}). * * @author Sebastien Deleuze - * @author Brian Clozel * @since 5.0 */ public class UrlBasedCorsConfigurationSource implements CorsConfigurationSource { - private final PathPatternRegistry patternRegistry = new PathPatternRegistry(); + private final Map corsConfigurations = new LinkedHashMap<>(); - private final Map corsConfigurations = new LinkedHashMap<>(); + private PathMatcher pathMatcher = new AntPathMatcher(); private HttpRequestPathHelper pathHelper = new HttpRequestPathHelper(); + /** + * Set the PathMatcher implementation to use for matching URL paths + * against registered URL patterns. Default is AntPathMatcher. + * @see AntPathMatcher + */ + public void setPathMatcher(PathMatcher pathMatcher) { + Assert.notNull(pathMatcher, "PathMatcher must not be null"); + this.pathMatcher = pathMatcher; + } + /** * Set if context path and request URI should be URL-decoded. Both are returned * undecoded by the Servlet API, in contrast to the servlet path. @@ -73,20 +80,16 @@ public class UrlBasedCorsConfigurationSource implements CorsConfigurationSource * Set CORS configuration based on URL patterns. */ public void setCorsConfigurations(Map corsConfigurations) { - this.patternRegistry.clear(); this.corsConfigurations.clear(); if (corsConfigurations != null) { - corsConfigurations.forEach((pattern, config) -> { - List registered = this.patternRegistry.register(pattern); - registered.forEach(p -> this.corsConfigurations.put(p, config)); - }); + this.corsConfigurations.putAll(corsConfigurations); } } /** * Get the CORS configuration. */ - public Map getCorsConfigurations() { + public Map getCorsConfigurations() { return Collections.unmodifiableMap(this.corsConfigurations); } @@ -94,18 +97,17 @@ public class UrlBasedCorsConfigurationSource implements CorsConfigurationSource * Register a {@link CorsConfiguration} for the specified path pattern. */ public void registerCorsConfiguration(String path, CorsConfiguration config) { - this.patternRegistry - .register(path) - .forEach(pattern -> this.corsConfigurations.put(pattern, config)); + this.corsConfigurations.put(path, config); } @Override public CorsConfiguration getCorsConfiguration(ServerWebExchange exchange) { String lookupPath = this.pathHelper.getLookupPathForRequest(exchange); - SortedSet matches = this.patternRegistry.findMatches(lookupPath); - if(!matches.isEmpty()) { - return this.corsConfigurations.get(matches.first()); + for (Map.Entry entry : this.corsConfigurations.entrySet()) { + if (this.pathMatcher.match(entry.getKey(), lookupPath)) { + return entry.getValue(); + } } return null; } diff --git a/spring-web/src/main/java/org/springframework/web/util/patterns/PathPatternRegistry.java b/spring-web/src/main/java/org/springframework/web/util/patterns/PathPatternRegistry.java deleted file mode 100644 index ee4fc95ba4d..00000000000 --- a/spring-web/src/main/java/org/springframework/web/util/patterns/PathPatternRegistry.java +++ /dev/null @@ -1,336 +0,0 @@ -/* - * Copyright 2002-2017 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.web.util.patterns; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.Comparator; -import java.util.HashSet; -import java.util.Iterator; -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.StringUtils; - -/** - * Registry that holds {@code PathPattern}s instances - * sorted according to their specificity (most specific patterns first). - *

For a given path pattern string, {@code PathPattern} variants - * can be generated and registered automatically, depending - * on the {@code useTrailingSlashMatch}, {@code useSuffixPatternMatch} - * and {@code fileExtensions} properties. - * - * @author Brian Clozel - * @since 5.0 - */ -public class PathPatternRegistry { - - private final PathPatternParser pathPatternParser; - - private final HashSet patterns; - - private boolean useSuffixPatternMatch = false; - - private boolean useTrailingSlashMatch = false; - - private Set fileExtensions = Collections.emptySet(); - - /** - * Create a new {@code PathPatternRegistry} with defaults options for - * pattern variants generation. - *

By default, no pattern variant will be generated. - */ - public PathPatternRegistry() { - this.pathPatternParser = new PathPatternParser(); - this.patterns = new HashSet<>(); - } - - public PathPatternRegistry(Set patterns) { - this(); - this.patterns.addAll(patterns); - } - - /** - * Whether to match to paths irrespective of the presence of a trailing slash. - */ - public boolean useSuffixPatternMatch() { - return useSuffixPatternMatch; - } - - /** - * Whether to use suffix pattern match (".*") when matching patterns to - * requests. If enabled a path pattern such as "/users" will also - * generate the following pattern variant: "/users.*". - *

By default this is set to {@code false}. - */ - public void setUseSuffixPatternMatch(boolean useSuffixPatternMatch) { - this.useSuffixPatternMatch = useSuffixPatternMatch; - } - - /** - * Whether to generate path pattern variants with a trailing slash. - */ - public boolean useTrailingSlashMatch() { - return useTrailingSlashMatch; - } - - /** - * Whether to match to paths irrespective of the presence of a trailing slash. - * If enabled a path pattern such as "/users" will also generate the - * following pattern variant: "/users/". - *

The default value is {@code false}. - */ - public void setUseTrailingSlashMatch(boolean useTrailingSlashMatch) { - this.useTrailingSlashMatch = useTrailingSlashMatch; - } - - /** - * Return the set of file extensions to use for suffix pattern matching. - */ - public Set getFileExtensions() { - return fileExtensions; - } - - /** - * Configure the set of file extensions to use for suffix pattern matching. - * For a given path "/users", each file extension will be used to - * generate a path pattern variant such as "json" -> "/users.json". - *

The default value is an empty {@code Set} - */ - public void setFileExtensions(Set fileExtensions) { - Set fixedFileExtensions = (fileExtensions != null) ? fileExtensions.stream() - .map(ext -> (ext.charAt(0) != '.') ? "." + ext : ext) - .collect(Collectors.toSet()) : Collections.emptySet(); - this.fileExtensions = fixedFileExtensions; - } - - /** - * Return a (read-only) set of all patterns for matching (including generated pattern variants). - */ - public Set getPatterns() { - return Collections.unmodifiableSet(this.patterns); - } - - /** - * 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.patterns.stream() - .filter(pattern -> pattern.matches(lookupPath)) - .collect(Collectors.toCollection(() -> - new TreeSet<>(new PatternSetComparator(lookupPath)))); - } - - /** - * Process the path pattern data using the internal {@link PathPatternParser} - * instance, producing a {@link PathPattern} object that can be used for fast matching - * against paths. - * - * @param pathPattern the input path pattern, e.g. /foo/{bar} - * @return a PathPattern for quickly matching paths against the specified path pattern - */ - public PathPattern parsePattern(String pathPattern) { - return this.pathPatternParser.parse(pathPattern); - } - - /** - * Remove all {@link PathPattern}s from this registry - */ - public void clear() { - this.patterns.clear(); - } - - /** - * Parse the given {@code rawPattern} and adds it to this registry, - * as well as pattern variants, depending on the given options and - * the nature of the input pattern. - *

The following set of patterns will be added: - *

    - *
  • the pattern given as input, e.g. "/foo/{bar}" - *
  • if {@link #useSuffixPatternMatch()}, variants for each given - * {@link #getFileExtensions()}, such as "/foo/{bar}.pdf" or a variant for all extensions, - * such as "/foo/{bar}.*" - *
  • if {@link #useTrailingSlashMatch()}, a variant such as "/foo/{bar}/" - *
- * @param rawPattern raw path pattern to parse and register - * @return the list of {@link PathPattern} that were registered as a result - */ - public List register(String rawPattern) { - List newPatterns = generatePathPatterns(rawPattern); - this.patterns.addAll(newPatterns); - return newPatterns; - } - - private String prependLeadingSlash(String pattern) { - if (StringUtils.hasLength(pattern) && !pattern.startsWith("/")) { - return "/" + pattern; - } - else { - return pattern; - } - } - - private List generatePathPatterns(String rawPattern) { - String fixedPattern = prependLeadingSlash(rawPattern); - List patterns = new ArrayList<>(); - PathPattern pattern = this.pathPatternParser.parse(fixedPattern); - patterns.add(pattern); - if (StringUtils.hasLength(fixedPattern) && !pattern.isCatchAll()) { - if (this.useSuffixPatternMatch) { - if (this.fileExtensions != null && !this.fileExtensions.isEmpty()) { - for (String extension : this.fileExtensions) { - patterns.add(this.pathPatternParser.parse(fixedPattern + extension)); - } - } - else { - patterns.add(this.pathPatternParser.parse(fixedPattern + ".*")); - } - } - if (this.useTrailingSlashMatch && !fixedPattern.endsWith("/")) { - patterns.add(this.pathPatternParser.parse(fixedPattern + "/")); - } - } - return patterns; - } - - /** - * Parse the given {@code rawPattern} and removes it to this registry, - * as well as pattern variants, depending on the given options and - * the nature of the input pattern. - * - * @param rawPattern raw path pattern to parse and unregister - * @return the list of {@link PathPattern} that were unregistered as a result - */ - public List unregister(String rawPattern) { - List unregisteredPatterns = generatePathPatterns(rawPattern); - this.patterns.removeAll(unregisteredPatterns); - return unregisteredPatterns; - } - - - /** - * Combine the patterns contained in the current registry - * with the ones in the other, into a new {@code PathPatternRegistry} instance. - *

Given the current registry contains "/prefix" and the other contains - * "/foo" and "/bar/{item}", the combined result will be: a new registry - * containing "/prefix/foo" and "/prefix/bar/{item}". - * @param other other {@code PathPatternRegistry} to combine with - * @return a new instance of {@code PathPatternRegistry} that combines both - * @see PathPattern#combine(String) - */ - public PathPatternRegistry combine(PathPatternRegistry other) { - PathPatternRegistry result = new PathPatternRegistry(); - result.setUseSuffixPatternMatch(this.useSuffixPatternMatch); - result.setUseTrailingSlashMatch(this.useTrailingSlashMatch); - result.setFileExtensions(this.fileExtensions); - if (!this.patterns.isEmpty() && !other.patterns.isEmpty()) { - for (PathPattern pattern1 : this.patterns) { - for (PathPattern pattern2 : other.patterns) { - String combined = pattern1.combine(pattern2.getPatternString()); - result.register(combined); - } - } - } - else if (!this.patterns.isEmpty()) { - result.patterns.addAll(this.patterns); - } - else if (!other.patterns.isEmpty()) { - result.patterns.addAll(other.patterns); - } - else { - result.register(""); - } - return result; - } - - /** - * Given a full path, returns a {@link Comparator} suitable for sorting pattern - * registries in order of explicitness for that path. - *

The returned {@code Comparator} will - * {@linkplain java.util.Collections#sort(java.util.List, java.util.Comparator) sort} - * a list so that more specific patterns registries come before generic ones. - * @param path the full path to use for comparison - * @return a comparator capable of sorting patterns in order of explicitness - */ - public Comparator getComparator(final String path) { - return (r1, r2) -> { - PatternSetComparator comparator = new PatternSetComparator(path); - Iterator it1 = r1.patterns.stream() - .sorted(comparator).collect(Collectors.toList()).iterator(); - Iterator it2 = r2.patterns.stream() - .sorted(comparator).collect(Collectors.toList()).iterator(); - while (it1.hasNext() && it2.hasNext()) { - int result = comparator.compare(it1.next(), it2.next()); - if (result != 0) { - return result; - } - } - if (it1.hasNext()) { - return -1; - } - else if (it2.hasNext()) { - return 1; - } - else { - return 0; - } - }; - } - - private class PatternSetComparator implements Comparator { - - private final String path; - - public PatternSetComparator(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; - } - - } - -} diff --git a/spring-web/src/test/java/org/springframework/web/cors/reactive/UrlBasedCorsConfigurationSourceTests.java b/spring-web/src/test/java/org/springframework/web/cors/reactive/UrlBasedCorsConfigurationSourceTests.java index faf19310ad1..f29d3da41fb 100644 --- a/spring-web/src/test/java/org/springframework/web/cors/reactive/UrlBasedCorsConfigurationSourceTests.java +++ b/spring-web/src/test/java/org/springframework/web/cors/reactive/UrlBasedCorsConfigurationSourceTests.java @@ -25,11 +25,9 @@ import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse import org.springframework.web.cors.CorsConfiguration; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.adapter.DefaultServerWebExchange; -import org.springframework.web.util.patterns.PathPattern; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; -import static org.mockito.Mockito.mock; /** * Unit tests for {@link UrlBasedCorsConfigurationSource}. @@ -62,7 +60,7 @@ public class UrlBasedCorsConfigurationSourceTests { @Test(expected = UnsupportedOperationException.class) public void unmodifiableConfigurationsMap() { - this.configSource.getCorsConfigurations().put(mock(PathPattern.class), new CorsConfiguration()); + this.configSource.getCorsConfigurations().put("/**", new CorsConfiguration()); } private ServerWebExchange createExchange(HttpMethod httpMethod, String url) { diff --git a/spring-web/src/test/java/org/springframework/web/util/patterns/PathPatternRegistryTests.java b/spring-web/src/test/java/org/springframework/web/util/patterns/PathPatternRegistryTests.java deleted file mode 100644 index 823a1b52b62..00000000000 --- a/spring-web/src/test/java/org/springframework/web/util/patterns/PathPatternRegistryTests.java +++ /dev/null @@ -1,177 +0,0 @@ -/* - * Copyright 2002-2017 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.web.util.patterns; - -import java.util.Collection; -import java.util.HashSet; -import java.util.List; -import java.util.Set; -import java.util.stream.Collectors; - -import org.hamcrest.Matchers; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; - -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; - - @Rule - public ExpectedException thrown = ExpectedException.none(); - - @Before - public void setUp() throws Exception { - this.registry = new PathPatternRegistry(); - } - - @Test - public void shouldFixFileExtensions() { - Set fileExtensions = new HashSet<>(); - fileExtensions.add("json"); - fileExtensions.add("xml"); - this.registry.setFileExtensions(fileExtensions); - assertThat(this.registry.getFileExtensions(), contains(".json", ".xml")); - } - - @Test - public void shouldPrependPatternsWithSlash() { - this.registry.register("foo/bar"); - assertThat(getPatternList(this.registry.getPatterns()), Matchers.containsInAnyOrder("/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"); - } - - @Test - public void shouldNotRegisterPatternVariants() { - List patterns = this.registry.register("/foo/{bar}"); - assertThat(getPatternList(patterns), Matchers.containsInAnyOrder("/foo/{bar}")); - } - - @Test - public void shouldRegisterTrailingSlashVariants() { - this.registry.setUseTrailingSlashMatch(true); - List patterns = this.registry.register("/foo/{bar}"); - assertThat(getPatternList(patterns), Matchers.containsInAnyOrder("/foo/{bar}", "/foo/{bar}/")); - } - - @Test - public void shouldRegisterSuffixVariants() { - this.registry.setUseSuffixPatternMatch(true); - List patterns = this.registry.register("/foo/{bar}"); - assertThat(getPatternList(patterns), Matchers.containsInAnyOrder("/foo/{bar}", "/foo/{bar}.*")); - } - - @Test - public void shouldRegisterExtensionsVariants() { - Set fileExtensions = new HashSet<>(); - fileExtensions.add(".json"); - fileExtensions.add(".xml"); - this.registry.setUseSuffixPatternMatch(true); - this.registry.setFileExtensions(fileExtensions); - List patterns = this.registry.register("/foo/{bar}"); - assertThat(getPatternList(patterns), - Matchers.containsInAnyOrder("/foo/{bar}", "/foo/{bar}.xml", "/foo/{bar}.json")); - } - - @Test - public void shouldRegisterAllVariants() { - Set fileExtensions = new HashSet<>(); - fileExtensions.add(".json"); - fileExtensions.add(".xml"); - this.registry.setUseSuffixPatternMatch(true); - this.registry.setUseTrailingSlashMatch(true); - this.registry.setFileExtensions(fileExtensions); - List patterns = this.registry.register("/foo/{bar}"); - assertThat(getPatternList(patterns), Matchers.containsInAnyOrder("/foo/{bar}", - "/foo/{bar}.xml", "/foo/{bar}.json", "/foo/{bar}/")); - } - - @Test - public void combineEmptyRegistries() { - PathPatternRegistry result = this.registry.combine(new PathPatternRegistry()); - assertThat(getPatternList(result.getPatterns()), Matchers.containsInAnyOrder("")); - } - - @Test - public void combineWithEmptyRegistry() { - this.registry.register("/foo"); - PathPatternRegistry result = this.registry.combine(new PathPatternRegistry()); - assertThat(getPatternList(result.getPatterns()), Matchers.containsInAnyOrder("/foo")); - } - - @Test - public void combineRegistries() { - this.registry.register("/foo"); - PathPatternRegistry other = new PathPatternRegistry(); - other.register("/bar"); - other.register("/baz"); - PathPatternRegistry result = this.registry.combine(other); - assertThat(getPatternList(result.getPatterns()), Matchers.containsInAnyOrder("/foo/bar", "/foo/baz")); - } - - @Test - public void registerPatternsWithSameSpecificity() { - PathPattern fooOne = this.registry.parsePattern("/fo?"); - PathPattern fooTwo = this.registry.parsePattern("/f?o"); - assertThat(fooOne.compareTo(fooTwo), is(0)); - - this.registry.register("/fo?"); - this.registry.register("/f?o"); - Set matches = this.registry.findMatches("/foo"); - assertThat(getPatternList(matches), Matchers.contains("/f?o", "/fo?")); - } - - @Test - public void findNoMatch() { - this.registry.register("/foo/{bar}"); - assertThat(this.registry.findMatches("/other"), hasSize(0)); - } - - @Test - public void orderMatchesBySpecificity() { - this.registry.register("/foo/{*baz}"); - this.registry.register("/foo/bar/baz"); - this.registry.register("/foo/bar/{baz}"); - Set matches = this.registry.findMatches("/foo/bar/baz"); - assertThat(getPatternList(matches), Matchers.contains("/foo/bar/baz", "/foo/bar/{baz}", - "/foo/{*baz}")); - } - - - private List getPatternList(Collection parsedPatterns) { - return parsedPatterns.stream().map(pattern -> pattern.getPatternString()).collect(Collectors.toList()); - - } - -} 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 a24714601da..32e5b1ac2cd 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2016 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. @@ -16,6 +16,7 @@ package org.springframework.web.reactive.config; +import org.springframework.util.PathMatcher; import org.springframework.web.server.support.HttpRequestPathHelper; /** @@ -26,22 +27,24 @@ import org.springframework.web.server.support.HttpRequestPathHelper; */ public class PathMatchConfigurer { - private boolean suffixPatternMatch = false; + private Boolean suffixPatternMatch; - private boolean trailingSlashMatch = true; + private Boolean trailingSlashMatch; - private boolean registeredSuffixPatternMatch = false; + private Boolean registeredSuffixPatternMatch; private HttpRequestPathHelper pathHelper; + private PathMatcher pathMatcher; + /** * 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 false}. + *

By default this is set to {@code true}. * @see #registeredSuffixPatternMatch */ - public PathMatchConfigurer setUseSuffixPatternMatch(boolean suffixPatternMatch) { + public PathMatchConfigurer setUseSuffixPatternMatch(Boolean suffixPatternMatch) { this.suffixPatternMatch = suffixPatternMatch; return this; } @@ -51,7 +54,7 @@ public class PathMatchConfigurer { * If enabled a method mapped to "/users" also matches to "/users/". *

The default value is {@code true}. */ - public PathMatchConfigurer setUseTrailingSlashMatch(boolean trailingSlashMatch) { + public PathMatchConfigurer setUseTrailingSlashMatch(Boolean trailingSlashMatch) { this.trailingSlashMatch = trailingSlashMatch; return this; } @@ -61,9 +64,9 @@ public class PathMatchConfigurer { * 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 "false". + *

By default this is set to "true". */ - public PathMatchConfigurer setUseRegisteredSuffixPatternMatch(boolean registeredSuffixPatternMatch) { + public PathMatchConfigurer setUseRegisteredSuffixPatternMatch(Boolean registeredSuffixPatternMatch) { this.registeredSuffixPatternMatch = registeredSuffixPatternMatch; return this; } @@ -77,15 +80,24 @@ public class PathMatchConfigurer { return this; } - protected boolean isUseSuffixPatternMatch() { + /** + * Set the PathMatcher for matching URL paths against registered URL patterns. + *

Default is {@link org.springframework.util.AntPathMatcher AntPathMatcher}. + */ + public PathMatchConfigurer setPathMatcher(PathMatcher pathMatcher) { + this.pathMatcher = pathMatcher; + return this; + } + + protected Boolean isUseSuffixPatternMatch() { return this.suffixPatternMatch; } - protected boolean isUseTrailingSlashMatch() { + protected Boolean isUseTrailingSlashMatch() { return this.trailingSlashMatch; } - protected boolean isUseRegisteredSuffixPatternMatch() { + protected Boolean isUseRegisteredSuffixPatternMatch() { return this.registeredSuffixPatternMatch; } @@ -93,4 +105,8 @@ public class PathMatchConfigurer { return this.pathHelper; } + protected PathMatcher getPathMatcher() { + return this.pathMatcher; + } + } 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 1b9790f8ee4..112b5d6dc75 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 @@ -81,7 +81,6 @@ import org.springframework.web.reactive.result.view.ViewResolver; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebExceptionHandler; import org.springframework.web.server.handler.ResponseStatusExceptionHandler; -import org.springframework.web.util.patterns.PathPatternRegistry; /** * The main class for Spring Web Reactive configuration. @@ -137,23 +136,28 @@ public class WebFluxConfigurationSupport implements ApplicationContextAware { @Bean public RequestMappingHandlerMapping requestMappingHandlerMapping() { - CompositeContentTypeResolver contentTypeResolver = webFluxContentTypeResolver(); RequestMappingHandlerMapping mapping = createRequestMappingHandlerMapping(); mapping.setOrder(0); - mapping.setContentTypeResolver(contentTypeResolver); + mapping.setContentTypeResolver(webFluxContentTypeResolver()); mapping.setCorsConfigurations(getCorsConfigurations()); - PathPatternRegistry pathPatternRegistry = new PathPatternRegistry(); - mapping.setPatternRegistry(pathPatternRegistry); PathMatchConfigurer configurer = getPathMatchConfigurer(); - pathPatternRegistry.setUseSuffixPatternMatch(configurer.isUseSuffixPatternMatch()); - pathPatternRegistry.setUseTrailingSlashMatch(configurer.isUseTrailingSlashMatch()); - if (configurer.isUseRegisteredSuffixPatternMatch() && contentTypeResolver != null) { - pathPatternRegistry.setFileExtensions(contentTypeResolver.getKeys()); + if (configurer.isUseSuffixPatternMatch() != null) { + mapping.setUseSuffixPatternMatch(configurer.isUseSuffixPatternMatch()); + } + if (configurer.isUseRegisteredSuffixPatternMatch() != null) { + mapping.setUseRegisteredSuffixPatternMatch(configurer.isUseRegisteredSuffixPatternMatch()); + } + if (configurer.isUseTrailingSlashMatch() != null) { + mapping.setUseTrailingSlashMatch(configurer.isUseTrailingSlashMatch()); + } + if (configurer.getPathMatcher() != null) { + mapping.setPathMatcher(configurer.getPathMatcher()); } if (configurer.getPathHelper() != null) { mapping.setPathHelper(configurer.getPathHelper()); } + return mapping; } @@ -242,6 +246,9 @@ public class WebFluxConfigurationSupport implements ApplicationContextAware { AbstractHandlerMapping handlerMapping = registry.getHandlerMapping(); if (handlerMapping != null) { PathMatchConfigurer pathMatchConfigurer = getPathMatchConfigurer(); + if (pathMatchConfigurer.getPathMatcher() != null) { + handlerMapping.setPathMatcher(pathMatchConfigurer.getPathMatcher()); + } if (pathMatchConfigurer.getPathHelper() != null) { handlerMapping.setPathHelper(pathMatchConfigurer.getPathHelper()); } 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 be91f5c9075..1e68d58f88f 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 @@ -23,6 +23,7 @@ import reactor.core.publisher.Mono; import org.springframework.context.support.ApplicationObjectSupport; import org.springframework.core.Ordered; 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; @@ -33,8 +34,7 @@ import org.springframework.web.reactive.HandlerMapping; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebHandler; import org.springframework.web.server.support.HttpRequestPathHelper; -import org.springframework.web.util.patterns.PathPattern; -import org.springframework.web.util.patterns.PathPatternRegistry; +import org.springframework.web.util.ParsingPathMatcher; /** * Abstract base class for {@link org.springframework.web.reactive.HandlerMapping} @@ -53,11 +53,12 @@ public abstract class AbstractHandlerMapping extends ApplicationObjectSupport im private HttpRequestPathHelper pathHelper = new HttpRequestPathHelper(); + private PathMatcher pathMatcher = new ParsingPathMatcher(); + private final UrlBasedCorsConfigurationSource globalCorsConfigSource = new UrlBasedCorsConfigurationSource(); private CorsProcessor corsProcessor = new DefaultCorsProcessor(); - protected PathPatternRegistry patternRegistry = new PathPatternRegistry(); /** * Specify the order value for this HandlerMapping bean. @@ -101,19 +102,22 @@ public abstract class AbstractHandlerMapping extends ApplicationObjectSupport im } /** - * Return the {@link PathPatternRegistry} instance to use for parsing - * and matching path patterns. + * Set the PathMatcher implementation to use for matching URL paths + * against registered URL patterns. Default is AntPathMatcher. + * @see org.springframework.util.AntPathMatcher */ - public PathPatternRegistry getPatternRegistry() { - return patternRegistry; + public void setPathMatcher(PathMatcher pathMatcher) { + Assert.notNull(pathMatcher, "PathMatcher must not be null"); + this.pathMatcher = pathMatcher; + this.globalCorsConfigSource.setPathMatcher(pathMatcher); } /** - * Set the {@link PathPatternRegistry} instance to use for parsing - * and matching path patterns. + * Return the PathMatcher implementation to use for matching URL paths + * against registered URL patterns. */ - public void setPatternRegistry(PathPatternRegistry patternRegistry) { - this.patternRegistry = patternRegistry; + public PathMatcher getPathMatcher() { + return this.pathMatcher; } /** @@ -122,16 +126,13 @@ public abstract class AbstractHandlerMapping extends ApplicationObjectSupport im * configuration if any. */ public void setCorsConfigurations(Map corsConfigurations) { - for(String patternString : corsConfigurations.keySet()) { - this.globalCorsConfigSource - .registerCorsConfiguration(patternString, corsConfigurations.get(patternString)); - } + this.globalCorsConfigSource.setCorsConfigurations(corsConfigurations); } /** * Return the "global" CORS configuration. */ - public Map getCorsConfigurations() { + public Map getCorsConfigurations() { return this.globalCorsConfigSource.getCorsConfigurations(); } 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 2f1037e57b0..57122fd8aae 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,32 +16,32 @@ 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.SortedSet; import reactor.core.publisher.Mono; import org.springframework.beans.BeansException; import org.springframework.util.Assert; import org.springframework.web.server.ServerWebExchange; -import org.springframework.web.util.patterns.PathPattern; -import org.springframework.web.util.patterns.PathPatternParser; /** * 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 path pattern matches, e.g. a registered "/t*" pattern matches + * 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 PathPatternParser} javadoc. + * {@link org.springframework.util.AntPathMatcher AntPathMatcher} javadoc. * - *

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. + *

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. * * @author Rossen Stoyanchev * @author Juergen Hoeller @@ -49,9 +49,12 @@ import org.springframework.web.util.patterns.PathPatternParser; */ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { + private boolean useTrailingSlashMatch = false; + private boolean lazyInitHandlers = false; - private final Map handlerMap = new LinkedHashMap<>(); + private final Map handlerMap = new LinkedHashMap<>(); + /** * Whether to match to URLs irrespective of the presence of a trailing slash. @@ -59,14 +62,14 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { *

The default value is {@code false}. */ public void setUseTrailingSlashMatch(boolean useTrailingSlashMatch) { - this.patternRegistry.setUseTrailingSlashMatch(useTrailingSlashMatch); + this.useTrailingSlashMatch = useTrailingSlashMatch; } /** * Whether to match to URLs irrespective of the presence of a trailing slash. */ public boolean useTrailingSlashMatch() { - return this.patternRegistry.useTrailingSlashMatch(); + return this.useTrailingSlashMatch; } /** @@ -88,7 +91,7 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { * as key and the handler object (or handler bean name in case of a lazy-init handler) * as value. */ - public final Map getHandlerMap() { + public final Map getHandlerMap() { return Collections.unmodifiableMap(this.handlerMap); } @@ -119,7 +122,7 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { * *

Supports direct matches, e.g. a registered "/test" matches "/test", * and various Ant-style pattern matches, e.g. a registered "/t*" matches - * both "/test" and "/team". For details, see the PathPattern class. + * both "/test" and "/team". For details, see the AntPathMatcher class. * *

Looks for the most exact pattern, where most exact is defined as * the longest path pattern. @@ -130,21 +133,54 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { * @see org.springframework.util.AntPathMatcher */ protected Object lookupHandler(String urlPath, ServerWebExchange exchange) throws Exception { - SortedSet matches = this.patternRegistry.findMatches(urlPath); + // Direct match? + Object handler = this.handlerMap.get(urlPath); + if (handler != null) { + return handleMatch(handler, urlPath, urlPath, exchange); + } + + // Pattern match? + List matches = new ArrayList<>(); + for (String pattern : this.handlerMap.keySet()) { + if (getPathMatcher().match(pattern, urlPath)) { + matches.add(pattern); + } + else if (useTrailingSlashMatch()) { + if (!pattern.endsWith("/") && getPathMatcher().match(pattern + "/", urlPath)) { + matches.add(pattern +"/"); + } + } + } + + String bestMatch = null; + Comparator comparator = getPathMatcher().getPatternComparator(urlPath); if (!matches.isEmpty()) { + Collections.sort(matches, comparator); if (logger.isDebugEnabled()) { logger.debug("Matching patterns for request [" + urlPath + "] are " + matches); } - PathPattern bestMatch = matches.first(); - String pathWithinMapping = bestMatch.extractPathWithinPattern(urlPath); - return handleMatch(this.handlerMap.get(bestMatch), bestMatch, pathWithinMapping, exchange); + bestMatch = matches.get(0); + } + if (bestMatch != null) { + handler = this.handlerMap.get(bestMatch); + 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 + "]"); + } + } + String pathWithinMapping = getPathMatcher().extractPathWithinPattern(bestMatch, urlPath); + return handleMatch(handler, bestMatch, pathWithinMapping, exchange); } // No handler found... return null; } - private Object handleMatch(Object handler, PathPattern bestMatch, String pathWithinMapping, + private Object handleMatch(Object handler, String bestMatch, String pathWithinMapping, ServerWebExchange exchange) throws Exception { // Bean name or resolved handler? @@ -207,22 +243,20 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { resolvedHandler = getApplicationContext().getBean(handlerName); } } - for (PathPattern newPattern : this.patternRegistry.register(urlPath)) { - Object mappedHandler = this.handlerMap.get(newPattern); - 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(newPattern, resolvedHandler); + 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."); } } - if (logger.isInfoEnabled()) { - logger.info("Mapped URL path [" + urlPath + "] onto " + getHandlerDescription(handler)); + else { + this.handlerMap.put(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/resource/ResourceUrlProvider.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java index 0e104abf967..873817d014d 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2016 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. @@ -17,10 +17,11 @@ 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.SortedSet; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -32,11 +33,11 @@ import org.springframework.context.ApplicationListener; import org.springframework.context.event.ContextRefreshedEvent; import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.http.server.reactive.ServerHttpRequest; +import org.springframework.util.PathMatcher; import org.springframework.web.reactive.handler.SimpleUrlHandlerMapping; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.support.HttpRequestPathHelper; -import org.springframework.web.util.patterns.PathPattern; -import org.springframework.web.util.patterns.PathPatternRegistry; +import org.springframework.web.util.ParsingPathMatcher; /** * A central component to use to obtain the public URL path that clients should @@ -55,9 +56,9 @@ public class ResourceUrlProvider implements ApplicationListener handlerMap = new LinkedHashMap<>(); + private final Map handlerMap = new LinkedHashMap<>(); private boolean autodetect = true; @@ -78,6 +79,21 @@ public class ResourceUrlProvider implements ApplicationListenerNote: by default resource mappings are auto-detected @@ -86,14 +102,8 @@ public class ResourceUrlProvider implements ApplicationListener handlerMap) { if (handlerMap != null) { - this.patternRegistry.clear(); this.handlerMap.clear(); - - handlerMap.forEach((pattern, handler) -> { - this.patternRegistry - .register(pattern) - .forEach(pathPattern -> this.handlerMap.put(pathPattern, handler)); - }); + this.handlerMap.putAll(handlerMap); this.autodetect = false; } } @@ -102,7 +112,7 @@ public class ResourceUrlProvider implements ApplicationListener getHandlerMap() { + public Map getHandlerMap() { return this.handlerMap; } @@ -117,7 +127,6 @@ 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; @@ -210,18 +218,26 @@ public class ResourceUrlProvider implements ApplicationListener matches = this.patternRegistry.findMatches(lookupPath); - if (matches.isEmpty()) { + List matchingPatterns = new ArrayList<>(); + for (String pattern : this.handlerMap.keySet()) { + if (getPathMatcher().match(pattern, lookupPath)) { + matchingPatterns.add(pattern); + } + } + + if (matchingPatterns.isEmpty()) { return Mono.empty(); } - return Flux.fromIterable(matches) + Comparator patternComparator = getPathMatcher().getPatternComparator(lookupPath); + Collections.sort(matchingPatterns, patternComparator); + + return Flux.fromIterable(matchingPatterns) .concatMap(pattern -> { - String pathWithinMapping = pattern.extractPathWithinPattern(lookupPath); + String pathWithinMapping = getPathMatcher().extractPathWithinPattern(pattern, lookupPath); String pathMapping = lookupPath.substring(0, lookupPath.indexOf(pathWithinMapping)); if (logger.isTraceEnabled()) { - logger.trace("Invoking ResourceResolverChain for URL pattern \"" - + pattern.getPatternString() + "\""); + logger.trace("Invoking ResourceResolverChain for URL pattern \"" + pattern + "\""); } ResourceWebHandler handler = this.handlerMap.get(pattern); ResourceResolverChain chain = new DefaultResourceResolverChain(handler.getResourceResolvers()); 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 d1568523882..b26af55ce7d 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2016 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. @@ -16,39 +16,52 @@ package org.springframework.web.reactive.result.condition; +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 org.springframework.util.PathMatcher; +import org.springframework.util.StringUtils; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.support.HttpRequestPathHelper; -import org.springframework.web.util.patterns.PathPattern; -import org.springframework.web.util.patterns.PathPatternRegistry; +import org.springframework.web.util.ParsingPathMatcher; /** * 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 PathPatternRegistry patternRegistry; + private final Set patterns; private final HttpRequestPathHelper pathHelper; + private final PathMatcher pathMatcher; + + private final boolean useSuffixPatternMatch; + + private final boolean useTrailingSlashMatch; + + private final Set fileExtensions = new HashSet<>(); + + /** * Creates a new instance with the given URL patterns. * Each pattern that is not empty and does not start with "/" is prepended with "/". * @param patterns 0 or more URL patterns; if 0 the condition will match to every request. */ public PatternsRequestCondition(String... patterns) { - this(patterns, null, null); + this(asList(patterns), null, null, true, true, null); } /** @@ -56,35 +69,66 @@ public final class PatternsRequestCondition extends AbstractRequestCondition extensions) { + + this(asList(patterns), pathHelper, pathMatcher, useSuffixPatternMatch, useTrailingSlashMatch, extensions); } - private static PathPatternRegistry createPatternSet(String[] patterns, PathPatternRegistry pathPatternRegistry) { - PathPatternRegistry patternSet = pathPatternRegistry != null ? pathPatternRegistry : new PathPatternRegistry(); - if(patterns != null) { - Arrays.asList(patterns).stream().forEach(p -> patternSet.register(p)); + /** + * Private constructor accepting a collection of patterns. + */ + private PatternsRequestCondition(Collection patterns, HttpRequestPathHelper pathHelper, + PathMatcher pathMatcher, boolean useSuffixPatternMatch, boolean useTrailingSlashMatch, + Set fileExtensions) { + + this.patterns = Collections.unmodifiableSet(prependLeadingSlash(patterns)); + this.pathHelper = (pathHelper != null ? pathHelper : new HttpRequestPathHelper()); + 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); + } } - return patternSet; } - private PatternsRequestCondition(PathPatternRegistry patternRegistry, HttpRequestPathHelper pathHelper) { - this.patternRegistry = patternRegistry; - this.pathHelper = pathHelper; + + private static List asList(String... patterns) { + return (patterns != null ? Arrays.asList(patterns) : Collections.emptyList()); } + private static Set 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 this.patternRegistry.getPatterns(); + public Set getPatterns() { + return this.patterns; } @Override - protected Collection getContent() { - return this.patternRegistry.getPatterns(); + protected Collection getContent() { + return this.patterns; } @Override @@ -104,7 +148,25 @@ public final class PatternsRequestCondition extends AbstractRequestCondition result = new LinkedHashSet<>(); + if (!this.patterns.isEmpty() && !other.patterns.isEmpty()) { + for (String pattern1 : this.patterns) { + for (String pattern2 : other.patterns) { + result.add(this.pathMatcher.combine(pattern1, pattern2)); + } + } + } + else if (!this.patterns.isEmpty()) { + result.addAll(this.patterns); + } + else if (!other.patterns.isEmpty()) { + result.addAll(other.patterns); + } + else { + result.add(""); + } + return new PatternsRequestCondition(result, this.pathHelper, this.pathMatcher, this.useSuffixPatternMatch, + this.useTrailingSlashMatch, this.fileExtensions); } /** @@ -125,18 +187,16 @@ public final class PatternsRequestCondition extends AbstractRequestCondition matches = getMatchingPatterns(lookupPath); + List matches = getMatchingPatterns(lookupPath); - if(!matches.isEmpty()) { - PathPatternRegistry registry = new PathPatternRegistry(matches); - return new PatternsRequestCondition(registry, this.pathHelper); - } - return null; + return matches.isEmpty() ? null : + new PatternsRequestCondition(matches, this.pathHelper, this.pathMatcher, this.useSuffixPatternMatch, + this.useTrailingSlashMatch, this.fileExtensions); } /** @@ -146,16 +206,54 @@ public final class PatternsRequestCondition extends AbstractRequestCondition getMatchingPatterns(String lookupPath) { - return this.patternRegistry.findMatches(lookupPath); + public List 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.useSuffixPatternMatch) { + if (!this.fileExtensions.isEmpty() && lookupPath.indexOf('.') != -1) { + for (String extension : this.fileExtensions) { + if (this.pathMatcher.match(pattern + extension, lookupPath)) { + return pattern + extension; + } + } + } + else { + boolean hasSuffix = pattern.indexOf('.') != -1; + if (!hasSuffix && this.pathMatcher.match(pattern + ".*", 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; } /** * Compare the two conditions based on the URL patterns they contain. * Patterns are compared one at a time, from top to bottom via - * {@link PathPatternRegistry#getComparator(String)}. If all compared + * {@link PathMatcher#getPatternComparator(String)}. 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 @@ -166,8 +264,24 @@ public final class PatternsRequestCondition extends AbstractRequestCondition comparator = this.patternRegistry.getComparator(lookupPath); - return comparator.compare(this.patternRegistry, other.patternRegistry); + Comparator patternComparator = this.pathMatcher.getPatternComparator(lookupPath); + Iterator iterator = this.patterns.iterator(); + Iterator iteratorOther = other.patterns.iterator(); + while (iterator.hasNext() && iteratorOther.hasNext()) { + int result = patternComparator.compare(iterator.next(), iteratorOther.next()); + if (result != 0) { + return result; + } + } + if (iterator.hasNext()) { + return -1; + } + else if (iteratorOther.hasNext()) { + return 1; + } + else { + return 0; + } } } 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 e0e90d12b23..325fd55acd2 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 @@ -28,7 +28,6 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.locks.ReentrantReadWriteLock; -import java.util.stream.Collectors; import reactor.core.publisher.Mono; @@ -45,7 +44,6 @@ import org.springframework.web.method.HandlerMethod; import org.springframework.web.reactive.HandlerMapping; import org.springframework.web.reactive.handler.AbstractHandlerMapping; import org.springframework.web.server.ServerWebExchange; -import org.springframework.web.util.patterns.PathPattern; /** * Abstract base class for {@link HandlerMapping} implementations that define @@ -90,9 +88,6 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap ALLOW_CORS_CONFIG.setAllowCredentials(true); } - private final MultiValueMap mappingLookup = new LinkedMultiValueMap<>(); - - private final ReentrantReadWriteLock readWriteLock = new ReentrantReadWriteLock(); private final MappingRegistry mappingRegistry = new MappingRegistry(); @@ -103,12 +98,12 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap * Return a (read-only) map with all mappings and HandlerMethod's. */ public Map getHandlerMethods() { - this.readWriteLock.readLock().lock(); + this.mappingRegistry.acquireReadLock(); try { return Collections.unmodifiableMap(this.mappingRegistry.getMappings()); } finally { - this.readWriteLock.readLock().unlock(); + this.mappingRegistry.releaseReadLock(); } } @@ -127,18 +122,7 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap * @param method the method */ public void registerMapping(T mapping, Object handler, Method method) { - this.readWriteLock.writeLock().lock(); - try { - getMappingPathPatterns(mapping).forEach(pattern -> { - getPatternRegistry().register(pattern).forEach( - pathPattern -> this.mappingLookup.add(pathPattern, mapping) - ); - }); - this.mappingRegistry.register(mapping, handler, method); - } - finally { - this.readWriteLock.writeLock().unlock(); - } + this.mappingRegistry.register(mapping, handler, method); } /** @@ -147,18 +131,7 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap * @param mapping the mapping to unregister */ public void unregisterMapping(T mapping) { - this.readWriteLock.writeLock().lock(); - try { - getMappingPathPatterns(mapping).forEach(pattern -> { - getPatternRegistry().unregister(pattern).forEach( - pathPattern -> this.mappingLookup.remove(pathPattern, mapping) - ); - }); - this.mappingRegistry.unregister(mapping); - } - finally { - this.readWriteLock.writeLock().unlock(); - } + this.mappingRegistry.unregister(mapping); } @@ -222,10 +195,23 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap for (Map.Entry entry : methods.entrySet()) { Method invocableMethod = AopUtils.selectInvocableMethod(entry.getKey(), userType); T mapping = entry.getValue(); - registerMapping(mapping, handler, invocableMethod); + registerHandlerMethod(handler, invocableMethod, mapping); } } + /** + * Register a handler method and its unique mapping. Invoked at startup for + * each detected handler method. + * @param handler the bean name of the handler or the handler instance + * @param method the method to register + * @param mapping the mapping conditions associated with the handler method + * @throws IllegalStateException if another method was already registered + * under the same mapping + */ + protected void registerHandlerMethod(Object handler, Method method, T mapping) { + this.mappingRegistry.register(mapping, handler, method); + } + /** * Create the HandlerMethod instance. * @param handler either a bean name or an actual handler instance @@ -272,30 +258,36 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap if (logger.isDebugEnabled()) { logger.debug("Looking up handler method for path " + lookupPath); } + this.mappingRegistry.acquireReadLock(); - // Ensure form data is parsed for "params" conditions... - return exchange.getRequestParams() - .then(() -> { - HandlerMethod handlerMethod = null; - try { - handlerMethod = lookupHandlerMethod(lookupPath, exchange); - } - catch (Exception ex) { - return Mono.error(ex); - } - if (logger.isDebugEnabled()) { - if (handlerMethod != null) { - logger.debug("Returning handler method [" + handlerMethod + "]"); + try { + // Ensure form data is parsed for "params" conditions... + return exchange.getRequestParams() + .then(() -> { + HandlerMethod handlerMethod = null; + try { + handlerMethod = lookupHandlerMethod(lookupPath, exchange); } - else { - logger.debug("Did not find handler method for [" + lookupPath + "]"); + catch (Exception ex) { + return Mono.error(ex); } - } - if (handlerMethod != null) { - handlerMethod = handlerMethod.createWithResolvedBean(); - } - return Mono.justOrEmpty(handlerMethod); - }); + if (logger.isDebugEnabled()) { + if (handlerMethod != null) { + logger.debug("Returning handler method [" + handlerMethod + "]"); + } + else { + logger.debug("Did not find handler method for [" + lookupPath + "]"); + } + } + if (handlerMethod != null) { + handlerMethod = handlerMethod.createWithResolvedBean(); + } + return Mono.justOrEmpty(handlerMethod); + }); + } + finally { + this.mappingRegistry.releaseReadLock(); + } } /** @@ -304,17 +296,25 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap * @param lookupPath mapping lookup path within the current servlet 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) */ protected HandlerMethod lookupHandlerMethod(String lookupPath, ServerWebExchange exchange) throws Exception { - List matches = findMatchingMappings(lookupPath, exchange); + 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); + } if (!matches.isEmpty()) { Comparator comparator = new MatchComparator(getMappingComparator(exchange)); - matches.sort(comparator); + Collections.sort(matches, comparator); if (logger.isTraceEnabled()) { logger.trace("Found " + matches.size() + " matching mapping(s) for [" + lookupPath + "] : " + matches); @@ -340,27 +340,13 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap } } - private List findMatchingMappings(String lookupPath, ServerWebExchange exchange) { - List matches = new ArrayList<>(); - try { - this.readWriteLock.readLock().lock(); - // Fast lookup of potential matching mappings given the current lookup path - Set matchingPatterns = getPatternRegistry().findMatches(lookupPath); - Set mappings = matchingPatterns.stream() - .flatMap(pattern -> this.mappingLookup.get(pattern).stream()) - .collect(Collectors.toSet()); - - for (T mapping : mappings) { - T match = getMatchingMapping(mapping, exchange); - if (match != null) { - matches.add(new Match(match, this.mappingRegistry.getMappings().get(mapping))); - } + private void addMatchingMappings(Collection mappings, List matches, ServerWebExchange exchange) { + for (T mapping : mappings) { + T match = getMatchingMapping(mapping, exchange); + if (match != null) { + matches.add(new Match(match, this.mappingRegistry.getMappings().get(mapping))); } } - finally { - this.readWriteLock.readLock().unlock(); - } - return matches; } /** @@ -445,7 +431,7 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap /** * A registry that maintains all mappings to handler methods, exposing methods - * to perform lookups. + * to perform lookups and providing concurrent access. * *

Package-private for testing purposes. */ @@ -453,15 +439,28 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap private final Map> registry = new HashMap<>(); - private final Map handlerMethodLookup = new LinkedHashMap<>(); + private final Map mappingLookup = new LinkedHashMap<>(); + + private final MultiValueMap urlLookup = new LinkedMultiValueMap<>(); private final Map corsLookup = new ConcurrentHashMap<>(); + private final ReentrantReadWriteLock readWriteLock = new ReentrantReadWriteLock(); + /** * Return all mappings and handler methods. Not thread-safe. + * @see #acquireReadLock() */ public Map getMappings() { - return this.handlerMethodLookup; + return this.mappingLookup; + } + + /** + * Return matches for the given URL path. Not thread-safe. + * @see #acquireReadLock() + */ + public List getMappingsByUrl(String urlPath) { + return this.urlLookup.get(urlPath); } /** @@ -472,42 +471,92 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap return this.corsLookup.get(original != null ? original : handlerMethod); } + /** + * Acquire the read lock when using getMappings and getMappingsByUrl. + */ + public void acquireReadLock() { + this.readWriteLock.readLock().lock(); + } + + /** + * Release the read lock after using getMappings and getMappingsByUrl. + */ + public void releaseReadLock() { + this.readWriteLock.readLock().unlock(); + } + public void register(T mapping, Object handler, Method method) { - HandlerMethod handlerMethod = createHandlerMethod(handler, method); - assertUniqueMethodMapping(handlerMethod, mapping); + this.readWriteLock.writeLock().lock(); + try { + HandlerMethod handlerMethod = createHandlerMethod(handler, method); + assertUniqueMethodMapping(handlerMethod, mapping); - if (logger.isInfoEnabled()) { - logger.info("Mapped \"" + mapping + "\" onto " + handlerMethod); - } - this.handlerMethodLookup.put(mapping, handlerMethod); + if (logger.isInfoEnabled()) { + logger.info("Mapped \"" + mapping + "\" onto " + handlerMethod); + } + this.mappingLookup.put(mapping, handlerMethod); - CorsConfiguration corsConfig = initCorsConfiguration(handler, method, mapping); - if (corsConfig != null) { - this.corsLookup.put(handlerMethod, corsConfig); - } + List directUrls = getDirectUrls(mapping); + for (String url : directUrls) { + this.urlLookup.add(url, mapping); + } - this.registry.put(mapping, new MappingRegistration<>(mapping, handlerMethod)); + CorsConfiguration corsConfig = initCorsConfiguration(handler, method, mapping); + if (corsConfig != null) { + this.corsLookup.put(handlerMethod, corsConfig); + } + + this.registry.put(mapping, new MappingRegistration<>(mapping, handlerMethod, directUrls)); + } + finally { + this.readWriteLock.writeLock().unlock(); + } } private void assertUniqueMethodMapping(HandlerMethod newHandlerMethod, T mapping) { - HandlerMethod handlerMethod = this.handlerMethodLookup.get(mapping); + 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."); } } - public void unregister(T mapping) { - MappingRegistration definition = this.registry.remove(mapping); - if (definition == null) { - return; + 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 { + MappingRegistration definition = this.registry.remove(mapping); + if (definition == null) { + return; + } - this.handlerMethodLookup.remove(definition.getMapping()); + this.mappingLookup.remove(definition.getMapping()); - this.corsLookup.remove(definition.getHandlerMethod()); + 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 { + this.readWriteLock.writeLock().unlock(); + } } } @@ -518,11 +567,14 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap private final HandlerMethod handlerMethod; - public MappingRegistration(T mapping, HandlerMethod handlerMethod) { + private final List directUrls; + + public MappingRegistration(T mapping, HandlerMethod handlerMethod, List directUrls) { 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() { @@ -532,6 +584,10 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap public HandlerMethod getHandlerMethod() { 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 c9da7fddb60..6dfe57170aa 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2016 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. @@ -18,6 +18,7 @@ package org.springframework.web.reactive.result.method; import java.util.Set; +import org.springframework.util.PathMatcher; import org.springframework.util.StringUtils; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.reactive.accept.MappingContentTypeResolver; @@ -32,7 +33,6 @@ 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.server.support.HttpRequestPathHelper; -import org.springframework.web.util.patterns.PathPatternRegistry; /** * Encapsulates the following request mapping conditions: @@ -471,15 +471,10 @@ public final class RequestMappingInfo implements RequestCondition fileExtensions = ((MappingContentTypeResolver) resolver).getKeys(); - this.pathPatternRegistry.setFileExtensions(fileExtensions); - } + /** + * Set a custom PathMatcher to use for the PatternsRequestCondition. + *

By default this is not set. + */ + public void setPathMatcher(PathMatcher pathMatcher) { + this.pathMatcher = pathMatcher; + } - } - } - return this.pathPatternRegistry; + 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; } /** @@ -547,6 +565,7 @@ public final class RequestMappingInfo implements RequestConditionBy default, a new instance of {@link PathPatternRegistry} with - * {@link PathPatternRegistry#setUseTrailingSlashMatch(boolean)} set to {@code true} + * Return the file extensions to use for suffix pattern matching. If + * {@code registeredSuffixPatternMatch=true}, the extensions are obtained + * from the configured {@code contentTypeResolver}. */ - public void setPathPatternRegistry(PathPatternRegistry pathPatternRegistry) { - this.pathPatternRegistry = pathPatternRegistry; + public Set getFileExtensions() { + RequestedContentTypeResolver resolver = getContentTypeResolver(); + if (useRegisteredSuffixPatternMatch() && resolver != null) { + if (resolver instanceof MappingContentTypeResolver) { + return ((MappingContentTypeResolver) resolver).getKeys(); + } + } + return null; } /** 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 629772fccba..f445aeaa509 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2016 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. @@ -26,7 +26,6 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Set; -import java.util.SortedSet; import java.util.StringTokenizer; import java.util.stream.Collectors; @@ -46,7 +45,6 @@ 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.patterns.PathPattern; /** * Abstract base class for classes for which {@link RequestMappingInfo} defines @@ -75,9 +73,7 @@ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMe */ @Override protected Set getMappingPathPatterns(RequestMappingInfo info) { - return info.getPatternsCondition().getPatterns().stream() - .map(p -> p.getPatternString()) - .collect(Collectors.toSet()); + return info.getPatternsCondition().getPatterns(); } /** @@ -109,22 +105,23 @@ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMe protected void handleMatch(RequestMappingInfo info, String lookupPath, ServerWebExchange exchange) { super.handleMatch(info, lookupPath, exchange); + String bestPattern; Map uriVariables; Map decodedUriVariables; - SortedSet patterns = info.getPatternsCondition().getMatchingPatterns(lookupPath); + Set patterns = info.getPatternsCondition().getPatterns(); if (patterns.isEmpty()) { + bestPattern = lookupPath; uriVariables = Collections.emptyMap(); decodedUriVariables = Collections.emptyMap(); - exchange.getAttributes().put(BEST_MATCHING_PATTERN_ATTRIBUTE, - getPatternRegistry().parsePattern(lookupPath)); } else { - PathPattern bestPattern = patterns.first(); - uriVariables = bestPattern.matchAndExtract(lookupPath); + bestPattern = patterns.iterator().next(); + uriVariables = getPathMatcher().extractUriTemplateVariables(bestPattern, lookupPath); decodedUriVariables = getPathHelper().decodePathVariables(exchange, uriVariables); - exchange.getAttributes().put(BEST_MATCHING_PATTERN_ATTRIBUTE, bestPattern); } + + exchange.getAttributes().put(BEST_MATCHING_PATTERN_ATTRIBUTE, bestPattern); exchange.getAttributes().put(URI_TEMPLATE_VARIABLES_ATTRIBUTE, decodedUriVariables); Map> matrixVars = extractMatrixVariables(exchange, uriVariables); 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 f275ffa265d..539d54fe754 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2016 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. @@ -18,6 +18,7 @@ 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; @@ -47,12 +48,53 @@ 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; 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. @@ -71,11 +113,37 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi public void afterPropertiesSet() { this.config = new RequestMappingInfo.BuilderConfiguration(); this.config.setPathHelper(getPathHelper()); + this.config.setPathMatcher(getPathMatcher()); + this.config.setSuffixPatternMatch(this.useSuffixPatternMatch); + this.config.setTrailingSlashMatch(this.useTrailingSlashMatch); + this.config.setRegisteredSuffixPatternMatch(this.useRegisteredSuffixPatternMatch); 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}. */ @@ -83,7 +151,14 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi return this.contentTypeResolver; } - + /** + * Return the file extensions to use for suffix pattern matching. + */ + 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/WebFluxConfigurationSupportTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/config/WebFluxConfigurationSupportTests.java index 8b5b6b51dc0..cd03137c0eb 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 @@ -19,10 +19,8 @@ package org.springframework.web.reactive.config; import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; - import javax.xml.bind.annotation.XmlRootElement; -import org.hamcrest.Matchers; import org.jetbrains.annotations.NotNull; import org.junit.Before; import org.junit.Test; @@ -53,7 +51,6 @@ 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.accept.RequestedContentTypeResolverBuilder; import org.springframework.web.reactive.handler.AbstractHandlerMapping; import org.springframework.web.reactive.handler.SimpleUrlHandlerMapping; import org.springframework.web.reactive.result.method.annotation.RequestMappingHandlerAdapter; @@ -73,7 +70,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.springframework.http.MediaType.APPLICATION_JSON; import static org.springframework.http.MediaType.APPLICATION_OCTET_STREAM; @@ -106,9 +102,9 @@ public class WebFluxConfigurationSupportTests { assertEquals(0, mapping.getOrder()); - assertFalse(mapping.getPatternRegistry().useSuffixPatternMatch()); - assertThat(mapping.getPatternRegistry().getFileExtensions(), Matchers.empty()); - assertTrue(mapping.getPatternRegistry().useTrailingSlashMatch()); + assertTrue(mapping.useSuffixPatternMatch()); + assertTrue(mapping.useTrailingSlashMatch()); + assertTrue(mapping.useRegisteredSuffixPatternMatch()); name = "webFluxContentTypeResolver"; RequestedContentTypeResolver resolver = context.getBean(name, RequestedContentTypeResolver.class); @@ -130,9 +126,8 @@ public class WebFluxConfigurationSupportTests { RequestMappingHandlerMapping mapping = context.getBean(name, RequestMappingHandlerMapping.class); assertNotNull(mapping); - assertFalse(mapping.getPatternRegistry().useTrailingSlashMatch()); - assertTrue(mapping.getPatternRegistry().useSuffixPatternMatch()); - assertThat(mapping.getPatternRegistry().getFileExtensions(), Matchers.contains(".json", ".xml")); + assertFalse(mapping.useSuffixPatternMatch()); + assertFalse(mapping.useTrailingSlashMatch()); } @Test @@ -267,6 +262,7 @@ public class WebFluxConfigurationSupportTests { assertEquals(Ordered.LOWEST_PRECEDENCE - 1, handlerMapping.getOrder()); assertNotNull(handlerMapping.getPathHelper()); + assertNotNull(handlerMapping.getPathMatcher()); SimpleUrlHandlerMapping urlHandlerMapping = (SimpleUrlHandlerMapping) handlerMapping; WebHandler webHandler = (WebHandler) urlHandlerMapping.getUrlMap().get("/images/**"); @@ -311,15 +307,8 @@ public class WebFluxConfigurationSupportTests { @Override public void configurePathMatching(PathMatchConfigurer configurer) { + configurer.setUseSuffixPatternMatch(false); configurer.setUseTrailingSlashMatch(false); - configurer.setUseSuffixPatternMatch(true); - configurer.setUseRegisteredSuffixPatternMatch(true); - } - - @Override - protected void configureContentTypeResolver(RequestedContentTypeResolverBuilder builder) { - builder.mediaType("json", MediaType.APPLICATION_JSON); - builder.mediaType("xml", MediaType.APPLICATION_XML); } } 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 cc46d1c58b6..4514c678e26 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2016 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. @@ -55,12 +55,11 @@ public class SimpleUrlHandlerMappingTests { Object mainController = wac.getBean("mainController"); Object otherController = wac.getBean("otherController"); - // TODO: direct matches have been removed, path within mapping is indeed "" - testUrl("/welcome.html", mainController, handlerMapping, ""); + testUrl("/welcome.html", mainController, handlerMapping, "/welcome.html"); testUrl("/welcome.x", otherController, handlerMapping, "welcome.x"); testUrl("/welcome/", otherController, handlerMapping, "welcome"); - testUrl("/show.html", mainController, handlerMapping, ""); - testUrl("/bookseats.html", mainController, handlerMapping, ""); + testUrl("/show.html", mainController, handlerMapping, "/show.html"); + testUrl("/bookseats.html", mainController, handlerMapping, "/bookseats.html"); } @Test @@ -75,10 +74,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, ""); + testUrl("/administrator/pathmatching.html", mainController, handlerMapping, "/administrator/pathmatching.html"); testUrl("/administrator/test/pathmatching.html", mainController, handlerMapping, "test/pathmatching.html"); testUrl("/administratort/pathmatching.html", null, handlerMapping, null); - testUrl("/administrator/another/bla.xml", mainController, handlerMapping, ""); + testUrl("/administrator/another/bla.xml", mainController, handlerMapping, "/administrator/another/bla.xml"); testUrl("/administrator/another/bla.gif", null, handlerMapping, null); testUrl("/administrator/test/testlastbit", mainController, handlerMapping, "test/testlastbit"); testUrl("/administrator/test/testla", null, handlerMapping, null); @@ -90,7 +89,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, ""); + testUrl("/show123.html", mainController, handlerMapping, "/show123.html"); 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); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/CssLinkResourceTransformerTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/CssLinkResourceTransformerTests.java index 78cdb67e543..e3239f1ffe7 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/CssLinkResourceTransformerTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/CssLinkResourceTransformerTests.java @@ -39,7 +39,6 @@ import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse import org.springframework.util.StringUtils; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.adapter.DefaultServerWebExchange; -import org.springframework.web.util.patterns.PathPatternParser; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertSame; 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 d1063c05312..8e841ce0045 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2016 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. @@ -36,7 +36,8 @@ import org.springframework.web.context.support.AnnotationConfigWebApplicationCon import org.springframework.web.reactive.handler.SimpleUrlHandlerMapping; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.adapter.DefaultServerWebExchange; -import org.springframework.web.util.patterns.PathPatternParser; +import org.springframework.web.server.session.DefaultWebSessionManager; +import org.springframework.web.server.session.WebSessionManager; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -136,8 +137,7 @@ public class ResourceUrlProviderTests { context.refresh(); ResourceUrlProvider urlProviderBean = context.getBean(ResourceUrlProvider.class); - assertThat(urlProviderBean.getHandlerMap(), - Matchers.hasKey(new PathPatternParser().parse("/resources/**"))); + assertThat(urlProviderBean.getHandlerMap(), Matchers.hasKey("/resources/**")); assertFalse(urlProviderBean.isAutodetect()); } 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 9d1c547bfc6..f02a6f9603c 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2016 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. @@ -18,7 +18,6 @@ package org.springframework.web.reactive.result.condition; import java.net.URISyntaxException; import java.util.Collections; -import java.util.Iterator; import java.util.Set; import org.junit.Test; @@ -28,8 +27,6 @@ import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.adapter.DefaultServerWebExchange; -import org.springframework.web.util.patterns.PathPattern; -import org.springframework.web.util.patterns.PathPatternRegistry; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -45,14 +42,13 @@ public class PatternsRequestConditionTests { @Test public void prependSlash() { PatternsRequestCondition c = new PatternsRequestCondition("foo"); - assertEquals("/foo", c.getPatterns().iterator().next().getPatternString()); + assertEquals("/foo", c.getPatterns().iterator().next()); } @Test public void prependNonEmptyPatternsOnly() { PatternsRequestCondition c = new PatternsRequestCondition(""); - assertEquals("Do not prepend empty patterns (SPR-8255)", "", - c.getPatterns().iterator().next().getPatternString()); + assertEquals("Do not prepend empty patterns (SPR-8255)", "", c.getPatterns().iterator().next()); } @Test @@ -117,14 +113,13 @@ public class PatternsRequestConditionTests { PatternsRequestCondition match = condition.getMatchingCondition(exchange); assertNotNull(match); - assertEquals("/{foo}", match.getPatterns().iterator().next().getPatternString()); + assertEquals("/{foo}.*", match.getPatterns().iterator().next()); - condition = new PatternsRequestCondition(new String[] {"/foo"}, null, - createPatternRegistry(true, false, null)); + condition = new PatternsRequestCondition(new String[] {"/{foo}"}, null, null, false, false, null); match = condition.getMatchingCondition(exchange); assertNotNull(match); - assertEquals("/foo.*", match.getPatterns().iterator().next().getPatternString()); + assertEquals("/{foo}", match.getPatterns().iterator().next()); } // SPR-8410 @@ -133,33 +128,28 @@ public class PatternsRequestConditionTests { public void matchSuffixPatternUsingFileExtensions() throws Exception { String[] patterns = new String[] {"/jobs/{jobName}"}; Set extensions = Collections.singleton("json"); - PatternsRequestCondition condition = new PatternsRequestCondition(patterns, null, - createPatternRegistry(true, false, extensions)); + PatternsRequestCondition condition = new PatternsRequestCondition(patterns, null, null, true, false, extensions); ServerWebExchange exchange = createExchange("/jobs/my.job"); PatternsRequestCondition match = condition.getMatchingCondition(exchange); assertNotNull(match); - assertEquals("/jobs/{jobName}", match.getPatterns().iterator().next().getPatternString()); + assertEquals("/jobs/{jobName}", match.getPatterns().iterator().next()); exchange = createExchange("/jobs/my.job.json"); match = condition.getMatchingCondition(exchange); assertNotNull(match); - Iterator matchedPatterns = match.getPatterns().iterator(); - assertEquals("/jobs/{jobName}", matchedPatterns.next().getPatternString()); - assertEquals("/jobs/{jobName}.json", matchedPatterns.next().getPatternString()); + assertEquals("/jobs/{jobName}.json", match.getPatterns().iterator().next()); } @Test public void matchSuffixPatternUsingFileExtensions2() throws Exception { PatternsRequestCondition condition1 = new PatternsRequestCondition( - new String[] {"/prefix"}, null, - createPatternRegistry(true, false, Collections.singleton("json"))); + new String[] {"/prefix"}, null, null, true, false, Collections.singleton("json")); PatternsRequestCondition condition2 = new PatternsRequestCondition( - new String[] {"/suffix"}, null, - createPatternRegistry(true, false, null)); + new String[] {"/suffix"}, null, null, true, false, null); PatternsRequestCondition combined = condition1.combine(condition2); @@ -176,22 +166,20 @@ public class PatternsRequestConditionTests { PatternsRequestCondition condition = new PatternsRequestCondition("/foo"); PatternsRequestCondition match = condition.getMatchingCondition(exchange); - assertNull("Should not match by default", match); + assertNotNull(match); + assertEquals("Should match by default", "/foo/", match.getPatterns().iterator().next()); - condition = new PatternsRequestCondition(new String[] {"/foo"}, null, - createPatternRegistry(false, true, null)); + condition = new PatternsRequestCondition(new String[] {"/foo"}, null, null, false, true, 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().getPatternString()); + "/foo/", match.getPatterns().iterator().next()); - condition = new PatternsRequestCondition(new String[] {"/foo"}, null, - createPatternRegistry(true, true, null)); + condition = new PatternsRequestCondition(new String[] {"/foo"}, null, null, false, false, null); match = condition.getMatchingCondition(exchange); - assertNotNull(match); - assertEquals("/foo/", match.getPatterns().iterator().next().getPatternString()); + assertNull(match); } @Test @@ -228,8 +216,8 @@ public class PatternsRequestConditionTests { PatternsRequestCondition match1 = c1.getMatchingCondition(exchange); PatternsRequestCondition match2 = c2.getMatchingCondition(exchange); - assertNull(match1); - assertEquals("/*.html", match2.getPatterns().iterator().next().getPatternString()); + assertNotNull(match1); + assertEquals(1, match1.compareTo(match2, exchange)); } @@ -238,13 +226,4 @@ public class PatternsRequestConditionTests { return new DefaultServerWebExchange(request, new MockServerHttpResponse()); } - private PathPatternRegistry createPatternRegistry(boolean useSuffixPatternMatch, boolean useTrailingSlashMatch, - Set extensions) { - PathPatternRegistry registry = new PathPatternRegistry(); - registry.setUseSuffixPatternMatch(useSuffixPatternMatch); - registry.setUseTrailingSlashMatch(useTrailingSlashMatch); - registry.setFileExtensions(extensions); - return registry; - } - } 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 2db4cdc2f4e..f28d1a23bf2 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2016 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. @@ -20,9 +20,8 @@ import java.lang.reflect.Method; import java.net.URISyntaxException; import java.util.Collections; import java.util.Comparator; +import java.util.List; import java.util.Set; -import java.util.SortedSet; -import java.util.TreeSet; import org.junit.Before; import org.junit.Test; @@ -34,16 +33,14 @@ import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse; import org.springframework.stereotype.Controller; +import org.springframework.util.PathMatcher; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.method.HandlerMethod; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.adapter.DefaultServerWebExchange; import org.springframework.web.server.session.MockWebSessionManager; import org.springframework.web.server.session.WebSessionManager; -import org.springframework.web.util.patterns.PathPattern; -import org.springframework.web.util.patterns.PathPatternComparator; -import org.springframework.web.util.patterns.PathPatternParser; -import org.springframework.web.util.patterns.PatternComparatorConsideringPath; +import org.springframework.web.util.ParsingPathMatcher; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -55,8 +52,6 @@ import static org.junit.Assert.assertNull; */ public class HandlerMethodMappingTests { - private PathPatternParser patternParser = new PathPatternParser(); - private AbstractHandlerMethodMapping mapping; private MyHandler handler; @@ -77,13 +72,13 @@ public class HandlerMethodMappingTests { @Test(expected = IllegalStateException.class) public void registerDuplicates() { - this.mapping.registerMapping("/foo", this.handler, this.method1); - this.mapping.registerMapping("/foo", this.handler, this.method2); + this.mapping.registerMapping("foo", this.handler, this.method1); + this.mapping.registerMapping("foo", this.handler, this.method2); } @Test public void directMatch() throws Exception { - String key = "/foo"; + String key = "foo"; this.mapping.registerMapping(key, this.handler, this.method1); Mono result = this.mapping.getHandler(createExchange(HttpMethod.GET, key)); @@ -115,26 +110,32 @@ public class HandlerMethodMappingTests { this.mapping.registerMapping(key1, this.handler, this.method1); this.mapping.registerMapping(key2, this.handler, this.method2); - HandlerMethod match = this.mapping.getMappingRegistry().getMappings().get(key1); - assertNotNull(match); + List directUrlMatches = this.mapping.getMappingRegistry().getMappingsByUrl(key1); + + assertNotNull(directUrlMatches); + assertEquals(1, directUrlMatches.size()); + assertEquals(key1, directUrlMatches.get(0)); } @Test public void registerMappingWithSameMethodAndTwoHandlerInstances() throws Exception { - String key1 = "/foo"; - String key2 = "/bar"; + String key1 = "foo"; + String key2 = "bar"; MyHandler handler1 = new MyHandler(); MyHandler handler2 = new MyHandler(); this.mapping.registerMapping(key1, handler1, this.method1); this.mapping.registerMapping(key2, handler2, this.method1); - HandlerMethod match = this.mapping.getMappingRegistry().getMappings().get(key1); - assertNotNull(match); + List directUrlMatches = this.mapping.getMappingRegistry().getMappingsByUrl(key1); + + assertNotNull(directUrlMatches); + assertEquals(1, directUrlMatches.size()); + assertEquals(key1, directUrlMatches.get(0)); } @Test public void unregisterMapping() throws Exception { - String key = "/foo"; + String key = "foo"; this.mapping.registerMapping(key, this.handler, this.method1); Mono result = this.mapping.getHandler(createExchange(HttpMethod.GET, key)); @@ -144,7 +145,7 @@ public class HandlerMethodMappingTests { result = this.mapping.getHandler(createExchange(HttpMethod.GET, key)); assertNull(result.block()); - assertNull(this.mapping.getMappingRegistry().getMappings().get(key)); + assertNull(this.mapping.getMappingRegistry().getMappingsByUrl(key)); } @@ -157,7 +158,7 @@ public class HandlerMethodMappingTests { private static class MyHandlerMethodMapping extends AbstractHandlerMethodMapping { - private PathPatternParser patternParser = new PathPatternParser(); + private PathMatcher pathMatcher = new ParsingPathMatcher(); @Override protected boolean isHandler(Class beanType) { @@ -172,27 +173,19 @@ public class HandlerMethodMappingTests { @Override protected Set getMappingPathPatterns(String key) { - return Collections.singleton(key); + return (this.pathMatcher.isPattern(key) ? Collections.emptySet() : Collections.singleton(key)); } @Override protected String getMatchingMapping(String pattern, ServerWebExchange exchange) { String lookupPath = exchange.getRequest().getURI().getPath(); - PathPattern pathPattern = this.patternParser.parse(pattern); - return (pathPattern.matches(lookupPath) ? pattern : null); + return (this.pathMatcher.match(pattern, lookupPath) ? pattern : null); } @Override protected Comparator getMappingComparator(ServerWebExchange exchange) { String lookupPath = exchange.getRequest().getURI().getPath(); - PatternComparatorConsideringPath comparator = new PatternComparatorConsideringPath(lookupPath); - return new Comparator() { - @Override - public int compare(String o1, String o2) { - - return comparator.compare(patternParser.parse(o1), patternParser.parse(o2)); - } - }; + return this.pathMatcher.getPatternComparator(lookupPath); } } 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 0adf6a7a9b8..3147d015f93 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2016 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. @@ -26,9 +26,7 @@ import java.util.Optional; import java.util.Set; import java.util.function.Consumer; -import org.hamcrest.Matchers; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; @@ -61,8 +59,6 @@ import org.springframework.web.server.ServerWebInputException; import org.springframework.web.server.UnsupportedMediaTypeStatusException; import org.springframework.web.server.adapter.DefaultServerWebExchange; import org.springframework.web.server.support.HttpRequestPathHelper; -import org.springframework.web.util.patterns.PathPattern; -import org.springframework.web.util.patterns.PathPatternRegistry; import static org.hamcrest.CoreMatchers.containsString; import static org.junit.Assert.assertEquals; @@ -85,6 +81,7 @@ public class RequestMappingInfoHandlerMappingTests { private ServerHttpRequest request; + @Before public void setUp() throws Exception { this.handlerMapping = new TestRequestMappingInfoHandlerMapping(); @@ -98,8 +95,7 @@ public class RequestMappingInfoHandlerMappingTests { RequestMappingInfo info = paths(patterns).build(); Set actual = this.handlerMapping.getMappingPathPatterns(info); - assertThat(actual, Matchers.containsInAnyOrder("/foo/*", "/foo", "/bar/*", "/bar", - "/foo/*/", "/foo/", "/bar/*/", "/bar/")); + assertEquals(new HashSet<>(Arrays.asList(patterns)), actual); } @Test @@ -127,9 +123,6 @@ public class RequestMappingInfoHandlerMappingTests { } @Test - @Ignore - // TODO: for "" patterns, should we generate the "/" variant (and break SPR-8255) - // or handle matching in a different way? Here, setTrailingSlashMatch is set to false for tests public void getHandlerEmptyPathMatch() throws Exception { String[] patterns = new String[] {""}; Method expected = resolveMethod(new TestController(), patterns, null, null); @@ -189,7 +182,7 @@ public class RequestMappingInfoHandlerMappingTests { assertError(mono, UnsupportedMediaTypeStatusException.class, ex -> assertEquals("Request failure [status: 415, " + - "reason: \"Invalid mime type \"bogus\": does not contain '/'\"]", + "reason: \"Invalid mime type \"bogus\": does not contain '/'\"]", ex.getMessage())); } @@ -235,8 +228,7 @@ public class RequestMappingInfoHandlerMappingTests { exchange.getAttributes().get(name)); } - @Test - @SuppressWarnings("unchecked") + @Test @SuppressWarnings("unchecked") public void handleMatchUriTemplateVariables() throws Exception { String lookupPath = "/1/2"; this.request = MockServerHttpRequest.get(lookupPath).build(); @@ -282,9 +274,7 @@ public class RequestMappingInfoHandlerMappingTests { ServerWebExchange exchange = createExchange(); this.handlerMapping.handleMatch(key, "/1/2", exchange); - PathPattern pattern = (PathPattern) exchange.getAttributes() - .get(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE); - assertEquals("/{path1}/2", pattern.getPatternString()); + assertEquals("/{path1}/2", exchange.getAttributes().get(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE)); } @Test @@ -295,9 +285,7 @@ public class RequestMappingInfoHandlerMappingTests { this.handlerMapping.handleMatch(key, "/1/2", exchange); - PathPattern pattern = (PathPattern) exchange.getAttributes() - .get(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE); - assertEquals("/1/2", pattern.getPatternString()); + assertEquals("/1/2", exchange.getAttributes().get(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE)); } @Test @@ -368,7 +356,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 -> { @@ -467,11 +455,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() { } @@ -479,31 +467,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 ""; } @@ -544,12 +532,11 @@ public class RequestMappingInfoHandlerMappingTests { protected RequestMappingInfo getMappingForMethod(Method method, Class handlerType) { RequestMapping annot = AnnotatedElementUtils.findMergedAnnotation(method, RequestMapping.class); if (annot != null) { - PathPatternRegistry pathPatternRegistry = new PathPatternRegistry(); - pathPatternRegistry.setUseSuffixPatternMatch(true); - pathPatternRegistry.setUseTrailingSlashMatch(true); BuilderConfiguration options = new BuilderConfiguration(); options.setPathHelper(getPathHelper()); - options.setPathPatternRegistry(pathPatternRegistry); + options.setPathMatcher(getPathMatcher()); + options.setSuffixPatternMatch(true); + options.setTrailingSlashMatch(true); 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 1cd293b51cf..6f07210efbc 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2016 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. @@ -22,10 +22,10 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import java.lang.reflect.Method; import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; import java.util.Set; -import java.util.stream.Collectors; -import org.hamcrest.Matchers; import org.junit.Before; import org.junit.Test; @@ -40,12 +40,16 @@ import org.springframework.web.bind.annotation.PutMapping; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.context.support.StaticWebApplicationContext; +import org.springframework.web.reactive.accept.MappingContentTypeResolver; import org.springframework.web.reactive.result.method.RequestMappingInfo; -import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; /** * Unit tests for {@link RequestMappingHandlerMapping}. @@ -64,6 +68,64 @@ public class RequestMappingHandlerMappingTests { this.handlerMapping.setApplicationContext(wac); } + + @Test + public void useRegisteredSuffixPatternMatch() { + assertTrue(this.handlerMapping.useSuffixPatternMatch()); + assertTrue(this.handlerMapping.useRegisteredSuffixPatternMatch()); + + MappingContentTypeResolver contentTypeResolver = mock(MappingContentTypeResolver.class); + when(contentTypeResolver.getKeys()).thenReturn(Collections.singleton("json")); + + this.handlerMapping.setContentTypeResolver(contentTypeResolver); + this.handlerMapping.afterPropertiesSet(); + + assertTrue(this.handlerMapping.useSuffixPatternMatch()); + assertTrue(this.handlerMapping.useRegisteredSuffixPatternMatch()); + assertEquals(Collections.singleton("json"), this.handlerMapping.getFileExtensions()); + } + + @Test + public void useRegisteredSuffixPatternMatchInitialization() { + MappingContentTypeResolver contentTypeResolver = mock(MappingContentTypeResolver.class); + when(contentTypeResolver.getKeys()).thenReturn(Collections.singleton("json")); + + final Set actualExtensions = new HashSet<>(); + RequestMappingHandlerMapping localHandlerMapping = new RequestMappingHandlerMapping() { + @Override + protected RequestMappingInfo getMappingForMethod(Method method, Class handlerType) { + actualExtensions.addAll(getFileExtensions()); + return super.getMappingForMethod(method, handlerType); + } + }; + this.wac.registerSingleton("testController", ComposedAnnotationController.class); + this.wac.refresh(); + + localHandlerMapping.setContentTypeResolver(contentTypeResolver); + localHandlerMapping.setUseRegisteredSuffixPatternMatch(true); + localHandlerMapping.setApplicationContext(this.wac); + localHandlerMapping.afterPropertiesSet(); + + assertEquals(Collections.singleton("json"), actualExtensions); + } + + @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( @@ -135,10 +197,9 @@ public class RequestMappingHandlerMappingTests { assertNotNull(info); - Set paths = info.getPatternsCondition().getPatterns() - .stream().map(p -> p.getPatternString()).collect(Collectors.toSet()); - assertEquals(2, paths.size()); - assertThat(paths, Matchers.containsInAnyOrder(path, path + "/")); + Set paths = info.getPatternsCondition().getPatterns(); + assertEquals(1, paths.size()); + assertEquals(path, paths.iterator().next()); Set methods = info.getMethodsCondition().getMethods(); assertEquals(1, methods.size());