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

Author: rstoyanchev

spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManager.java

@@ -97,11 +97,23 @@ public List<MediaType> resolveMediaTypes(NativeWebRequest webRequest) throws Htt
 	 * 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);
 	}
 
 }

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<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,19 +58,23 @@ public MappingMediaTypeFileExtensionResolver(Map<String, MediaType> mediaTypes)
 	}
 
 	/**
-	 * 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) {
 		List<String> fileExtensions = this.fileExtensions.get(mediaType);
 		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 @@ protected void addMapping(String extension, MediaType mediaType) {
 		MediaType previous = this.mediaTypes.putIfAbsent(extension, mediaType);
 		if (previous == null) {
 			this.fileExtensions.add(mediaType, extension);
+			this.allFileExtensions.add(extension);
 		}
 	}
 

spring-web/src/main/java/org/springframework/web/accept/MediaTypeFileExtensionResolver.java

@@ -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();
+
 }

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PatternsRequestCondition.java

@@ -34,63 +34,86 @@
 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 @@ protected String getToStringInfix() {
 	}
 
 	/**
-	 * 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,26 +177,26 @@ else if (!other.patterns.isEmpty()) {
 			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
 	 * 	<li>Pattern match with ".*" appended if the pattern doesn't already contain a "."
 	 * 	<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,19 +212,28 @@ public PatternsRequestCondition getMatchingCondition(HttpServletRequest request)
 			}
 		}
 		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) {
 		if (pattern.equals(lookupPath)) {
 			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 @@ private String getMatchingPattern(String pattern, String lookupPath) {
 	}
 
 	/**
-	 * Compare the two conditions based on the URL patterns they contain. 
-	 * Patterns are compared one at a time, from top to bottom via 
-	 * {@link PathMatcher#getPatternComparator(String)}. If all compared 
-	 * 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) {

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<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 void setUseTrailingSlashMatch(boolean useTrailingSlashMatch) {
 	 * 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 boolean useTrailingSlashMatch() {
 	 * 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 @@ protected RequestCondition<?> getCustomMethodCondition(Method method) {
 	 */
 	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()),