diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/WebContentInterceptor.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/WebContentInterceptor.java index be33c5f07b3..67d08abcf3c 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/WebContentInterceptor.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/WebContentInterceptor.java @@ -55,7 +55,9 @@ public class WebContentInterceptor extends WebContentGenerator implements Handle private PathMatcher pathMatcher = new AntPathMatcher(); - private Map cacheMappings = new HashMap(); + private Map cacheMappings = new HashMap(); + + private Map cacheControlMappings = new HashMap(); public WebContentInterceptor() { // no restriction of HTTP methods by default, @@ -124,15 +126,7 @@ public class WebContentInterceptor extends WebContentGenerator implements Handle while (propNames.hasMoreElements()) { String path = (String) propNames.nextElement(); int cacheSeconds = Integer.valueOf(cacheMappings.getProperty(path)); - if (cacheSeconds > 0) { - this.cacheMappings.put(path, CacheControl.maxAge(cacheSeconds, TimeUnit.SECONDS)); - } - else if (cacheSeconds == 0) { - this.cacheMappings.put(path, CacheControl.noStore()); - } - else { - this.cacheMappings.put(path, CacheControl.empty()); - } + this.cacheMappings.put(path, cacheSeconds); } } @@ -154,7 +148,7 @@ public class WebContentInterceptor extends WebContentGenerator implements Handle */ public void addCacheMapping(CacheControl cacheControl, String... paths) { for (String path : paths) { - this.cacheMappings.put(path, cacheControl); + this.cacheControlMappings.put(path, cacheControl); } } @@ -182,13 +176,20 @@ public class WebContentInterceptor extends WebContentGenerator implements Handle logger.debug("Looking up cache seconds for [" + lookupPath + "]"); } - CacheControl cacheControl = lookupCacheSeconds(lookupPath); + CacheControl cacheControl = lookupCacheControl(lookupPath); + Integer cacheSeconds = lookupCacheSeconds(lookupPath); if (cacheControl != null) { if (logger.isDebugEnabled()) { logger.debug("Applying CacheControl to [" + lookupPath + "]"); } checkAndPrepare(request, response, cacheControl); } + else if (cacheSeconds != null) { + if (logger.isDebugEnabled()) { + logger.debug("Applying CacheControl to [" + lookupPath + "]"); + } + checkAndPrepare(request, response, cacheSeconds); + } else { if (logger.isDebugEnabled()) { logger.debug("Applying default cache seconds to [" + lookupPath + "]"); @@ -208,20 +209,43 @@ public class WebContentInterceptor extends WebContentGenerator implements Handle * @return the associated {@code CacheControl}, or {@code null} if not found * @see org.springframework.util.AntPathMatcher */ - protected CacheControl lookupCacheSeconds(String urlPath) { + protected CacheControl lookupCacheControl(String urlPath) { // direct match? - CacheControl cacheControl = this.cacheMappings.get(urlPath); + CacheControl cacheControl = this.cacheControlMappings.get(urlPath); if (cacheControl == null) { // pattern match? - for (String registeredPath : this.cacheMappings.keySet()) { + for (String registeredPath : this.cacheControlMappings.keySet()) { if (this.pathMatcher.match(registeredPath, urlPath)) { - cacheControl = this.cacheMappings.get(registeredPath); + cacheControl = this.cacheControlMappings.get(registeredPath); } } } return cacheControl; } + /** + * Look up a cacheSeconds integer value for the given URL path. + *

Supports direct matches, e.g. a registered "/test" matches "/test", + * and various Ant-style pattern matches, e.g. a registered "/t*" matches + * both "/test" and "/team". For details, see the AntPathMatcher class. + * @param urlPath URL the bean is mapped to + * @return the cacheSeconds integer value, or {@code null} if not found + * @see org.springframework.util.AntPathMatcher + */ + protected Integer lookupCacheSeconds(String urlPath) { + // direct match? + Integer cacheSeconds = this.cacheMappings.get(urlPath); + if (cacheSeconds == null) { + // pattern match? + for (String registeredPath : this.cacheMappings.keySet()) { + if (this.pathMatcher.match(registeredPath, urlPath)) { + cacheSeconds = this.cacheMappings.get(registeredPath); + } + } + } + return cacheSeconds; + } + /** * This implementation is empty. diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/support/WebContentGenerator.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/support/WebContentGenerator.java index 5db3572a827..ddda844c5f4 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/support/WebContentGenerator.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/support/WebContentGenerator.java @@ -347,7 +347,32 @@ public abstract class WebContentGenerator extends WebApplicationObjectSupport { else { cControl = CacheControl.empty(); } - checkAndPrepare(request, response, cControl); + checkRequest(request); + if (this.usePreviousHttpCachingBehavior) { + addHttp10CacheHeaders(cacheSeconds, response); + } + else { + String ccValue = cControl.getHeaderValue(); + if (ccValue != null) { + response.setHeader(HEADER_CACHE_CONTROL, ccValue); + } + } + } + + protected final void checkRequest(HttpServletRequest request) throws ServletException { + // Check whether we should support the request method. + String method = request.getMethod(); + if (this.supportedMethods != null && !this.supportedMethods.contains(method)) { + throw new HttpRequestMethodNotSupportedException( + method, StringUtils.toStringArray(this.supportedMethods)); + } + + // Check whether a session is required. + if (this.requireSession) { + if (request.getSession(false) == null) { + throw new HttpSessionRequiredException("Pre-existing session required but none found"); + } + } } /** @@ -366,22 +391,10 @@ public abstract class WebContentGenerator extends WebApplicationObjectSupport { HttpServletRequest request, HttpServletResponse response, CacheControl cacheControl) throws ServletException { - // Check whether we should support the request method. - String method = request.getMethod(); - if (this.supportedMethods != null && !this.supportedMethods.contains(method)) { - throw new HttpRequestMethodNotSupportedException( - method, StringUtils.toStringArray(this.supportedMethods)); - } - - // Check whether a session is required. - if (this.requireSession) { - if (request.getSession(false) == null) { - throw new HttpSessionRequiredException("Pre-existing session required but none found"); - } - } + checkRequest(request); if (this.usePreviousHttpCachingBehavior) { - addHttp10CacheHeaders(response); + addHttp10CacheHeaders(this.cacheSeconds, response); } else if (cacheControl != null) { String ccValue = cacheControl.getHeaderValue(); @@ -391,11 +404,11 @@ public abstract class WebContentGenerator extends WebApplicationObjectSupport { } } - protected void addHttp10CacheHeaders(HttpServletResponse response) { - if (this.cacheSeconds > 0) { - cacheForSeconds(response, this.cacheSeconds, this.alwaysMustRevalidate); + protected final void addHttp10CacheHeaders(int cacheSeconds, HttpServletResponse response) { + if (cacheSeconds > 0) { + cacheForSeconds(response, cacheSeconds, this.alwaysMustRevalidate); } - else if (this.cacheSeconds == 0) { + else if (cacheSeconds == 0) { preventCaching(response); } } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/WebContentInterceptorTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/WebContentInterceptorTests.java index acbce4a2bc1..19340357ec1 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/WebContentInterceptorTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/WebContentInterceptorTests.java @@ -16,9 +16,12 @@ package org.springframework.web.servlet.mvc; +import static org.junit.Assert.*; + import java.util.List; import java.util.Properties; +import org.hamcrest.Matchers; import org.junit.Before; import org.junit.Test; @@ -26,10 +29,9 @@ import org.springframework.mock.web.test.MockHttpServletRequest; import org.springframework.mock.web.test.MockHttpServletResponse; import org.springframework.web.servlet.support.WebContentGenerator; -import static org.junit.Assert.*; - /** * @author Rick Evans + * @author Brian Clozel */ public class WebContentInterceptorTests { @@ -47,19 +49,18 @@ public class WebContentInterceptorTests { @Test - public void preHandleSetsCacheSecondsOnMatchingRequest() throws Exception { + public void cacheResourcesConfiguration() throws Exception { WebContentInterceptor interceptor = new WebContentInterceptor(); interceptor.setCacheSeconds(10); interceptor.preHandle(request, response, null); - List cacheControlHeaders = response.getHeaders("Cache-Control"); - assertNotNull("'Cache-Control' header not set (must be) : null", cacheControlHeaders); - assertTrue("'Cache-Control' header not set (must be) : empty", cacheControlHeaders.size() > 0); + Iterable cacheControlHeaders = response.getHeaders("Cache-Control"); + assertThat(cacheControlHeaders, Matchers.hasItem("max-age=10")); } @Test - public void preHandleSetsCacheSecondsOnMatchingRequestWithCustomCacheMapping() throws Exception { + public void mappedCacheConfigurationOverridesGlobal() throws Exception { Properties mappings = new Properties(); mappings.setProperty("**/*handle.vm", "-1"); @@ -70,46 +71,74 @@ public class WebContentInterceptorTests { request.setRequestURI("http://localhost:7070/example/adminhandle.vm"); interceptor.preHandle(request, response, null); - List cacheControlHeaders = response.getHeaders("Cache-Control"); - assertSame("'Cache-Control' header set must be empty", 0, cacheControlHeaders.size()); + Iterable cacheControlHeaders = response.getHeaders("Cache-Control"); + assertThat(cacheControlHeaders, Matchers.emptyIterable()); request.setRequestURI("http://localhost:7070/example/bingo.html"); interceptor.preHandle(request, response, null); cacheControlHeaders = response.getHeaders("Cache-Control"); - assertNotNull("'Cache-Control' header not set (must be) : null", cacheControlHeaders); - assertTrue("'Cache-Control' header not set (must be) : empty", cacheControlHeaders.size() > 0); + assertThat(cacheControlHeaders, Matchers.hasItem("max-age=10")); } @Test - public void preHandleSetsCacheSecondsOnMatchingRequestWithNoCaching() throws Exception { + public void preventCacheConfiguration() throws Exception { WebContentInterceptor interceptor = new WebContentInterceptor(); interceptor.setCacheSeconds(0); interceptor.preHandle(request, response, null); - List cacheControlHeaders = response.getHeaders("Cache-Control"); - assertNotNull("'Cache-Control' header not set (must be) : null", cacheControlHeaders); - assertTrue("'Cache-Control' header not set (must be) : empty", cacheControlHeaders.size() > 0); + Iterable cacheControlHeaders = response.getHeaders("Cache-Control"); + assertThat(cacheControlHeaders, Matchers.contains("no-store")); } @Test - public void preHandleSetsCacheSecondsOnMatchingRequestWithCachingDisabled() throws Exception { + public void emptyCacheConfiguration() throws Exception { WebContentInterceptor interceptor = new WebContentInterceptor(); interceptor.setCacheSeconds(-1); interceptor.preHandle(request, response, null); - List expiresHeaders = response.getHeaders("Expires"); - assertSame("'Expires' header set (must not be) : empty", 0, expiresHeaders.size()); - List cacheControlHeaders = response.getHeaders("Cache-Control"); - assertSame("'Cache-Control' header set (must not be) : empty", 0, cacheControlHeaders.size()); + Iterable expiresHeaders = response.getHeaders("Expires"); + assertThat(expiresHeaders, Matchers.emptyIterable()); + Iterable cacheControlHeaders = response.getHeaders("Cache-Control"); + assertThat(cacheControlHeaders, Matchers.emptyIterable()); + } + + @SuppressWarnings("deprecation") + @Test + public void http10CachingConfigAndSpecificMapping() throws Exception { + WebContentInterceptor interceptor = new WebContentInterceptor(); + interceptor.setCacheSeconds(0); + interceptor.setAlwaysMustRevalidate(true); + Properties mappings = new Properties(); + mappings.setProperty("**/*.cache.html", "10"); + interceptor.setCacheMappings(mappings); + + request.setRequestURI("http://example.org/foo/page.html"); + interceptor.preHandle(request, response, null); + + Iterable expiresHeaders = response.getHeaders("Expires"); + assertThat(expiresHeaders, Matchers.iterableWithSize(1)); + Iterable cacheControlHeaders = response.getHeaders("Cache-Control"); + assertThat(cacheControlHeaders, Matchers.contains("no-cache", "no-store")); + Iterable pragmaHeaders = response.getHeaders("Pragma"); + assertThat(pragmaHeaders, Matchers.contains("no-cache")); + + response = new MockHttpServletResponse(); + request.setRequestURI("http://example.org/page.cache.html"); + interceptor.preHandle(request, response, null); + + expiresHeaders = response.getHeaders("Expires"); + assertThat(expiresHeaders, Matchers.iterableWithSize(1)); + cacheControlHeaders = response.getHeaders("Cache-Control"); + assertThat(cacheControlHeaders, Matchers.contains("max-age=10, must-revalidate")); } @Test(expected = IllegalArgumentException.class) - public void testSetPathMatcherToNull() throws Exception { - WebContentInterceptor interceptor = new WebContentInterceptor(); - interceptor.setPathMatcher(null); + public void throwsExceptionWithNullPathMatcher() throws Exception { + WebContentInterceptor interceptor = new WebContentInterceptor(); + interceptor.setPathMatcher(null); } }