From b7753a1f29b84ffa146d0961604c477c106a7fff Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Mon, 20 Nov 2017 16:58:26 +0100 Subject: [PATCH] Polish --- .../autoconfigure/web/ResourceProperties.java | 213 +++++++++--------- .../web/servlet/WebMvcAutoConfiguration.java | 10 +- .../web/ResourcePropertiesTests.java | 59 +++-- .../servlet/WebMvcAutoConfigurationTests.java | 24 -- .../appendix-application-properties.adoc | 11 + 5 files changed, 145 insertions(+), 172 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 62c6b54043..57727a0638 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 @@ -19,16 +19,10 @@ 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. @@ -45,7 +39,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/, @@ -54,17 +48,18 @@ public class ResourceProperties { private String[] staticLocations = CLASSPATH_RESOURCE_LOCATIONS; /** - * Cache period for the resources served by the resource handler. If a duration suffix - * is not specified, seconds will be used. + * Cache period for the resources served by the resource handler. + * If a duration suffix is not specified, seconds will be used. + * Can be overridden by the {@code cache-control} property. */ @DurationUnit(ChronoUnit.SECONDS) private Duration cachePeriod; /** - * Cache control headers. Either {@link #cachePeriod} or {@link #cacheControl} can be set. + * Cache control HTTP headers, only allows valid directive combinations. + * Overrides the {@code cache-period} property. */ - @NestedConfigurationProperty - private CacheControlProperties cacheControl; + private CacheControlProperties cacheControl = new CacheControlProperties(); /** * Enable default resource handling. @@ -106,7 +101,6 @@ public class ResourceProperties { this.cacheControl = cacheControl; } - public boolean isAddMappings() { return this.addMappings; } @@ -119,36 +113,6 @@ 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. */ @@ -328,38 +292,83 @@ public class ResourceProperties { } /** - * Configuration for the Cache Control header. + * Configuration for the Cache Control HTTP header. */ - public static class CacheControlProperties { - private Long maxAge; + /** + * Maximum time the response should be cached, + * in seconds if no duration suffix is not specified. + */ + @DurationUnit(ChronoUnit.SECONDS) + private Duration maxAge; + /** + * Indicate that the cached response can be reused only + * if re-validated with the server. + */ private Boolean noCache; + /** + * Indicate to not cache the response in any case. + */ private Boolean noStore; + /** + * Indicate that once it has become stale, a cache must not use + * the response without re-validating it with the server. + */ private Boolean mustRevalidate; + /** + * Indicate intermediaries (caches and others) that they should + * not transform the response content. + */ private Boolean noTransform; + /** + * Indicate that any cache may store the response. + */ private Boolean cachePublic; + /** + * Indicate that the response message is intended for a single user + * and must not be stored by a shared cache. + */ private Boolean cachePrivate; + /** + * Same meaning as the "must-revalidate" directive, + * except that it does not apply to private caches. + */ private Boolean proxyRevalidate; - private Long staleWhileRevalidate; + /** + * Maximum time the response can be served after it becomes stale, + * in seconds if no duration suffix is not specified. + */ + @DurationUnit(ChronoUnit.SECONDS) + private Duration staleWhileRevalidate; - private Long staleIfError; + /** + * Maximum time the response may be used when errors are encountered, + * in seconds if no duration suffix is not specified. + */ + @DurationUnit(ChronoUnit.SECONDS) + private Duration staleIfError; - private Long sMaxAge; + /** + * Maximum time the response should be cached by shared caches, + * in seconds if no duration suffix is not specified. + */ + @DurationUnit(ChronoUnit.SECONDS) + private Duration sMaxAge; - public Long getMaxAge() { + public Duration getMaxAge() { return this.maxAge; } - public void setMaxAge(Long maxAge) { + public void setMaxAge(Duration maxAge) { this.maxAge = maxAge; } @@ -419,91 +428,71 @@ public class ResourceProperties { this.proxyRevalidate = proxyRevalidate; } - public Long getStaleWhileRevalidate() { + public Duration getStaleWhileRevalidate() { return this.staleWhileRevalidate; } - public void setStaleWhileRevalidate(Long staleWhileRevalidate) { + public void setStaleWhileRevalidate(Duration staleWhileRevalidate) { this.staleWhileRevalidate = staleWhileRevalidate; } - public Long getStaleIfError() { + public Duration getStaleIfError() { return this.staleIfError; } - public void setStaleIfError(Long staleIfError) { + public void setStaleIfError(Duration staleIfError) { this.staleIfError = staleIfError; } - public Long getsMaxAge() { + public Duration getsMaxAge() { return this.sMaxAge; } - public void setsMaxAge(Long sMaxAge) { + public void setsMaxAge(Duration 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(); - } + public CacheControl toHttpCacheControl() { + CacheControl cc; if (Boolean.TRUE.equals(this.noStore)) { - return CacheControl.noStore(); + cc = 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(); + else if (Boolean.TRUE.equals(this.noCache)) { + cc = CacheControl.noCache(); } - 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); + else if (this.maxAge != null) { + cc = CacheControl.maxAge(this.maxAge.getSeconds(), TimeUnit.SECONDS); } - return cacheControl; + else { + cc = CacheControl.empty(); + } + if (Boolean.TRUE.equals(this.mustRevalidate)) { + cc.mustRevalidate(); + } + if (Boolean.TRUE.equals(this.noTransform)) { + cc.noTransform(); + } + if (Boolean.TRUE.equals(this.cachePublic)) { + cc.cachePublic(); + } + if (Boolean.TRUE.equals(this.cachePrivate)) { + cc.cachePrivate(); + } + if (Boolean.TRUE.equals(this.proxyRevalidate)) { + cc.proxyRevalidate(); + } + if (this.staleWhileRevalidate != null) { + cc.staleWhileRevalidate(this.staleWhileRevalidate.getSeconds(), TimeUnit.SECONDS); + } + if (this.staleIfError != null) { + cc.staleIfError(this.staleIfError.getSeconds(), TimeUnit.SECONDS); + } + if (this.sMaxAge != null) { + cc.sMaxAge(this.sMaxAge.getSeconds(), TimeUnit.SECONDS); + } + return cc; } } - } 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 529c8b31eb..584c00d531 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 @@ -309,14 +309,14 @@ public class WebMvcAutoConfiguration { return; } Duration cachePeriod = this.resourceProperties.getCachePeriod(); - CacheControl cacheControl = this.resourceProperties.createCacheControl(); + CacheControl cacheControl = this.resourceProperties.getCacheControl().toHttpCacheControl(); if (!registry.hasMappingForPattern("/webjars/**")) { customizeResourceHandlerRegistration( registry.addResourceHandler("/webjars/**") .addResourceLocations( "classpath:/META-INF/resources/webjars/") - .setCachePeriod(getSeconds(cachePeriod)) - .setCacheControl(cacheControl)); + .setCachePeriod(getSeconds(cachePeriod)) + .setCacheControl(cacheControl)); } String staticPathPattern = this.mvcProperties.getStaticPathPattern(); if (!registry.hasMappingForPattern(staticPathPattern)) { @@ -324,8 +324,8 @@ public class WebMvcAutoConfiguration { registry.addResourceHandler(staticPathPattern) .addResourceLocations(getResourceLocations( this.resourceProperties.getStaticLocations())) - .setCachePeriod(getSeconds(cachePeriod)) - .setCacheControl(cacheControl)); + .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 629d4175ba..564423edc5 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 @@ -16,7 +16,11 @@ package org.springframework.boot.autoconfigure.web; +import java.time.Duration; + +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.springframework.boot.autoconfigure.web.ResourceProperties.CacheControlProperties; import org.springframework.boot.testsupport.assertj.Matched; @@ -34,6 +38,9 @@ public class ResourcePropertiesTests { private final ResourceProperties properties = new ResourceProperties(); + @Rule + public ExpectedException thrown = ExpectedException.none(); + @Test public void resourceChainNoCustomization() { assertThat(this.properties.getChain().getEnabled()).isNull(); @@ -70,53 +77,43 @@ public class ResourcePropertiesTests { } @Test - public void cachePeriod() { - this.properties.setCachePeriod(5); - assertThat(this.properties.createCacheControl().getHeaderValue()) - .isEqualTo("max-age=5"); + public void emptyCacheControl() { + CacheControlProperties cacheControl = new CacheControlProperties(); + this.properties.setCacheControl(cacheControl); + assertThat(this.properties.getCacheControl().toHttpCacheControl().getHeaderValue()).isNull(); } @Test public void cacheControlAllPropertiesSet() { CacheControlProperties cacheControl = new CacheControlProperties(); + cacheControl.setMaxAge(Duration.ofSeconds(4)); 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); + cacheControl.setsMaxAge(Duration.ofSeconds(5)); + cacheControl.setStaleIfError(Duration.ofSeconds(6)); + cacheControl.setStaleWhileRevalidate(Duration.ofSeconds(7)); 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"); + assertThat(this.properties.getCacheControl().toHttpCacheControl().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 invalidCacheControlCombination() { + CacheControlProperties cacheControl = new CacheControlProperties(); + cacheControl.setMaxAge(Duration.ofSeconds(4)); + cacheControl.setNoStore(true); + this.properties.setCacheControl(cacheControl); + assertThat(this.properties.getCacheControl().toHttpCacheControl().getHeaderValue()).isEqualTo("no-store"); } @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(); + assertThat(this.properties.getCacheControl().toHttpCacheControl().getHeaderValue()).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 eb229451ac..1fa2f49841 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 @@ -99,9 +99,7 @@ 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; @@ -852,28 +850,6 @@ public class WebMvcAutoConfigurationTests { } } - @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( diff --git a/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc b/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc index c196d8024c..5a06cff8f9 100644 --- a/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc +++ b/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc @@ -398,6 +398,17 @@ content into your application; rather pick only the properties that you need. # SPRING RESOURCES HANDLING ({sc-spring-boot-autoconfigure}/web/ResourceProperties.{sc-ext}[ResourceProperties]) spring.resources.add-mappings=true # Enable default resource handling. + spring.resources.cache-control.max-age= # Maximum time the response should be cached, in seconds if no duration suffix is not specified. + spring.resources.cache-control.no-cache= # Indicate that the cached response can be reused only if re-validated with the server. + spring.resources.cache-control.no-store= # Indicate to not cache the response in any case. + spring.resources.cache-control.must-revalidate= # Indicate that once it has become stale, a cache must not use the response without re-validating it with the server. + spring.resources.cache-control.no-transform= # Indicate intermediaries (caches and others) that they should not transform the response content. + spring.resources.cache-control.cache-public= # Indicate that any cache may store the response. + spring.resources.cache-control.cache-private= # Indicate that the response message is intended for a single user and must not be stored by a shared cache. + spring.resources.cache-control.proxy-revalidate= # Same meaning as the "must-revalidate" directive, except that it does not apply to private caches. + spring.resources.cache-control.stale-while-revalidate= # Maximum time the response can be served after it becomes stale, in seconds if no duration suffix is not specified. + spring.resources.cache-control.stale-if-error= # Maximum time the response may be used when errors are encountered, in seconds if no duration suffix is not specified. + spring.resources.cache-control.s-max-age= # Maximum time the response should be cached by shared caches, in seconds if no duration suffix is not specified. spring.resources.cache-period= # Cache period for the resources served by the resource handler. If a duration suffix is not specified, seconds will be used. spring.resources.chain.cache=true # Enable caching in the Resource chain. spring.resources.chain.enabled= # Enable the Spring Resource Handling chain. Disabled by default unless at least one strategy has been enabled.