Sort handler matches in ResourceUrlProvider

Prior to this change, the `ResourceUrlProvider.getForLookupPath` method
would try to match handlers using the keySet order in the
handlerMappings Map. In case of several matches, the handler used for
the return value could vary, since the registration order in the
handlerMappings can't be guaranteed in the configuration.

This commit now collects all matching handlers and sort them using a
`PatternComparator`, in order to try each handler from the most specific
mapping to the least.

Issue: SPR-12647

Author: bclozel

spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlProvider.java

@@ -18,6 +18,7 @@
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -191,32 +192,41 @@ private int getLookupPathIndex(HttpServletRequest request) {
 	 * public use.
 	 * <p>It is expected that the given path is what Spring MVC would use for
 	 * request mapping purposes, i.e. excluding context and servlet path portions.
+	 * <p>If several handler mappings match, the handler used will be the one
+	 * configured with the most specific pattern.
 	 * @param lookupPath the lookup path to check
 	 * @return the resolved public URL path, or {@code null} if unresolved
 	 */
 	public final String getForLookupPath(String lookupPath) {
 		if (logger.isTraceEnabled()) {
 			logger.trace("Getting resource URL for lookupPath=" + lookupPath);
 		}
+		List<String> matchingPatterns = new ArrayList<String>();
 		for (String pattern : this.handlerMap.keySet()) {
-			if (!getPathMatcher().match(pattern, lookupPath)) {
-				continue;
+			if (getPathMatcher().match(pattern, lookupPath)) {
+				matchingPatterns.add(pattern);
 			}
-			String pathWithinMapping = getPathMatcher().extractPathWithinPattern(pattern, lookupPath);
-			String pathMapping = lookupPath.substring(0, lookupPath.indexOf(pathWithinMapping));
-			if (logger.isTraceEnabled()) {
-				logger.trace("Invoking ResourceResolverChain for URL pattern=\"" + pattern + "\"");
-			}
-			ResourceHttpRequestHandler handler = this.handlerMap.get(pattern);
-			ResourceResolverChain chain = new DefaultResourceResolverChain(handler.getResourceResolvers());
-			String resolved = chain.resolveUrlPath(pathWithinMapping, handler.getLocations());
-			if (resolved == null) {
-				continue;
-			}
-			if (logger.isTraceEnabled()) {
-				logger.trace("Resolved public resource URL path=\"" + resolved + "\"");
+		}
+		if (!matchingPatterns.isEmpty()) {
+			Comparator<String> patternComparator = getPathMatcher().getPatternComparator(lookupPath);
+			Collections.sort(matchingPatterns, patternComparator);
+			for(String pattern : matchingPatterns) {
+				String pathWithinMapping = getPathMatcher().extractPathWithinPattern(pattern, lookupPath);
+				String pathMapping = lookupPath.substring(0, lookupPath.indexOf(pathWithinMapping));
+				if (logger.isTraceEnabled()) {
+					logger.trace("Invoking ResourceResolverChain for URL pattern=\"" + pattern + "\"");
+				}
+				ResourceHttpRequestHandler handler = this.handlerMap.get(pattern);
+				ResourceResolverChain chain = new DefaultResourceResolverChain(handler.getResourceResolvers());
+				String resolved = chain.resolveUrlPath(pathWithinMapping, handler.getLocations());
+				if (resolved == null) {
+					continue;
+				}
+				if (logger.isTraceEnabled()) {
+					logger.trace("Resolved public resource URL path=\"" + resolved + "\"");
+				}
+				return pathMapping + resolved;
 			}
-			return pathMapping + resolved;
 		}
 		logger.debug("No matching resource mapping");
 		return null;

spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlProviderTests.java

@@ -44,6 +44,8 @@
  */
 public class ResourceUrlProviderTests {
 
+	private List<Resource> locations;
+
 	private ResourceUrlProvider translator;
 
 	private ResourceHttpRequestHandler handler;
@@ -53,9 +55,9 @@ public class ResourceUrlProviderTests {
 
 	@Before
 	public void setUp() {
-		List<Resource> locations = new ArrayList<Resource>();
-		locations.add(new ClassPathResource("test/", getClass()));
-		locations.add(new ClassPathResource("testalternatepath/", getClass()));
+		this.locations = new ArrayList<Resource>();
+		this.locations.add(new ClassPathResource("test/", getClass()));
+		this.locations.add(new ClassPathResource("testalternatepath/", getClass()));
 
 		this.handler = new ResourceHttpRequestHandler();
 		this.handler.setLocations(locations);
@@ -94,6 +96,28 @@ private void initTranslator() {
 		this.translator.setHandlerMap(this.handlerMap);
 	}
 
+	// SPR-12647
+	@Test
+	public void bestPatternMatch() throws Exception {
+		ResourceHttpRequestHandler otherHandler = new ResourceHttpRequestHandler();
+		otherHandler.setLocations(this.locations);
+		Map<String, VersionStrategy> versionStrategyMap = new HashMap<>();
+		versionStrategyMap.put("/**", new ContentVersionStrategy());
+		VersionResourceResolver versionResolver = new VersionResourceResolver();
+		versionResolver.setStrategyMap(versionStrategyMap);
+
+		List<ResourceResolver> resolvers = new ArrayList<ResourceResolver>();
+		resolvers.add(versionResolver);
+		resolvers.add(new PathResourceResolver());
+		otherHandler.setResourceResolvers(resolvers);
+
+		this.handlerMap.put("/resources/*.css", otherHandler);
+		initTranslator();
+
+		String url = this.translator.getForLookupPath("/resources/foo.css");
+		assertEquals("/resources/foo-e36d2e05253c6c7085a91522ce43a0b4.css", url);
+	}
+
 	// SPR-12592
 	@Test
 	public void initializeOnce() throws Exception {