From 09d18f2ef5f9d9ef254d060f1d189e5ce8ce7a9c Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Thu, 9 Feb 2017 15:56:15 +0100 Subject: [PATCH] Refactor HandlerMapping path match configuration Since the introduction of `PathPatternRegistry`, the various path match configuration flags are no longer needed in several places and that configuration can live in the registry itself. Issue: SPR-14544 --- .../util/patterns/PathPatternRegistry.java | 29 +++++-- .../patterns/PathPatternRegistryTests.java | 57 +++++++++----- .../reactive/config/PathMatchConfigurer.java | 41 +++------- .../config/WebFluxConfigurationSupport.java | 19 +++-- .../condition/PatternsRequestCondition.java | 42 ++-------- .../result/method/RequestMappingInfo.java | 70 ++++++++--------- .../RequestMappingHandlerMapping.java | 76 +------------------ .../WebFluxConfigurationSupportTests.java | 24 ++++-- .../PatternsRequestConditionTests.java | 29 +++++-- ...RequestMappingInfoHandlerMappingTests.java | 7 +- .../RequestMappingHandlerMappingTests.java | 63 --------------- 11 files changed, 163 insertions(+), 294 deletions(-) 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 index 4f6573ff832..0e04f510968 100644 --- 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 @@ -111,11 +111,14 @@ public class PathPatternRegistry { *

The default value is an empty {@code Set} */ public void setFileExtensions(Set fileExtensions) { - this.fileExtensions = 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, sorted according to their specificity. + * Return a (read-only) set of all patterns for matching (including generated pattern variants). */ public Set getPatterns() { return Collections.unmodifiableSet(this.patterns); @@ -194,28 +197,38 @@ public class PathPatternRegistry { * @return the list of {@link PathPattern} that were registered as a result */ public List register(String rawPattern) { + String fixedPattern = prependLeadingSlash(rawPattern); List newPatterns = new ArrayList<>(); - PathPattern pattern = this.pathPatternParser.parse(rawPattern); + PathPattern pattern = this.pathPatternParser.parse(fixedPattern); newPatterns.add(pattern); - if (StringUtils.hasLength(rawPattern) && !pattern.isCatchAll()) { + if (StringUtils.hasLength(fixedPattern) && !pattern.isCatchAll()) { if (this.useSuffixPatternMatch) { if (this.fileExtensions != null && !this.fileExtensions.isEmpty()) { for (String extension : this.fileExtensions) { - newPatterns.add(this.pathPatternParser.parse(rawPattern + "." + extension)); + newPatterns.add(this.pathPatternParser.parse(fixedPattern + extension)); } } else { - newPatterns.add(this.pathPatternParser.parse(rawPattern + ".*")); + newPatterns.add(this.pathPatternParser.parse(fixedPattern + ".*")); } } - if (this.useTrailingSlashMatch && !rawPattern.endsWith("/")) { - newPatterns.add(this.pathPatternParser.parse(rawPattern + "/")); + if (this.useTrailingSlashMatch && !fixedPattern.endsWith("/")) { + newPatterns.add(this.pathPatternParser.parse(fixedPattern + "/")); } } this.patterns.addAll(newPatterns); return newPatterns; } + private String prependLeadingSlash(String pattern) { + if (StringUtils.hasLength(pattern) && !pattern.startsWith("/")) { + return "/" + pattern; + } + else { + return pattern; + } + } + /** * Combine the patterns contained in the current registry * with the ones in the other, into a new {@code PathPatternRegistry} instance. 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 index 0c1eb4bcb1d..b72b850c88e 100644 --- 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 @@ -28,12 +28,14 @@ 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 { @@ -48,6 +50,21 @@ public class PathPatternRegistryTests { 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); @@ -58,58 +75,59 @@ public class PathPatternRegistryTests { @Test public void shouldNotRegisterPatternVariants() { List patterns = this.registry.register("/foo/{bar}"); - assertPathPatternListContains(patterns, "/foo/{bar}"); + assertThat(getPatternList(patterns), Matchers.containsInAnyOrder("/foo/{bar}")); } @Test public void shouldRegisterTrailingSlashVariants() { this.registry.setUseTrailingSlashMatch(true); List patterns = this.registry.register("/foo/{bar}"); - assertPathPatternListContains(patterns, "/foo/{bar}", "/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}"); - assertPathPatternListContains(patterns, "/foo/{bar}", "/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"); + fileExtensions.add(".json"); + fileExtensions.add(".xml"); this.registry.setUseSuffixPatternMatch(true); this.registry.setFileExtensions(fileExtensions); List patterns = this.registry.register("/foo/{bar}"); - assertPathPatternListContains(patterns, "/foo/{bar}", "/foo/{bar}.xml", "/foo/{bar}.json"); + 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"); + 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}"); - assertPathPatternListContains(patterns, "/foo/{bar}", - "/foo/{bar}.xml", "/foo/{bar}.json", "/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()); - assertPathPatternListContains(result.getPatterns(), ""); + assertThat(getPatternList(result.getPatterns()), Matchers.containsInAnyOrder("")); } @Test public void combineWithEmptyRegistry() { this.registry.register("/foo"); PathPatternRegistry result = this.registry.combine(new PathPatternRegistry()); - assertPathPatternListContains(result.getPatterns(), "/foo"); + assertThat(getPatternList(result.getPatterns()), Matchers.containsInAnyOrder("/foo")); } @Test @@ -119,7 +137,7 @@ public class PathPatternRegistryTests { other.register("/bar"); other.register("/baz"); PathPatternRegistry result = this.registry.combine(other); - assertPathPatternListContains(result.getPatterns(), "/foo/bar", "/foo/baz"); + assertThat(getPatternList(result.getPatterns()), Matchers.containsInAnyOrder("/foo/bar", "/foo/baz")); } @Test @@ -131,7 +149,7 @@ public class PathPatternRegistryTests { this.registry.add(fooOne); this.registry.add(fooTwo); Set matches = this.registry.findMatches("/foo"); - assertPathPatternListContains(matches, "/f?o", "/fo?"); + assertThat(getPatternList(matches), Matchers.contains("/f?o", "/fo?")); } @Test @@ -146,15 +164,14 @@ public class PathPatternRegistryTests { this.registry.register("/foo/bar/baz"); this.registry.register("/foo/bar/{baz}"); Set matches = this.registry.findMatches("/foo/bar/baz"); - assertPathPatternListContains(matches, "/foo/bar/baz", "/foo/bar/{baz}", - "/foo/{*baz}"); + assertThat(getPatternList(matches), Matchers.contains("/foo/bar/baz", "/foo/bar/{baz}", + "/foo/{*baz}")); } - private void assertPathPatternListContains(Collection parsedPatterns, String... pathPatterns) { - List patternList = parsedPatterns. - stream().map(pattern -> pattern.getPatternString()).collect(Collectors.toList()); - assertThat(patternList, Matchers.contains(pathPatterns)); + 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 2eda3dafea4..a24714601da 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-2016 the original author or authors. + * 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. @@ -16,7 +16,6 @@ package org.springframework.web.reactive.config; -import org.springframework.util.PathMatcher; import org.springframework.web.server.support.HttpRequestPathHelper; /** @@ -27,24 +26,22 @@ import org.springframework.web.server.support.HttpRequestPathHelper; */ public class PathMatchConfigurer { - private Boolean suffixPatternMatch; + private boolean suffixPatternMatch = false; - private Boolean trailingSlashMatch; + private boolean trailingSlashMatch = true; - private Boolean registeredSuffixPatternMatch; + private boolean registeredSuffixPatternMatch = false; 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 true}. + *

By default this is set to {@code false}. * @see #registeredSuffixPatternMatch */ - public PathMatchConfigurer setUseSuffixPatternMatch(Boolean suffixPatternMatch) { + public PathMatchConfigurer setUseSuffixPatternMatch(boolean suffixPatternMatch) { this.suffixPatternMatch = suffixPatternMatch; return this; } @@ -54,7 +51,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; } @@ -64,9 +61,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 "true". + *

By default this is set to "false". */ - public PathMatchConfigurer setUseRegisteredSuffixPatternMatch(Boolean registeredSuffixPatternMatch) { + public PathMatchConfigurer setUseRegisteredSuffixPatternMatch(boolean registeredSuffixPatternMatch) { this.registeredSuffixPatternMatch = registeredSuffixPatternMatch; return this; } @@ -80,24 +77,15 @@ public class PathMatchConfigurer { return this; } - /** - * 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() { + protected boolean isUseSuffixPatternMatch() { return this.suffixPatternMatch; } - protected Boolean isUseTrailingSlashMatch() { + protected boolean isUseTrailingSlashMatch() { return this.trailingSlashMatch; } - protected Boolean isUseRegisteredSuffixPatternMatch() { + protected boolean isUseRegisteredSuffixPatternMatch() { return this.registeredSuffixPatternMatch; } @@ -105,9 +93,4 @@ public class PathMatchConfigurer { return this.pathHelper; } - //TODO: remove - 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 42fbf804828..1b9790f8ee4 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,6 +81,7 @@ 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. @@ -136,25 +137,23 @@ public class WebFluxConfigurationSupport implements ApplicationContextAware { @Bean public RequestMappingHandlerMapping requestMappingHandlerMapping() { + CompositeContentTypeResolver contentTypeResolver = webFluxContentTypeResolver(); RequestMappingHandlerMapping mapping = createRequestMappingHandlerMapping(); mapping.setOrder(0); - mapping.setContentTypeResolver(webFluxContentTypeResolver()); + mapping.setContentTypeResolver(contentTypeResolver); mapping.setCorsConfigurations(getCorsConfigurations()); + PathPatternRegistry pathPatternRegistry = new PathPatternRegistry(); + mapping.setPatternRegistry(pathPatternRegistry); PathMatchConfigurer configurer = getPathMatchConfigurer(); - if (configurer.isUseSuffixPatternMatch() != null) { - mapping.setUseSuffixPatternMatch(configurer.isUseSuffixPatternMatch()); - } - if (configurer.isUseRegisteredSuffixPatternMatch() != null) { - mapping.setUseRegisteredSuffixPatternMatch(configurer.isUseRegisteredSuffixPatternMatch()); - } - if (configurer.isUseTrailingSlashMatch() != null) { - mapping.setUseTrailingSlashMatch(configurer.isUseTrailingSlashMatch()); + pathPatternRegistry.setUseSuffixPatternMatch(configurer.isUseSuffixPatternMatch()); + pathPatternRegistry.setUseTrailingSlashMatch(configurer.isUseTrailingSlashMatch()); + if (configurer.isUseRegisteredSuffixPatternMatch() && contentTypeResolver != null) { + pathPatternRegistry.setFileExtensions(contentTypeResolver.getKeys()); } if (configurer.getPathHelper() != null) { mapping.setPathHelper(configurer.getPathHelper()); } - return mapping; } 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 91a4fda0db3..413a48e967d 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 @@ -18,15 +18,11 @@ package org.springframework.web.reactive.result.condition; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.Comparator; import java.util.Set; import java.util.SortedSet; -import java.util.function.Function; -import java.util.stream.Collectors; 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; @@ -37,6 +33,7 @@ import org.springframework.web.util.patterns.PathPatternRegistry; * against a set of URL path patterns. * * @author Rossen Stoyanchev + * @author Brian Clozel * @since 5.0 */ public final class PatternsRequestCondition extends AbstractRequestCondition { @@ -51,7 +48,7 @@ public final class PatternsRequestCondition extends AbstractRequestCondition extensions) { - - this(createPatternSet(patterns, useSuffixPatternMatch, useTrailingSlashMatch, extensions), + PathPatternRegistry pathPatternRegistry) { + this(createPatternSet(patterns, pathPatternRegistry), (pathHelper != null ? pathHelper : new HttpRequestPathHelper())); } - private static PathPatternRegistry createPatternSet(String[] patterns,boolean useSuffixPatternMatch, - boolean useTrailingSlashMatch, Set extensions) { - Set fixedFileExtensions = (extensions != null) ? extensions.stream() - .map(ext -> (ext.charAt(0) != '.') ? "." + ext : ext) - .collect(Collectors.toSet()) : Collections.emptySet(); - PathPatternRegistry patternSet = new PathPatternRegistry(); - patternSet.setUseSuffixPatternMatch(useSuffixPatternMatch); - patternSet.setUseTrailingSlashMatch(useTrailingSlashMatch); - patternSet.setFileExtensions(extensions); + private static PathPatternRegistry createPatternSet(String[] patterns, PathPatternRegistry pathPatternRegistry) { + PathPatternRegistry patternSet = pathPatternRegistry != null ? pathPatternRegistry : new PathPatternRegistry(); if(patterns != null) { - Arrays.asList(patterns).stream() - .map(prependLeadingSlash()) - .forEach(p -> patternSet.register(p)); + Arrays.asList(patterns).stream().forEach(p -> patternSet.register(p)); } return patternSet; } - private static Function prependLeadingSlash() { - return pattern -> { - if (StringUtils.hasLength(pattern) && !pattern.startsWith("/")) { - return "/" + pattern; - } - else { - return pattern; - } - }; - } - private PatternsRequestCondition(PathPatternRegistry patternRegistry, HttpRequestPathHelper pathHelper) { this.patternRegistry = patternRegistry; this.pathHelper = pathHelper; 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 134561efd15..c9da7fddb60 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 @@ -32,6 +32,7 @@ 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: @@ -470,10 +471,15 @@ public final class RequestMappingInfo implements RequestConditionBy 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 PathPatternRegistry getPathPatternRegistry() { + if(this.pathPatternRegistry == null) { + this.pathPatternRegistry = new PathPatternRegistry(); + this.pathPatternRegistry.setUseTrailingSlashMatch(true); + } + if(this.registeredSuffixPatternMatch) { + RequestedContentTypeResolver resolver = getContentTypeResolver(); + if (resolver != null && resolver instanceof MappingContentTypeResolver) { + if (resolver instanceof MappingContentTypeResolver) { + Set fileExtensions = ((MappingContentTypeResolver) resolver).getKeys(); + this.pathPatternRegistry.setFileExtensions(fileExtensions); + } - public boolean useSuffixPatternMatch() { - return this.suffixPatternMatch; + } + } + return this.pathPatternRegistry; } /** @@ -550,7 +547,6 @@ public final class RequestMappingInfo implements RequestConditionBy default, a new instance of {@link PathPatternRegistry} with + * {@link PathPatternRegistry#setUseTrailingSlashMatch(boolean)} set to {@code true} */ - public Set getFileExtensions() { - RequestedContentTypeResolver resolver = getContentTypeResolver(); - if (useRegisteredSuffixPatternMatch() && resolver != null) { - if (resolver instanceof MappingContentTypeResolver) { - return ((MappingContentTypeResolver) resolver).getKeys(); - } - } - return null; + public void setPathPatternRegistry(PathPatternRegistry pathPatternRegistry) { + this.pathPatternRegistry = pathPatternRegistry; } /** 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 2d5bc1adb83..f275ffa265d 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMapping.java @@ -18,7 +18,6 @@ package org.springframework.web.reactive.result.method.annotation; import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Method; -import java.util.Set; import org.springframework.context.EmbeddedValueResolverAware; import org.springframework.core.annotation.AnnotatedElementUtils; @@ -48,53 +47,12 @@ import org.springframework.web.reactive.result.method.RequestMappingInfoHandlerM public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMapping implements EmbeddedValueResolverAware { - private boolean useSuffixPatternMatch = false; - - private boolean useRegisteredSuffixPatternMatch = false; - - 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. @@ -113,36 +71,11 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi public void afterPropertiesSet() { this.config = new RequestMappingInfo.BuilderConfiguration(); this.config.setPathHelper(getPathHelper()); - 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}. */ @@ -150,14 +83,7 @@ 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 de691c7623d..8b5b6b51dc0 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,8 +19,10 @@ 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; @@ -51,6 +53,7 @@ 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; @@ -70,6 +73,7 @@ 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; @@ -102,9 +106,9 @@ public class WebFluxConfigurationSupportTests { assertEquals(0, mapping.getOrder()); - assertFalse(mapping.useSuffixPatternMatch()); - assertFalse(mapping.useRegisteredSuffixPatternMatch()); - assertTrue(mapping.useTrailingSlashMatch()); + assertFalse(mapping.getPatternRegistry().useSuffixPatternMatch()); + assertThat(mapping.getPatternRegistry().getFileExtensions(), Matchers.empty()); + assertTrue(mapping.getPatternRegistry().useTrailingSlashMatch()); name = "webFluxContentTypeResolver"; RequestedContentTypeResolver resolver = context.getBean(name, RequestedContentTypeResolver.class); @@ -126,8 +130,9 @@ public class WebFluxConfigurationSupportTests { RequestMappingHandlerMapping mapping = context.getBean(name, RequestMappingHandlerMapping.class); assertNotNull(mapping); - assertFalse(mapping.useSuffixPatternMatch()); - assertFalse(mapping.useTrailingSlashMatch()); + assertFalse(mapping.getPatternRegistry().useTrailingSlashMatch()); + assertTrue(mapping.getPatternRegistry().useSuffixPatternMatch()); + assertThat(mapping.getPatternRegistry().getFileExtensions(), Matchers.contains(".json", ".xml")); } @Test @@ -306,8 +311,15 @@ 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/result/condition/PatternsRequestConditionTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/PatternsRequestConditionTests.java index 7c2b6b8fbc4..9d1c547bfc6 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 @@ -21,7 +21,6 @@ import java.util.Collections; import java.util.Iterator; import java.util.Set; -import org.junit.Ignore; import org.junit.Test; import org.springframework.http.server.reactive.ServerHttpRequest; @@ -30,6 +29,7 @@ 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; @@ -119,7 +119,8 @@ public class PatternsRequestConditionTests { assertNotNull(match); assertEquals("/{foo}", match.getPatterns().iterator().next().getPatternString()); - condition = new PatternsRequestCondition(new String[] {"/foo"}, null, true, false, null); + condition = new PatternsRequestCondition(new String[] {"/foo"}, null, + createPatternRegistry(true, false, null)); match = condition.getMatchingCondition(exchange); assertNotNull(match); @@ -132,7 +133,8 @@ 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, true, false, extensions); + PatternsRequestCondition condition = new PatternsRequestCondition(patterns, null, + createPatternRegistry(true, false, extensions)); ServerWebExchange exchange = createExchange("/jobs/my.job"); PatternsRequestCondition match = condition.getMatchingCondition(exchange); @@ -152,10 +154,12 @@ public class PatternsRequestConditionTests { @Test public void matchSuffixPatternUsingFileExtensions2() throws Exception { PatternsRequestCondition condition1 = new PatternsRequestCondition( - new String[] {"/prefix"}, null, true, false, Collections.singleton("json")); + new String[] {"/prefix"}, null, + createPatternRegistry(true, false, Collections.singleton("json"))); PatternsRequestCondition condition2 = new PatternsRequestCondition( - new String[] {"/suffix"}, null, true, false, null); + new String[] {"/suffix"}, null, + createPatternRegistry(true, false, null)); PatternsRequestCondition combined = condition1.combine(condition2); @@ -174,14 +178,16 @@ public class PatternsRequestConditionTests { assertNull("Should not match by default", match); - condition = new PatternsRequestCondition(new String[] {"/foo"}, null, false, true, null); + condition = new PatternsRequestCondition(new String[] {"/foo"}, null, + createPatternRegistry(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()); - condition = new PatternsRequestCondition(new String[] {"/foo"}, null, true, true, null); + condition = new PatternsRequestCondition(new String[] {"/foo"}, null, + createPatternRegistry(true, true, null)); match = condition.getMatchingCondition(exchange); assertNotNull(match); @@ -232,4 +238,13 @@ 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/RequestMappingInfoHandlerMappingTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java index 0c959f60bfb..8fb32940155 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 @@ -64,6 +64,7 @@ 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; @@ -545,10 +546,12 @@ 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.setSuffixPatternMatch(true); - options.setTrailingSlashMatch(true); + options.setPathPatternRegistry(pathPatternRegistry); 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 a8015455952..1cd293b51cf 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 @@ -22,10 +22,7 @@ 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.SortedSet; import java.util.stream.Collectors; import org.hamcrest.Matchers; @@ -43,19 +40,12 @@ 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 org.springframework.web.util.patterns.PathPattern; 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.ArgumentMatchers.contains; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; /** * Unit tests for {@link RequestMappingHandlerMapping}. @@ -74,59 +64,6 @@ public class RequestMappingHandlerMappingTests { this.handlerMapping.setApplicationContext(wac); } - - @Test - public void useRegisteredSuffixPatternMatch() { - assertFalse(this.handlerMapping.useSuffixPatternMatch()); - assertFalse(this.handlerMapping.useRegisteredSuffixPatternMatch()); - - MappingContentTypeResolver contentTypeResolver = mock(MappingContentTypeResolver.class); - when(contentTypeResolver.getKeys()).thenReturn(Collections.singleton("json")); - - this.handlerMapping.setUseSuffixPatternMatch(true); - this.handlerMapping.setUseRegisteredSuffixPatternMatch(true); - 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() { - assertFalse(this.handlerMapping.useSuffixPatternMatch()); - assertFalse(this.handlerMapping.useRegisteredSuffixPatternMatch()); - - this.handlerMapping.setUseRegisteredSuffixPatternMatch(true); - assertTrue("'true' registeredSuffixPatternMatch should enable suffixPatternMatch", - this.handlerMapping.useSuffixPatternMatch()); - } - @Test public void resolveEmbeddedValuesInPatterns() { this.handlerMapping.setEmbeddedValueResolver(