提交 4fd7645e 编写于 作者: R Rossen Stoyanchev

Enable smart suffix pattern match for request mapping

Following the introduction of ContentNegotiationManager that allows,
among other things, to configure the file extensions to use for content
negotiation, this change adds "smart" suffix pattern match that matches
against the configured file extensions only rather than against any
extension.

Given the request mapping "/jobs/{jobName}" and one configured file
extension ("json"), a request for "/jobs/my.job" will select the
pattern "/jobs/{jobName}" while a request for "/jobs/my.job.json" will
select the pattern "/jobs/{jobName}.json". Previously, both requests
would have resulted in the pattern "/jobs/{jobName}.*".

Issue: SPR-7632, SPR-8474
上级 f94aed83
......@@ -97,11 +97,23 @@ public class ContentNegotiationManager implements ContentNegotiationStrategy, Me
* the list of all file extensions found.
*/
public List<String> resolveFileExtensions(MediaType mediaType) {
Set<String> extensions = new LinkedHashSet<String>();
Set<String> result = new LinkedHashSet<String>();
for (MediaTypeFileExtensionResolver resolver : this.fileExtensionResolvers) {
extensions.addAll(resolver.resolveFileExtensions(mediaType));
result.addAll(resolver.resolveFileExtensions(mediaType));
}
return new ArrayList<String>(extensions);
return new ArrayList<String>(result);
}
/**
* Delegate to all configured MediaTypeFileExtensionResolver instances and aggregate
* the list of all known file extensions.
*/
public List<String> getAllFileExtensions() {
Set<String> result = new LinkedHashSet<String>();
for (MediaTypeFileExtensionResolver resolver : this.fileExtensionResolvers) {
result.addAll(resolver.getAllFileExtensions());
}
return new ArrayList<String>(result);
}
}
......@@ -15,6 +15,7 @@
*/
package org.springframework.web.accept;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
......@@ -40,6 +41,8 @@ public class MappingMediaTypeFileExtensionResolver implements MediaTypeFileExten
private final MultiValueMap<MediaType, String> fileExtensions = new LinkedMultiValueMap<MediaType, String>();
private final List<String> allFileExtensions = new ArrayList<String>();
/**
* Create an instance with the given mappings between extensions and media types.
* @throws IllegalArgumentException if a media type string cannot be parsed
......@@ -55,7 +58,7 @@ public class MappingMediaTypeFileExtensionResolver implements MediaTypeFileExten
}
/**
* Find the extensions applicable to the given MediaType.
* Find the file extensions mapped to the given MediaType.
* @return 0 or more extensions, never {@code null}
*/
public List<String> resolveFileExtensions(MediaType mediaType) {
......@@ -63,11 +66,15 @@ public class MappingMediaTypeFileExtensionResolver implements MediaTypeFileExten
return (fileExtensions != null) ? fileExtensions : Collections.<String>emptyList();
}
public List<String> getAllFileExtensions() {
return Collections.unmodifiableList(this.allFileExtensions);
}
/**
* Return the MediaType mapped to the given extension.
* @return a MediaType for the key or {@code null}
*/
public MediaType lookupMediaType(String extension) {
protected MediaType lookupMediaType(String extension) {
return this.mediaTypes.get(extension);
}
......@@ -78,6 +85,7 @@ public class MappingMediaTypeFileExtensionResolver implements MediaTypeFileExten
MediaType previous = this.mediaTypes.putIfAbsent(extension, mediaType);
if (previous == null) {
this.fileExtensions.add(mediaType, extension);
this.allFileExtensions.add(extension);
}
}
......
......@@ -37,4 +37,10 @@ public interface MediaTypeFileExtensionResolver {
*/
List<String> resolveFileExtensions(MediaType mediaType);
/**
* Return all known file extensions.
* @return a list of extensions or an empty list, never {@code null}
*/
List<String> getAllFileExtensions();
}
......@@ -34,63 +34,86 @@ import org.springframework.util.StringUtils;
import org.springframework.web.util.UrlPathHelper;
/**
* A logical disjunction (' || ') request condition that matches a request
* against a set of URL path patterns.
*
* A logical disjunction (' || ') request condition that matches a request
* against a set of URL path patterns.
*
* @author Rossen Stoyanchev
* @since 3.1
*/
public final class PatternsRequestCondition extends AbstractRequestCondition<PatternsRequestCondition> {
private final Set<String> patterns;
private final Set<String> patterns;
private final UrlPathHelper urlPathHelper;
private final PathMatcher pathMatcher;
private final boolean useSuffixPatternMatch;
private final boolean useTrailingSlashMatch;
private final List<String> fileExtensions = new ArrayList<String>();
/**
* Creates a new instance with the given URL patterns.
* Each pattern that is not empty and does not start with "/" is pre-pended with "/".
* @param patterns 0 or more URL patterns; if 0 the condition will match to every request.
* 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(asList(patterns), null, null, true, true);
this(asList(patterns), null, null, true, true, null);
}
/**
* Additional constructor with flags for using suffix pattern (.*) and
* trailing slash matches.
*
* @param patterns the URL patterns to use; if 0, the condition will match to every request.
* @param urlPathHelper for determining the lookup path of a request
* @param pathMatcher for path matching with patterns
* @param useSuffixPatternMatch whether to enable matching by suffix (".*")
* @param useTrailingSlashMatch whether to match irrespective of a trailing slash
*/
public PatternsRequestCondition(String[] patterns, UrlPathHelper urlPathHelper, PathMatcher pathMatcher,
boolean useSuffixPatternMatch, boolean useTrailingSlashMatch) {
this(asList(patterns), urlPathHelper, pathMatcher, useSuffixPatternMatch, useTrailingSlashMatch, null);
}
/**
* Creates a new instance with the given URL patterns.
* Each pattern that is not empty and does not start with "/" is pre-pended with "/".
* @param patterns the URL patterns to use; if 0, the condition will match to every request.
* @param patterns the URL patterns to use; if 0, the condition will match to every request.
* @param urlPathHelper a {@link UrlPathHelper} for determining the lookup path for a request
* @param pathMatcher a {@link PathMatcher} for pattern path matching
* @param useSuffixPatternMatch whether to enable matching by suffix (".*")
* @param useTrailingSlashMatch whether to match irrespective of a trailing slash
* @param fileExtensions a list of file extensions to consider for path matching
*/
public PatternsRequestCondition(String[] patterns,
UrlPathHelper urlPathHelper,
PathMatcher pathMatcher,
boolean useSuffixPatternMatch,
boolean useTrailingSlashMatch) {
this(asList(patterns), urlPathHelper, pathMatcher, useSuffixPatternMatch, useTrailingSlashMatch);
public PatternsRequestCondition(String[] patterns, UrlPathHelper urlPathHelper,
PathMatcher pathMatcher, boolean useSuffixPatternMatch, boolean useTrailingSlashMatch,
List<String> fileExtensions) {
this(asList(patterns), urlPathHelper, pathMatcher, useSuffixPatternMatch, useTrailingSlashMatch, fileExtensions);
}
/**
* Private constructor accepting a collection of patterns.
* @param fileExtensionResolver
*/
private PatternsRequestCondition(Collection<String> patterns,
UrlPathHelper urlPathHelper,
PathMatcher pathMatcher,
boolean useSuffixPatternMatch,
boolean useTrailingSlashMatch) {
private PatternsRequestCondition(Collection<String> patterns, UrlPathHelper urlPathHelper,
PathMatcher pathMatcher, boolean useSuffixPatternMatch, boolean useTrailingSlashMatch,
List<String> fileExtensions) {
this.patterns = Collections.unmodifiableSet(prependLeadingSlash(patterns));
this.urlPathHelper = urlPathHelper != null ? urlPathHelper : new UrlPathHelper();
this.pathMatcher = pathMatcher != null ? pathMatcher : new AntPathMatcher();
this.useSuffixPatternMatch = useSuffixPatternMatch;
this.useTrailingSlashMatch = useTrailingSlashMatch;
if (fileExtensions != null) {
for (String fileExtension : fileExtensions) {
this.fileExtensions.add("." + fileExtension);
}
}
}
private static List<String> asList(String... patterns) {
......@@ -126,15 +149,15 @@ public final class PatternsRequestCondition extends AbstractRequestCondition<Pat
}
/**
* Returns a new instance with URL patterns from the current instance ("this") and
* the "other" instance as follows:
* Returns a new instance with URL patterns from the current instance ("this") and
* the "other" instance as follows:
* <ul>
* <li>If there are patterns in both instances, combine the patterns in "this" with
* <li>If there are patterns in both instances, combine the patterns in "this" with
* the patterns in "other" using {@link PathMatcher#combine(String, String)}.
* <li>If only one instance has patterns, use them.
* <li>If neither instance has patterns, use an empty String (i.e. "").
* </ul>
*/
*/
public PatternsRequestCondition combine(PatternsRequestCondition other) {
Set<String> result = new LinkedHashSet<String>();
if (!this.patterns.isEmpty() && !other.patterns.isEmpty()) {
......@@ -154,14 +177,14 @@ public final class PatternsRequestCondition extends AbstractRequestCondition<Pat
result.add("");
}
return new PatternsRequestCondition(result, this.urlPathHelper, this.pathMatcher, this.useSuffixPatternMatch,
this.useTrailingSlashMatch);
this.useTrailingSlashMatch, this.fileExtensions);
}
/**
* Checks if any of the patterns match the given request and returns an instance
* that is guaranteed to contain matching patterns, sorted via
* {@link PathMatcher#getPatternComparator(String)}.
*
* Checks if any of the patterns match the given request and returns an instance
* that is guaranteed to contain matching patterns, sorted via
* {@link PathMatcher#getPatternComparator(String)}.
*
* <p>A matching pattern is obtained by making checks in the following order:
* <ul>
* <li>Direct match
......@@ -169,11 +192,11 @@ public final class PatternsRequestCondition extends AbstractRequestCondition<Pat
* <li>Pattern match
* <li>Pattern match with "/" appended if the pattern doesn't already end in "/"
* </ul>
*
*
* @param request the current request
*
* @return the same instance if the condition contains no patterns;
* or a new condition with sorted matching patterns;
*
* @return the same instance if the condition contains no patterns;
* or a new condition with sorted matching patterns;
* or {@code null} if no patterns match.
*/
public PatternsRequestCondition getMatchingCondition(HttpServletRequest request) {
......@@ -189,9 +212,9 @@ public final class PatternsRequestCondition extends AbstractRequestCondition<Pat
}
}
Collections.sort(matches, this.pathMatcher.getPatternComparator(lookupPath));
return matches.isEmpty() ? null :
return matches.isEmpty() ? null :
new PatternsRequestCondition(matches, this.urlPathHelper, this.pathMatcher, this.useSuffixPatternMatch,
this.useTrailingSlashMatch);
this.useTrailingSlashMatch, this.fileExtensions);
}
private String getMatchingPattern(String pattern, String lookupPath) {
......@@ -199,9 +222,18 @@ public final class PatternsRequestCondition extends AbstractRequestCondition<Pat
return pattern;
}
if (this.useSuffixPatternMatch) {
boolean hasSuffix = pattern.indexOf('.') != -1;
if (!hasSuffix && this.pathMatcher.match(pattern + ".*", lookupPath)) {
return pattern + ".*";
if (useSmartSuffixPatternMatch(pattern, lookupPath)) {
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)) {
......@@ -217,15 +249,23 @@ public final class PatternsRequestCondition extends AbstractRequestCondition<Pat
}
/**
* Compare the two conditions based on the URL patterns they contain.
* Patterns are compared one at a time, from top to bottom via
* {@link PathMatcher#getPatternComparator(String)}. If all compared
* patterns match equally, but one instance has more patterns, it is
* Whether to match by known file extensions. Return "true" if file extensions
* are configured, and the lookup path has a suffix.
*/
private boolean useSmartSuffixPatternMatch(String pattern, String lookupPath) {
return (!this.fileExtensions.isEmpty() && lookupPath.indexOf('.') != -1) ;
}
/**
* Compare the two conditions based on the URL patterns they contain.
* Patterns are compared one at a time, from top to bottom via
* {@link PathMatcher#getPatternComparator(String)}. If all compared
* patterns match equally, but one instance has more patterns, it is
* considered a closer match.
*
* <p>It is assumed that both instances have been obtained via
* {@link #getMatchingCondition(HttpServletRequest)} to ensure they
* contain only patterns that match the request and are sorted with
*
* <p>It is assumed that both instances have been obtained via
* {@link #getMatchingCondition(HttpServletRequest)} to ensure they
* contain only patterns that match the request and are sorted with
* the best matches on top.
*/
public int compareTo(PatternsRequestCondition other, HttpServletRequest request) {
......
......@@ -17,11 +17,13 @@
package org.springframework.web.servlet.mvc.method.annotation;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.List;
import org.springframework.core.annotation.AnnotationUtils;
import org.springframework.stereotype.Controller;
import org.springframework.util.Assert;
import org.springframework.web.accept.ContentNegotiationManager;
import org.springframework.web.accept.HeaderContentNegotiationStrategy;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.servlet.mvc.condition.AbstractRequestCondition;
import org.springframework.web.servlet.mvc.condition.CompositeRequestCondition;
......@@ -52,6 +54,8 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi
private ContentNegotiationManager contentNegotiationManager = new ContentNegotiationManager();
private final List<String> contentNegotiationFileExtensions = new ArrayList<String>();
/**
* Whether to use suffix pattern match (".*") when matching patterns to
* requests. If enabled a method mapped to "/users" also matches to "/users.*".
......@@ -75,7 +79,9 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi
* If not set, the default constructor is used.
*/
public void setContentNegotiationManager(ContentNegotiationManager contentNegotiationManager) {
Assert.notNull(contentNegotiationManager);
this.contentNegotiationManager = contentNegotiationManager;
this.contentNegotiationFileExtensions.addAll(contentNegotiationManager.getAllFileExtensions());
}
/**
......@@ -95,7 +101,14 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi
* Return the configured {@link ContentNegotiationManager}.
*/
public ContentNegotiationManager getContentNegotiationManager() {
return contentNegotiationManager;
return this.contentNegotiationManager;
}
/**
* Return the known file extensions for content negotiation.
*/
public List<String> getContentNegotiationFileExtensions() {
return this.contentNegotiationFileExtensions;
}
/**
......@@ -173,8 +186,8 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi
*/
private RequestMappingInfo createRequestMappingInfo(RequestMapping annotation, RequestCondition<?> customCondition) {
return new RequestMappingInfo(
new PatternsRequestCondition(annotation.value(),
getUrlPathHelper(), getPathMatcher(), this.useSuffixPatternMatch, this.useTrailingSlashMatch),
new PatternsRequestCondition(annotation.value(), getUrlPathHelper(), getPathMatcher(),
this.useSuffixPatternMatch, this.useTrailingSlashMatch, this.contentNegotiationFileExtensions),
new RequestMethodsRequestCondition(annotation.method()),
new ParamsRequestCondition(annotation.params()),
new HeadersRequestCondition(annotation.headers()),
......
......@@ -20,6 +20,9 @@ import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import java.util.Arrays;
import java.util.List;
import javax.servlet.http.HttpServletRequest;
import org.junit.Test;
......@@ -46,20 +49,20 @@ public class PatternsRequestConditionTests {
public void combineEmptySets() {
PatternsRequestCondition c1 = new PatternsRequestCondition();
PatternsRequestCondition c2 = new PatternsRequestCondition();
assertEquals(new PatternsRequestCondition(""), c1.combine(c2));
}
@Test
public void combineOnePatternWithEmptySet() {
PatternsRequestCondition c1 = new PatternsRequestCondition("/type1", "/type2");
PatternsRequestCondition c2 = new PatternsRequestCondition();
assertEquals(new PatternsRequestCondition("/type1", "/type2"), c1.combine(c2));
c1 = new PatternsRequestCondition();
c2 = new PatternsRequestCondition("/method1", "/method2");
assertEquals(new PatternsRequestCondition("/method1", "/method2"), c1.combine(c2));
}
......@@ -67,9 +70,9 @@ public class PatternsRequestConditionTests {
public void combineMultiplePatterns() {
PatternsRequestCondition c1 = new PatternsRequestCondition("/t1", "/t2");
PatternsRequestCondition c2 = new PatternsRequestCondition("/m1", "/m2");
assertEquals(new PatternsRequestCondition("/t1/m1", "/t1/m2", "/t2/m1", "/t2/m2"), c1.combine(c2));
}
}
@Test
public void matchDirectPath() {
......@@ -78,56 +81,78 @@ public class PatternsRequestConditionTests {
assertNotNull(match);
}
@Test
public void matchPattern() {
PatternsRequestCondition condition = new PatternsRequestCondition("/foo/*");
PatternsRequestCondition match = condition.getMatchingCondition(new MockHttpServletRequest("GET", "/foo/bar"));
assertNotNull(match);
}
@Test
public void matchSortPatterns() {
PatternsRequestCondition condition = new PatternsRequestCondition("/**", "/foo/bar", "/foo/*");
PatternsRequestCondition match = condition.getMatchingCondition(new MockHttpServletRequest("GET", "/foo/bar"));
PatternsRequestCondition expected = new PatternsRequestCondition("/foo/bar", "/foo/*", "/**");
assertEquals(expected, match);
}
@Test
public void matchSuffixPattern() {
public void matchSuffixPattern() {
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo.html");
PatternsRequestCondition condition = new PatternsRequestCondition("/{foo}");
PatternsRequestCondition match = condition.getMatchingCondition(request);
assertNotNull(match);
assertEquals("/{foo}.*", match.getPatterns().iterator().next());
condition = new PatternsRequestCondition(new String[] {"/{foo}"}, null, null, false, false);
boolean useSuffixPatternMatch = false;
condition = new PatternsRequestCondition(new String[] {"/{foo}"}, null, null, useSuffixPatternMatch, false);
match = condition.getMatchingCondition(request);
assertNotNull(match);
assertEquals("/{foo}", match.getPatterns().iterator().next());
}
// SPR-8410
@Test
public void matchSuffixPatternUsingFileExtensions() {
String[] patterns = new String[] {"/jobs/{jobName}"};
List<String> extensions = Arrays.asList("json");
PatternsRequestCondition condition = new PatternsRequestCondition(patterns, null, null, true, false, extensions);
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/jobs/my.job");
PatternsRequestCondition match = condition.getMatchingCondition(request);
assertNotNull(match);
assertEquals("/jobs/{jobName}", match.getPatterns().iterator().next());
request = new MockHttpServletRequest("GET", "/jobs/my.job.json");
match = condition.getMatchingCondition(request);
assertNotNull(match);
assertEquals("/jobs/{jobName}.json", match.getPatterns().iterator().next());
}
@Test
public void matchTrailingSlash() {
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo/");
PatternsRequestCondition condition = new PatternsRequestCondition("/foo");
PatternsRequestCondition match = condition.getMatchingCondition(request);
assertNotNull(match);
assertEquals("Should match by default", "/foo/", match.getPatterns().iterator().next());
condition = new PatternsRequestCondition(new String[] {"/foo"}, null, null, false, true);
match = condition.getMatchingCondition(request);
assertNotNull(match);
assertEquals("Trailing slash should be insensitive to useSuffixPatternMatch settings (SPR-6164, SPR-5636)",
assertEquals("Trailing slash should be insensitive to useSuffixPatternMatch settings (SPR-6164, SPR-5636)",
"/foo/", match.getPatterns().iterator().next());
condition = new PatternsRequestCondition(new String[] {"/foo"}, null, null, false, false);
......@@ -156,21 +181,21 @@ public class PatternsRequestConditionTests {
public void comparePatternSpecificity() {
PatternsRequestCondition c1 = new PatternsRequestCondition("/fo*");
PatternsRequestCondition c2 = new PatternsRequestCondition("/foo");
assertEquals(1, c1.compareTo(c2, new MockHttpServletRequest("GET", "/foo")));
}
@Test
public void compareNumberOfMatchingPatterns() throws Exception {
HttpServletRequest request = new MockHttpServletRequest("GET", "/foo.html");
PatternsRequestCondition c1 = new PatternsRequestCondition("/foo", "*.jpeg");
PatternsRequestCondition c2 = new PatternsRequestCondition("/foo", "*.html");
PatternsRequestCondition match1 = c1.getMatchingCondition(request);
PatternsRequestCondition match2 = c2.getMatchingCondition(request);
assertEquals(1, match1.compareTo(match2, request));
}
}
......@@ -12,6 +12,7 @@ Changes in version 3.2 M2
* infer return type of parameterized factory methods (SPR-9493)
* add ContentNegotiationManager/ContentNegotiationStrategy to resolve requested media types
* add support for the HTTP PATCH method
* enable smart suffix pattern match in @RequestMapping methods (SPR-7632)
Changes in version 3.2 M1 (2012-05-28)
--------------------------------------
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册