diff --git a/spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManager.java b/spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManager.java index cb14f7f6b4..285f547c57 100644 --- a/spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManager.java +++ b/spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManager.java @@ -97,11 +97,23 @@ public class ContentNegotiationManager implements ContentNegotiationStrategy, Me * the list of all file extensions found. */ public List resolveFileExtensions(MediaType mediaType) { - Set extensions = new LinkedHashSet(); + Set result = new LinkedHashSet(); for (MediaTypeFileExtensionResolver resolver : this.fileExtensionResolvers) { - extensions.addAll(resolver.resolveFileExtensions(mediaType)); + result.addAll(resolver.resolveFileExtensions(mediaType)); } - return new ArrayList(extensions); + return new ArrayList(result); + } + + /** + * Delegate to all configured MediaTypeFileExtensionResolver instances and aggregate + * the list of all known file extensions. + */ + public List getAllFileExtensions() { + Set result = new LinkedHashSet(); + for (MediaTypeFileExtensionResolver resolver : this.fileExtensionResolvers) { + result.addAll(resolver.getAllFileExtensions()); + } + return new ArrayList(result); } } diff --git a/spring-web/src/main/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolver.java b/spring-web/src/main/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolver.java index 29ee02a49e..b694510158 100644 --- a/spring-web/src/main/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolver.java +++ b/spring-web/src/main/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolver.java @@ -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 fileExtensions = new LinkedMultiValueMap(); + private final List allFileExtensions = new ArrayList(); + /** * 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 resolveFileExtensions(MediaType mediaType) { @@ -63,11 +66,15 @@ public class MappingMediaTypeFileExtensionResolver implements MediaTypeFileExten return (fileExtensions != null) ? fileExtensions : Collections.emptyList(); } + public List 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); } } diff --git a/spring-web/src/main/java/org/springframework/web/accept/MediaTypeFileExtensionResolver.java b/spring-web/src/main/java/org/springframework/web/accept/MediaTypeFileExtensionResolver.java index 73f5882603..218a07fe6c 100644 --- a/spring-web/src/main/java/org/springframework/web/accept/MediaTypeFileExtensionResolver.java +++ b/spring-web/src/main/java/org/springframework/web/accept/MediaTypeFileExtensionResolver.java @@ -37,4 +37,10 @@ public interface MediaTypeFileExtensionResolver { */ List resolveFileExtensions(MediaType mediaType); + /** + * Return all known file extensions. + * @return a list of extensions or an empty list, never {@code null} + */ + List getAllFileExtensions(); + } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PatternsRequestCondition.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PatternsRequestCondition.java index 9b61fe7c95..10743bb230 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PatternsRequestCondition.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PatternsRequestCondition.java @@ -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 { - - private final Set patterns; + + private final Set patterns; private final UrlPathHelper urlPathHelper; - + private final PathMatcher pathMatcher; private final boolean useSuffixPatternMatch; private final boolean useTrailingSlashMatch; - + + private final List fileExtensions = new ArrayList(); + /** * 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 fileExtensions) { + + this(asList(patterns), urlPathHelper, pathMatcher, useSuffixPatternMatch, useTrailingSlashMatch, fileExtensions); } /** * Private constructor accepting a collection of patterns. + * @param fileExtensionResolver */ - private PatternsRequestCondition(Collection patterns, - UrlPathHelper urlPathHelper, - PathMatcher pathMatcher, - boolean useSuffixPatternMatch, - boolean useTrailingSlashMatch) { + private PatternsRequestCondition(Collection patterns, UrlPathHelper urlPathHelper, + PathMatcher pathMatcher, boolean useSuffixPatternMatch, boolean useTrailingSlashMatch, + List 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 asList(String... patterns) { @@ -126,15 +149,15 @@ public final class PatternsRequestCondition extends AbstractRequestCondition - *
  • If there are patterns in both instances, combine the patterns in "this" with + *
  • If there are patterns in both instances, combine the patterns in "this" with * the patterns in "other" using {@link PathMatcher#combine(String, String)}. *
  • If only one instance has patterns, use them. *
  • If neither instance has patterns, use an empty String (i.e. ""). * - */ + */ public PatternsRequestCondition combine(PatternsRequestCondition other) { Set result = new LinkedHashSet(); if (!this.patterns.isEmpty() && !other.patterns.isEmpty()) { @@ -154,14 +177,14 @@ public final class PatternsRequestCondition extends AbstractRequestConditionA matching pattern is obtained by making checks in the following order: *
      *
    • Direct match @@ -169,11 +192,11 @@ public final class PatternsRequestCondition extends AbstractRequestConditionPattern match *
    • Pattern match with "/" appended if the pattern doesn't already end in "/" *
    - * + * * @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 AbstractRequestConditionIt 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 + * + *

    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) { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java index 211644612b..921948ae18 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java @@ -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 contentNegotiationFileExtensions = new ArrayList(); + /** * 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 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()), diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/PatternsRequestConditionTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/PatternsRequestConditionTests.java index d59dce9f3d..bd276ab398 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/PatternsRequestConditionTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/PatternsRequestConditionTests.java @@ -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 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)); } - + } diff --git a/src/dist/changelog.txt b/src/dist/changelog.txt index 41190821ca..f83a2d6d49 100644 --- a/src/dist/changelog.txt +++ b/src/dist/changelog.txt @@ -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) --------------------------------------