Clarify WebContentInterceptor path mappings and efficiently match them

Issue: SPR-15096

spring-web/src/main/java/org/springframework/http/converter/ResourceHttpMessageConverter.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.
@@ -30,14 +30,13 @@ import org.springframework.util.ClassUtils;
 import org.springframework.util.StreamUtils;
 
 /**
- * Implementation of {@link HttpMessageConverter} that can read and write {@link Resource Resources}
+ * Implementation of {@link HttpMessageConverter} that can read/write {@link Resource Resources}
  * and supports byte range requests.
  *
  * <p>By default, this converter can read all media types. The Java Activation Framework (JAF) -
  * if available - is used to determine the {@code Content-Type} of written resources.
  * If JAF is not available, {@code application/octet-stream} is used.
  *
- *
  * @author Arjen Poutsma
  * @author Juergen Hoeller
  * @author Kazuki Shimizu

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/WebContentInterceptor.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.
@@ -20,15 +20,14 @@ import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Properties;
-
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.springframework.http.CacheControl;
 import org.springframework.util.AntPathMatcher;
 import org.springframework.util.Assert;
 import org.springframework.util.PathMatcher;
-import org.springframework.http.CacheControl;
 import org.springframework.web.servlet.HandlerInterceptor;
 import org.springframework.web.servlet.ModelAndView;
 import org.springframework.web.servlet.support.WebContentGenerator;
@@ -115,6 +114,9 @@ public class WebContentInterceptor extends WebContentGenerator implements Handle
 	 * <p>Supports direct matches, e.g. a registered "/test" matches "/test",
 	 * and a various Ant-style pattern matches, e.g. a registered "/t*" matches
 	 * both "/test" and "/team". For details, see the AntPathMatcher javadoc.
+	 * <p><b>NOTE:</b> Path patterns are not supposed to overlap. If a request
+	 * matches several mappings, it is effectively undefined which one will apply
+	 * (due to the lack of key ordering in {@code java.util.Properties}).
 	 * @param cacheMappings a mapping between URL paths (as keys) and
 	 * cache seconds (as values, need to be integer-parsable)
 	 * @see #setCacheSeconds
@@ -138,11 +140,14 @@ public class WebContentInterceptor extends WebContentGenerator implements Handle
 	 * <p>Supports direct matches, e.g. a registered "/test" matches "/test",
 	 * and a various Ant-style pattern matches, e.g. a registered "/t*" matches
 	 * both "/test" and "/team". For details, see the AntPathMatcher javadoc.
+	 * <p><b>NOTE:</b> Path patterns are not supposed to overlap. If a request
+	 * matches several mappings, it is effectively undefined which one will apply
+	 * (due to the lack of key ordering in the underlying {@code java.util.HashMap}).
 	 * @param cacheControl the {@code CacheControl} to use
 	 * @param paths URL paths that will map to the given {@code CacheControl}
+	 * @since 4.2
 	 * @see #setCacheSeconds
 	 * @see org.springframework.util.AntPathMatcher
-	 * @since 4.2
 	 */
 	public void addCacheMapping(CacheControl cacheControl, String... paths) {
 		for (String path : paths) {
@@ -211,15 +216,16 @@ public class WebContentInterceptor extends WebContentGenerator implements Handle
 	protected CacheControl lookupCacheControl(String urlPath) {
 		// Direct match?
 		CacheControl cacheControl = this.cacheControlMappings.get(urlPath);
-		if (cacheControl == null) {
+		if (cacheControl != null) {
-			// Pattern match?
+			return cacheControl;
-			for (String registeredPath : this.cacheControlMappings.keySet()) {
+		}
-				if (this.pathMatcher.match(registeredPath, urlPath)) {
+		// Pattern match?
-					cacheControl = this.cacheControlMappings.get(registeredPath);
+		for (String registeredPath : this.cacheControlMappings.keySet()) {
-				}
+			if (this.pathMatcher.match(registeredPath, urlPath)) {
+				return this.cacheControlMappings.get(registeredPath);
 			}
 		}
-		return cacheControl;
+		return null;
 	}
 
 	/**
@@ -234,15 +240,16 @@ public class WebContentInterceptor extends WebContentGenerator implements Handle
 	protected Integer lookupCacheSeconds(String urlPath) {
 		// Direct match?
 		Integer cacheSeconds = this.cacheMappings.get(urlPath);
-		if (cacheSeconds == null) {
+		if (cacheSeconds != null) {
-			// Pattern match?
+			return cacheSeconds;
-			for (String registeredPath : this.cacheMappings.keySet()) {
+		}
-				if (this.pathMatcher.match(registeredPath, urlPath)) {
+		// Pattern match?
-					cacheSeconds = this.cacheMappings.get(registeredPath);
+		for (String registeredPath : this.cacheMappings.keySet()) {
-				}
+			if (this.pathMatcher.match(registeredPath, urlPath)) {
+				return this.cacheMappings.get(registeredPath);
 			}
 		}
-		return cacheSeconds;
+		return null;
 	}