From e2bc90b6bb20f26f607290327fcfc67995f77043 Mon Sep 17 00:00:00 2001 From: tinexw Date: Wed, 4 Oct 2017 23:46:12 +0200 Subject: [PATCH] Add CacheControl config keys in spring.resources.* This commit adds several configuration keys for customizing the "Cache-Control" HTTP response header when serving static resources. New keys are located in the "spring.resources.cache-control.*" namespace; anything configured there will prevail on existing "spring.resources.cache-period=" values, so as to mirror Spring MVC's behavior. Fixes gh-9432 --- .../autoconfigure/web/ResourceProperties.java | 244 +++++++++++++++++- .../web/servlet/WebMvcAutoConfiguration.java | 9 +- .../web/ResourcePropertiesTests.java | 52 ++++ .../servlet/WebMvcAutoConfigurationTests.java | 95 ++++++- 4 files changed, 387 insertions(+), 13 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ResourceProperties.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ResourceProperties.java index b03b0139ac..62c6b54043 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ResourceProperties.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ResourceProperties.java @@ -18,9 +18,17 @@ package org.springframework.boot.autoconfigure.web; import java.time.Duration; import java.time.temporal.ChronoUnit; +import java.util.concurrent.TimeUnit; +import java.util.function.BiFunction; +import java.util.function.Supplier; + +import javax.annotation.PostConstruct; import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.NestedConfigurationProperty; import org.springframework.boot.context.properties.bind.convert.DurationUnit; +import org.springframework.http.CacheControl; +import org.springframework.util.Assert; /** * Properties used to configure resource handling. @@ -29,6 +37,7 @@ import org.springframework.boot.context.properties.bind.convert.DurationUnit; * @author Brian Clozel * @author Dave Syer * @author Venil Noronha + * @author Kristine Jetzke * @since 1.1.0 */ @ConfigurationProperties(prefix = "spring.resources", ignoreUnknownFields = false) @@ -36,7 +45,7 @@ public class ResourceProperties { private static final String[] CLASSPATH_RESOURCE_LOCATIONS = { "classpath:/META-INF/resources/", "classpath:/resources/", - "classpath:/static/", "classpath:/public/" }; + "classpath:/static/", "classpath:/public/"}; /** * Locations of static resources. Defaults to classpath:[/META-INF/resources/, @@ -51,6 +60,12 @@ public class ResourceProperties { @DurationUnit(ChronoUnit.SECONDS) private Duration cachePeriod; + /** + * Cache control headers. Either {@link #cachePeriod} or {@link #cacheControl} can be set. + */ + @NestedConfigurationProperty + private CacheControlProperties cacheControl; + /** * Enable default resource handling. */ @@ -83,6 +98,15 @@ public class ResourceProperties { this.cachePeriod = cachePeriod; } + public CacheControlProperties getCacheControl() { + return this.cacheControl; + } + + public void setCacheControl(CacheControlProperties cacheControl) { + this.cacheControl = cacheControl; + } + + public boolean isAddMappings() { return this.addMappings; } @@ -95,6 +119,36 @@ public class ResourceProperties { return this.chain; } + public CacheControl createCacheControl() { + if (this.cachePeriod != null) { + return CacheControl.maxAge(this.cachePeriod.getSeconds(), TimeUnit.SECONDS); + } + if (this.cacheControl != null) { + return this.cacheControl.transformToHttpSpringCacheControl(); + } + return null; + } + + @PostConstruct + public void checkIncompatibleCacheOptions() { + Assert.state(this.cachePeriod == null || this.cacheControl == null, + "Only one of cache-period or cache-control may be set."); + if (this.cacheControl != null) { + if (this.cacheControl.getMaxAge() != null) { + Assert.state(!Boolean.TRUE.equals(this.cacheControl.getNoCache()), "no-cache may not be set if max-age is set."); + Assert.state(!Boolean.TRUE.equals(this.cacheControl.getNoStore()), "no-store may not be set if max-age is set."); + } + if (this.cacheControl.getNoCache() != null) { + Assert.state(this.cacheControl.getMaxAge() == null, "max-age may not be set if no-cache is set."); + Assert.state(!Boolean.TRUE.equals(this.cacheControl.getNoStore()), "no-store may not be set if no-cache is set."); + } + if (this.cacheControl.getNoStore() != null) { + Assert.state(this.cacheControl.getMaxAge() == null, "max-age may not be set if no-store is set."); + Assert.state(!Boolean.TRUE.equals(this.cacheControl.getNoCache()), "no-cache may not be set if no-store is set."); + } + } + } + /** * Configuration for the Spring Resource Handling chain. */ @@ -127,8 +181,9 @@ public class ResourceProperties { /** * Return whether the resource chain is enabled. Return {@code null} if no * specific settings are present. - * @return whether the resource chain is enabled or {@code null} if no specified - * settings are present. + * + * @return whether the resource chain is enabled or {@code null} if no specified settings are + * present. */ public Boolean getEnabled() { return getEnabled(getStrategy().getFixed().isEnabled(), @@ -206,7 +261,7 @@ public class ResourceProperties { /** * Comma-separated list of patterns to apply to the Version Strategy. */ - private String[] paths = new String[] { "/**" }; + private String[] paths = new String[]{"/**"}; public boolean isEnabled() { return this.enabled; @@ -239,7 +294,7 @@ public class ResourceProperties { /** * Comma-separated list of patterns to apply to the Version Strategy. */ - private String[] paths = new String[] { "/**" }; + private String[] paths = new String[]{"/**"}; /** * Version string to use for the Version Strategy. @@ -272,4 +327,183 @@ public class ResourceProperties { } + /** + * Configuration for the Cache Control header. + */ + + public static class CacheControlProperties { + + private Long maxAge; + + private Boolean noCache; + + private Boolean noStore; + + private Boolean mustRevalidate; + + private Boolean noTransform; + + private Boolean cachePublic; + + private Boolean cachePrivate; + + private Boolean proxyRevalidate; + + private Long staleWhileRevalidate; + + private Long staleIfError; + + private Long sMaxAge; + + public Long getMaxAge() { + return this.maxAge; + } + + public void setMaxAge(Long maxAge) { + this.maxAge = maxAge; + } + + public Boolean getNoCache() { + return this.noCache; + } + + public void setNoCache(Boolean noCache) { + this.noCache = noCache; + } + + public Boolean getNoStore() { + return this.noStore; + } + + public void setNoStore(Boolean noStore) { + this.noStore = noStore; + } + + public Boolean getMustRevalidate() { + return this.mustRevalidate; + } + + public void setMustRevalidate(Boolean mustRevalidate) { + this.mustRevalidate = mustRevalidate; + } + + public Boolean getNoTransform() { + return this.noTransform; + } + + public void setNoTransform(Boolean noTransform) { + this.noTransform = noTransform; + } + + public Boolean getCachePublic() { + return this.cachePublic; + } + + public void setCachePublic(Boolean cachePublic) { + this.cachePublic = cachePublic; + } + + public Boolean getCachePrivate() { + return this.cachePrivate; + } + + public void setCachePrivate(Boolean cachePrivate) { + this.cachePrivate = cachePrivate; + } + + public Boolean getProxyRevalidate() { + return this.proxyRevalidate; + } + + public void setProxyRevalidate(Boolean proxyRevalidate) { + this.proxyRevalidate = proxyRevalidate; + } + + public Long getStaleWhileRevalidate() { + return this.staleWhileRevalidate; + } + + public void setStaleWhileRevalidate(Long staleWhileRevalidate) { + this.staleWhileRevalidate = staleWhileRevalidate; + } + + public Long getStaleIfError() { + return this.staleIfError; + } + + public void setStaleIfError(Long staleIfError) { + this.staleIfError = staleIfError; + } + + public Long getsMaxAge() { + return this.sMaxAge; + } + + public void setsMaxAge(Long sMaxAge) { + this.sMaxAge = sMaxAge; + } + + CacheControl transformToHttpSpringCacheControl() { + CacheControl httpSpringCacheControl = initCacheControl(); + httpSpringCacheControl = setFlags(httpSpringCacheControl); + httpSpringCacheControl = setTimes(httpSpringCacheControl); + return httpSpringCacheControl; + } + + private CacheControl initCacheControl() { + if (this.maxAge != null) { + return CacheControl.maxAge(this.maxAge, TimeUnit.SECONDS); + } + if (Boolean.TRUE.equals(this.noCache)) { + return CacheControl.noCache(); + } + if (Boolean.TRUE.equals(this.noStore)) { + return CacheControl.noStore(); + } + return CacheControl.empty(); + } + + private CacheControl setFlags(CacheControl cacheControl) { + cacheControl = setBoolean(this.mustRevalidate, cacheControl::mustRevalidate, + cacheControl); + cacheControl = setBoolean(this.noTransform, cacheControl::noTransform, + cacheControl); + cacheControl = setBoolean(this.cachePublic, cacheControl::cachePublic, + cacheControl); + cacheControl = setBoolean(this.cachePrivate, cacheControl::cachePrivate, + cacheControl); + cacheControl = setBoolean(this.proxyRevalidate, cacheControl::proxyRevalidate, + cacheControl); + return cacheControl; + } + + private static CacheControl setBoolean(Boolean value, + Supplier setter, CacheControl cacheControl) { + if (Boolean.TRUE.equals(value)) { + return setter.get(); + } + return cacheControl; + } + + private CacheControl setTimes(CacheControl cacheControl) { + cacheControl = setLong(this.staleWhileRevalidate, + cacheControl::staleWhileRevalidate, cacheControl); + cacheControl = setLong(this.staleIfError, cacheControl::staleIfError, + cacheControl); + cacheControl = setLong(this.sMaxAge, cacheControl::sMaxAge, cacheControl); + return cacheControl; + } + + private static CacheControl setLong(Long value, + BiFunction setter, + CacheControl cacheControl) { + if (value != null) { + return setter.apply(value, TimeUnit.SECONDS); + } + return cacheControl; + } + + } + + } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/WebMvcAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/WebMvcAutoConfiguration.java index 83dbdbb18f..529c8b31eb 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/WebMvcAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/WebMvcAutoConfiguration.java @@ -73,6 +73,7 @@ import org.springframework.core.io.ResourceLoader; import org.springframework.format.Formatter; import org.springframework.format.FormatterRegistry; import org.springframework.format.datetime.DateFormatter; +import org.springframework.http.CacheControl; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.http.converter.HttpMessageConverter; @@ -133,6 +134,7 @@ import org.springframework.web.servlet.view.InternalResourceViewResolver; * @author Sébastien Deleuze * @author Eddú Meléndez * @author Stephane Nicoll + * @author Kristine Jetzke */ @Configuration @ConditionalOnWebApplication(type = Type.SERVLET) @@ -307,12 +309,14 @@ public class WebMvcAutoConfiguration { return; } Duration cachePeriod = this.resourceProperties.getCachePeriod(); + CacheControl cacheControl = this.resourceProperties.createCacheControl(); if (!registry.hasMappingForPattern("/webjars/**")) { customizeResourceHandlerRegistration( registry.addResourceHandler("/webjars/**") .addResourceLocations( "classpath:/META-INF/resources/webjars/") - .setCachePeriod(getSeconds(cachePeriod))); + .setCachePeriod(getSeconds(cachePeriod)) + .setCacheControl(cacheControl)); } String staticPathPattern = this.mvcProperties.getStaticPathPattern(); if (!registry.hasMappingForPattern(staticPathPattern)) { @@ -320,7 +324,8 @@ public class WebMvcAutoConfiguration { registry.addResourceHandler(staticPathPattern) .addResourceLocations(getResourceLocations( this.resourceProperties.getStaticLocations())) - .setCachePeriod(getSeconds(cachePeriod))); + .setCachePeriod(getSeconds(cachePeriod)) + .setCacheControl(cacheControl)); } } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ResourcePropertiesTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ResourcePropertiesTests.java index 8621b1b4b6..629d4175ba 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ResourcePropertiesTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ResourcePropertiesTests.java @@ -18,6 +18,7 @@ package org.springframework.boot.autoconfigure.web; import org.junit.Test; +import org.springframework.boot.autoconfigure.web.ResourceProperties.CacheControlProperties; import org.springframework.boot.testsupport.assertj.Matched; import static org.assertj.core.api.Assertions.assertThat; @@ -27,6 +28,7 @@ import static org.hamcrest.CoreMatchers.endsWith; * Tests for {@link ResourceProperties}. * * @author Stephane Nicoll + * @author Kristine Jetzke */ public class ResourcePropertiesTests { @@ -67,4 +69,54 @@ public class ResourcePropertiesTests { assertThat(actual).containsExactly("/foo/", "/bar/", "/baz/"); } + @Test + public void cachePeriod() { + this.properties.setCachePeriod(5); + assertThat(this.properties.createCacheControl().getHeaderValue()) + .isEqualTo("max-age=5"); + } + + @Test + public void cacheControlAllPropertiesSet() { + CacheControlProperties cacheControl = new CacheControlProperties(); + cacheControl.setCachePrivate(true); + cacheControl.setCachePublic(true); + cacheControl.setMaxAge(4L); + cacheControl.setMustRevalidate(true); + cacheControl.setNoCache(true); + cacheControl.setNoCache(true); + cacheControl.setNoStore(true); + cacheControl.setNoTransform(true); + cacheControl.setProxyRevalidate(true); + cacheControl.setsMaxAge(5L); + cacheControl.setStaleIfError(6L); + cacheControl.setStaleWhileRevalidate(7L); + this.properties.setCacheControl(cacheControl); + assertThat(this.properties.createCacheControl().getHeaderValue()).isEqualTo( + "max-age=4, must-revalidate, no-transform, public, private, proxy-revalidate, s-maxage=5, stale-if-error=6, stale-while-revalidate=7"); + } + + @Test + public void cacheControlNoPropertiesSet() { + this.properties.setCacheControl(new CacheControlProperties()); + assertThat(this.properties.createCacheControl().getHeaderValue()).isNull(); + } + + @Test + public void cacheControlAndCachePeriodSet() { + CacheControlProperties cacheControl = new CacheControlProperties(); + cacheControl.setMaxAge(12L); + this.properties.setCacheControl(cacheControl); + this.properties.setCachePeriod(6); + assertThat(this.properties.createCacheControl().getHeaderValue()) + .isEqualTo("max-age=6"); + } + + @Test + public void cacheControlAndCachePeriodBothNotSet() { + this.properties.setCacheControl(null); + this.properties.setCachePeriod(null); + assertThat(this.properties.createCacheControl()).isNull(); + } + } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/WebMvcAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/WebMvcAutoConfigurationTests.java index df2b4031b4..eb229451ac 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/WebMvcAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/WebMvcAutoConfigurationTests.java @@ -16,11 +16,13 @@ package org.springframework.boot.autoconfigure.web.servlet; +import java.util.Collections; import java.util.Date; import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.concurrent.TimeUnit; import java.util.function.Consumer; import javax.servlet.http.HttpServletRequest; @@ -52,6 +54,7 @@ import org.springframework.core.convert.ConversionService; import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.Resource; import org.springframework.format.support.FormattingConversionService; +import org.springframework.http.CacheControl; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.http.converter.HttpMessageConverter; @@ -96,7 +99,9 @@ import org.springframework.web.servlet.resource.VersionStrategy; import org.springframework.web.servlet.view.AbstractView; import org.springframework.web.servlet.view.ContentNegotiatingViewResolver; + import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.mock; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.forwardedUrl; @@ -111,6 +116,7 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers. * @author Stephane Nicoll * @author Brian Clozel * @author Eddú Meléndez + * @author Kristine Jetzke */ public class WebMvcAutoConfigurationTests { @@ -797,6 +803,77 @@ public class WebMvcAutoConfigurationTests { .run((context) -> assertThat(context).hasNotFailed()); } + @Test + public void cachePeriod() throws Exception { + this.contextRunner.withPropertyValues("spring.resources.cache-period:5") + .run((context) -> { + assertCachePeriod(context); + }); + } + + private void assertCachePeriod(AssertableWebApplicationContext context) { + Map handlerMap = getHandlerMap(context + .getBean("resourceHandlerMapping", HandlerMapping.class)); + assertThat(handlerMap).hasSize(2); + for (Object handler : handlerMap.keySet()) { + if (handler instanceof ResourceHttpRequestHandler) { + assertThat(((ResourceHttpRequestHandler) handler) + .getCacheSeconds()).isEqualTo(-1); + assertThat(((ResourceHttpRequestHandler) handler) + .getCacheControl()).isEqualToComparingFieldByField( + CacheControl.maxAge(5, TimeUnit.SECONDS)); + } + } + } + + @Test + public void cacheControl() throws Exception { + this.contextRunner + .withPropertyValues("spring.resources.cache-control.max-age:5", + "spring.resources.cache-control.proxy-revalidate:true") + .run((context) -> { + assertCacheControl(context); + }); + } + + private void assertCacheControl(AssertableWebApplicationContext context) { + Map handlerMap = getHandlerMap(context + .getBean("resourceHandlerMapping", HandlerMapping.class)); + assertThat(handlerMap).hasSize(2); + for (Object handler : handlerMap.keySet()) { + if (handler instanceof ResourceHttpRequestHandler) { + assertThat(((ResourceHttpRequestHandler) handler) + .getCacheSeconds()).isEqualTo(-1); + assertThat(((ResourceHttpRequestHandler) handler) + .getCacheControl()).isEqualToComparingFieldByField( + CacheControl.maxAge(5, TimeUnit.SECONDS) + .proxyRevalidate()); + } + } + } + + @Test + public void invalidCacheConfig() throws Exception { + assertThatThrownBy(() -> this.contextRunner + .withPropertyValues("spring.resources.cache-control.max-age:5", + "spring.resources.cache-period:6") + .run((context) -> getHandlerMap( + context.getBean("resourceHandlerMapping", HandlerMapping.class)))) + .hasRootCauseInstanceOf(IllegalStateException.class) + .hasStackTraceContaining("Only one of cache-period or cache-control may be set"); + } + + @Test + public void invalidCacheControl() throws Exception { + assertThatThrownBy(() -> this.contextRunner + .withPropertyValues("spring.resources.cache-control.max-age:5", + "spring.resources.cache-control.no-cache:true") + .run((context) -> getHandlerMap( + context.getBean("resourceHandlerMapping", HandlerMapping.class)))) + .hasRootCauseInstanceOf(IllegalStateException.class) + .hasStackTraceContaining("no-cache may not be set if max-age is set"); + } + protected Map> getFaviconMappingLocations( ApplicationContext context) { return getMappingLocations( @@ -829,15 +906,21 @@ public class WebMvcAutoConfigurationTests { @SuppressWarnings("unchecked") protected Map> getMappingLocations(HandlerMapping mapping) { Map> mappingLocations = new LinkedHashMap<>(); - if (mapping instanceof SimpleUrlHandlerMapping) { - ((SimpleUrlHandlerMapping) mapping).getHandlerMap().forEach((key, value) -> { - Object locations = ReflectionTestUtils.getField(value, "locations"); - mappingLocations.put(key, (List) locations); - }); - } + getHandlerMap(mapping).forEach((key, value) -> { + Object locations = ReflectionTestUtils.getField(value, "locations"); + mappingLocations.put(key, (List) locations); + }); return mappingLocations; } + protected Map getHandlerMap(HandlerMapping mapping) { + if (mapping instanceof SimpleUrlHandlerMapping) { + return ((SimpleUrlHandlerMapping) mapping).getHandlerMap(); + } + return Collections.emptyMap(); + } + + @Configuration protected static class ViewConfig {