From 0b02551e2f8f8ae7107b2303c10432542a201fbe Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Thu, 28 Aug 2014 22:32:38 -0400 Subject: [PATCH] Update resource handler Java config This change separates out resource chain related methods previously in ResourceHandlerRegistration into a new class ResourceChainRegistration with the goal of improving readability. Along with that, the registration of caching resolver and transformer is now explicitly controled with a boolean flag (on the method used to obtain the ResourceChainRegistration) and an overloaded method also allows providing a Cache instance. Issue: SPR-12124 --- .../annotation/ResourceChainRegistration.java | 118 ++++++++++ .../ResourceHandlerRegistration.java | 213 +++++------------- .../ResourceHandlerRegistryTests.java | 67 +++--- .../ResourceUrlProviderJavaConfigTests.java | 2 +- 4 files changed, 199 insertions(+), 201 deletions(-) create mode 100644 spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ResourceChainRegistration.java diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ResourceChainRegistration.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ResourceChainRegistration.java new file mode 100644 index 0000000000..4b110d353e --- /dev/null +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ResourceChainRegistration.java @@ -0,0 +1,118 @@ +/* + * Copyright 2002-2014 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.web.servlet.config.annotation; + +import org.springframework.cache.Cache; +import org.springframework.cache.concurrent.ConcurrentMapCache; +import org.springframework.util.Assert; +import org.springframework.web.servlet.resource.CachingResourceResolver; +import org.springframework.web.servlet.resource.CachingResourceTransformer; +import org.springframework.web.servlet.resource.CssLinkResourceTransformer; +import org.springframework.web.servlet.resource.PathResourceResolver; +import org.springframework.web.servlet.resource.ResourceResolver; +import org.springframework.web.servlet.resource.ResourceTransformer; +import org.springframework.web.servlet.resource.VersionResourceResolver; + +import java.util.ArrayList; +import java.util.List; + +/** + * Assists with the registration of resource resolvers and transformers. + * + * @author Rossen Stoyanchev + * @since 4.1 + */ +public class ResourceChainRegistration { + + private static final String DEFAULT_CACHE_NAME = "spring-resource-chain-cache"; + + private final List resolvers = new ArrayList(4); + + private final List transformers = new ArrayList(4); + + private boolean hasVersionResolver; + + private boolean hasPathResolver; + + private boolean hasCssLinkTransformer; + + + public ResourceChainRegistration(boolean cacheResources) { + this(cacheResources, cacheResources ? new ConcurrentMapCache(DEFAULT_CACHE_NAME) : null); + } + + public ResourceChainRegistration(boolean cacheResources, Cache cache) { + Assert.isTrue(!cacheResources || cache != null, "'cache' is required when cacheResources=true"); + if (cacheResources) { + this.resolvers.add(new CachingResourceResolver(cache)); + this.transformers.add(new CachingResourceTransformer(cache)); + } + } + + + /** + * Add a resource resolver to the chain. + * @param resolver the resolver to add + * @return the current instance for chained method invocation + */ + public ResourceChainRegistration addResolver(ResourceResolver resolver) { + Assert.notNull(resolver, "The provided ResourceResolver should not be null"); + this.resolvers.add(resolver); + if (resolver instanceof VersionResourceResolver) { + this.hasVersionResolver = true; + } + else if (resolver instanceof PathResourceResolver) { + this.hasPathResolver = true; + } + return this; + } + + /** + * Add a resource transformer to the chain. + * @param transformer the transformer to add + * @return the current instance for chained method invocation + */ + public ResourceChainRegistration addTransformer(ResourceTransformer transformer) { + Assert.notNull(transformer, "The provided ResourceTransformer should not be null"); + this.transformers.add(transformer); + if (transformer instanceof CssLinkResourceTransformer) { + this.hasCssLinkTransformer = true; + } + return this; + } + + protected List getResourceResolvers() { + if (!this.hasPathResolver) { + List result = new ArrayList(this.resolvers); + result.add(new PathResourceResolver()); + return result; + } + return this.resolvers; + } + + protected List getResourceTransformers() { + if (this.hasVersionResolver && !this.hasCssLinkTransformer) { + List result = new ArrayList(this.transformers); + boolean hasTransformers = !this.transformers.isEmpty(); + boolean hasCaching = hasTransformers && this.transformers.get(0) instanceof CachingResourceTransformer; + result.add(hasCaching ? 1 : 0, new CssLinkResourceTransformer()); + return result; + } + return this.transformers; + } + +} diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ResourceHandlerRegistration.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ResourceHandlerRegistration.java index 24ec91a1f4..ae32369f28 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ResourceHandlerRegistration.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ResourceHandlerRegistration.java @@ -16,27 +16,16 @@ package org.springframework.web.servlet.config.annotation; -import java.util.ArrayList; -import java.util.List; - import org.springframework.cache.Cache; -import org.springframework.cache.concurrent.ConcurrentMapCache; import org.springframework.core.io.Resource; import org.springframework.core.io.ResourceLoader; import org.springframework.util.Assert; -import org.springframework.util.ClassUtils; import org.springframework.util.CollectionUtils; -import org.springframework.web.servlet.resource.CachingResourceResolver; -import org.springframework.web.servlet.resource.CachingResourceTransformer; -import org.springframework.web.servlet.resource.ContentVersionStrategy; -import org.springframework.web.servlet.resource.CssLinkResourceTransformer; -import org.springframework.web.servlet.resource.FixedVersionStrategy; import org.springframework.web.servlet.resource.PathResourceResolver; import org.springframework.web.servlet.resource.ResourceHttpRequestHandler; -import org.springframework.web.servlet.resource.ResourceResolver; -import org.springframework.web.servlet.resource.ResourceTransformer; -import org.springframework.web.servlet.resource.VersionResourceResolver; -import org.springframework.web.servlet.resource.VersionStrategy; + +import java.util.ArrayList; +import java.util.List; /** * Encapsulates information required to create a resource handlers. @@ -49,8 +38,6 @@ import org.springframework.web.servlet.resource.VersionStrategy; */ public class ResourceHandlerRegistration { - private static final String RESOURCE_CACHE_NAME = "spring-resourcehandler-cache"; - private final ResourceLoader resourceLoader; private final String[] pathPatterns; @@ -59,17 +46,7 @@ public class ResourceHandlerRegistration { private Integer cachePeriod; - private List customResolvers = new ArrayList(); - - private List customTransformers = new ArrayList(); - - private boolean hasVersionResolver; - - private boolean hasCssLinkTransformer; - - private boolean isDevMode = false; - - private Cache resourceCache; + private ResourceChainRegistration resourceChainRegistration; /** @@ -100,78 +77,6 @@ public class ResourceHandlerRegistration { return this; } - /** - * Add a {@code ResourceResolver} to the chain, allowing to resolve server-side resources from - * HTTP requests. - * - *

{@link ResourceResolver}s are registered, in the following order: - *

    - *
  1. a {@link org.springframework.web.servlet.resource.CachingResourceResolver} - * for caching the results of the next Resolvers; this resolver is only registered if you - * did not provide your own instance of {@link CachingResourceResolver} at the beginning of the chain
    2. - *
  2. all {@code ResourceResolver}s registered using this method, in the order of methods calls
    3. - *
  3. a {@link VersionResourceResolver} if a versioning configuration has been applied with - * {@code addVersionStrategy}, {@code addVersion}, etc.
    4. - *
  4. a {@link PathResourceResolver} for resolving resources on the file system
    5. - *
- * - * @param resolver a {@link ResourceResolver} to add to the chain of resolvers - * @return the same {@link ResourceHandlerRegistration} instance for chained method invocation - * @see ResourceResolver - * @since 4.1 - */ - public ResourceHandlerRegistration addResolver(ResourceResolver resolver) { - Assert.notNull(resolver, "The provided ResourceResolver should not be null"); - this.customResolvers.add(resolver); - if (resolver instanceof VersionResourceResolver) { - this.hasVersionResolver = true; - } - return this; - } - - /** - * Add a {@code ResourceTransformer} to the chain, allowing to transform the content - * of server-side resources when serving them to HTTP clients. - * - *

{@link ResourceTransformer}s are registered, in the following order: - *

    - *
  1. a {@link org.springframework.web.servlet.resource.CachingResourceTransformer} - * for caching the results of the next Transformers; this transformer is only registered if you - * did not provide your own instance of {@link CachingResourceTransformer} at the beginning of the chain
    2. - *
  2. a {@link CssLinkResourceTransformer} for updating links within CSS files; this transformer - * is only registered if a versioning configuration has been applied with {@code addVersionStrategy}, - * {@code addVersion}, etc
    3. - *
  3. all {@code ResourceTransformer}s registered using this method, in the order of methods calls
    4. - *
- * - * @param transformer a {@link ResourceTransformer} to add to the chain of transformers - * @return the same {@link ResourceHandlerRegistration} instance for chained method invocation - * @see ResourceResolver - * @since 4.1 - */ - public ResourceHandlerRegistration addTransformer(ResourceTransformer transformer) { - Assert.notNull(transformer, "The provided ResourceTransformer should not be null"); - this.customTransformers.add(transformer); - if (transformer instanceof CssLinkResourceTransformer) { - this.hasCssLinkTransformer = true; - } - return this; - } - - /** - * Disable automatic registration of caching Resolver/Transformer, thus disabling {@code Resource} caching - * if no caching Resolver/Transformer was manually registered. - *

Useful when updating static resources at runtime, i.e. during the development phase.

- * @return the same {@link ResourceHandlerRegistration} instance for chained method invocation - * @see ResourceResolver - * @see ResourceTransformer - * @since 4.1 - */ - public ResourceHandlerRegistration enableDevMode() { - this.isDevMode = true; - return this; - } - /** * Specify the cache period for the resources served by the resource handler, in seconds. The default is to not * send any cache headers but to rely on last-modified timestamps only. Set to 0 in order to send cache headers @@ -184,6 +89,47 @@ public class ResourceHandlerRegistration { return this; } + /** + * Configure a chain of resource resolvers and transformers to use. This + * can be useful for example to apply a version strategy to resource URLs. + * + *

If this method is not invoked, by default only a simple + * {@link PathResourceResolver} is used in order to match URL paths to + * resources under the configured locations. + * + * @param cacheResources whether to cache the result of resource resolution; + * setting this to "true" is recommended for production (and "false" for + * development, especially when applying a version strategy. + * @return the same {@link ResourceHandlerRegistration} instance for chained method invocation + * @since 4.1 + */ + public ResourceChainRegistration resourceChain(boolean cacheResources) { + this.resourceChainRegistration = new ResourceChainRegistration(cacheResources); + return this.resourceChainRegistration; + } + + /** + * Configure a chain of resource resolvers and transformers to use. This + * can be useful for example to apply a version strategy to resource URLs. + * + *

If this method is not invoked, by default only a simple + * {@link PathResourceResolver} is used in order to match URL paths to + * resources under the configured locations. + * + * @param cacheResources whether to cache the result of resource resolution; + * setting this to "true" is recommended for production (and "false" for + * development, especially when applying a version strategy. + * @param cache the cache to use for storing resolved and transformed resources; + * by default a {@link org.springframework.cache.concurrent.ConcurrentMapCache} + * is used. + * @return the same {@link ResourceHandlerRegistration} instance for chained method invocation + * @since 4.1 + */ + public ResourceChainRegistration resourceChain(boolean cacheResources, Cache cache) { + this.resourceChainRegistration = new ResourceChainRegistration(cacheResources, cache); + return this.resourceChainRegistration; + } + /** * Returns the URL path patterns for the resource handler. */ @@ -191,76 +137,21 @@ public class ResourceHandlerRegistration { return this.pathPatterns; } - protected List getResourceResolvers() { - if (this.customResolvers.isEmpty()) { - return null; - } - List resolvers = new ArrayList(); - ResourceResolver first = this.customResolvers.get(0); - if (!ClassUtils.isAssignable(CachingResourceResolver.class, first.getClass()) && !this.isDevMode) { - resolvers.add(new CachingResourceResolver(getDefaultResourceCache())); - } - resolvers.addAll(this.customResolvers); - ResourceResolver last = this.customResolvers.get(this.customResolvers.size() - 1); - if (!ClassUtils.isAssignable(PathResourceResolver.class, last.getClass())) { - resolvers.add(new PathResourceResolver()); - } - return resolvers; - } - - protected List getResourceTransformers() { - if (this.customTransformers.isEmpty()) { - return null; - } - List transformers = new ArrayList(); - ResourceTransformer first = this.customTransformers.get(0); - ResourceTransformer cachingTransformer = null; - if (!this.isDevMode) { - if (ClassUtils.isAssignable(CachingResourceTransformer.class, first.getClass())) { - cachingTransformer = first; - } - else { - cachingTransformer = new CachingResourceTransformer(getDefaultResourceCache()); - transformers.add(cachingTransformer); - } - } - transformers.addAll(this.customTransformers); - if (this.hasVersionResolver && !this.hasCssLinkTransformer) { - transformers.add(cachingTransformer != null ? 1 : 0, new CssLinkResourceTransformer()); - } - return transformers; - } - /** * Returns a {@link ResourceHttpRequestHandler} instance. */ protected ResourceHttpRequestHandler getRequestHandler() { Assert.isTrue(!CollectionUtils.isEmpty(locations), "At least one location is required for resource handling."); - ResourceHttpRequestHandler requestHandler = new ResourceHttpRequestHandler(); - List resourceResolvers = getResourceResolvers(); - if (!CollectionUtils.isEmpty(resourceResolvers)) { - requestHandler.setResourceResolvers(resourceResolvers); + ResourceHttpRequestHandler handler = new ResourceHttpRequestHandler(); + if (this.resourceChainRegistration != null) { + handler.setResourceResolvers(this.resourceChainRegistration.getResourceResolvers()); + handler.setResourceTransformers(this.resourceChainRegistration.getResourceTransformers()); } - List resourceTransformers = getResourceTransformers(); - if (!CollectionUtils.isEmpty(resourceTransformers)) { - requestHandler.setResourceTransformers(resourceTransformers); - } - requestHandler.setLocations(this.locations); + handler.setLocations(this.locations); if (this.cachePeriod != null) { - requestHandler.setCacheSeconds(this.cachePeriod); + handler.setCacheSeconds(this.cachePeriod); } - return requestHandler; - } - - /** - * Return a default instance of a {@code ConcurrentCacheMap} for - * caching resolved/transformed resources. - */ - private Cache getDefaultResourceCache() { - if(this.resourceCache == null) { - this.resourceCache = new ConcurrentMapCache(RESOURCE_CACHE_NAME); - } - return this.resourceCache; + return handler; } } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/ResourceHandlerRegistryTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/ResourceHandlerRegistryTests.java index 6b0be2bfd0..baee5ad8e0 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/ResourceHandlerRegistryTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/ResourceHandlerRegistryTests.java @@ -17,14 +17,12 @@ package org.springframework.web.servlet.config.annotation; import java.util.List; -import java.util.Map; import org.hamcrest.Matchers; import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; -import org.springframework.beans.DirectFieldAccessor; import org.springframework.cache.concurrent.ConcurrentMapCache; import org.springframework.mock.web.test.MockHttpServletRequest; import org.springframework.mock.web.test.MockHttpServletResponse; @@ -41,7 +39,6 @@ import org.springframework.web.servlet.resource.ResourceHttpRequestHandler; import org.springframework.web.servlet.resource.ResourceResolver; import org.springframework.web.servlet.resource.ResourceTransformer; import org.springframework.web.servlet.resource.VersionResourceResolver; -import org.springframework.web.servlet.resource.VersionStrategy; import static org.junit.Assert.*; @@ -108,10 +105,10 @@ public class ResourceHandlerRegistryTests { } @Test - public void simpleResourceChain() throws Exception { + public void resourceChain() throws Exception { ResourceResolver mockResolver = Mockito.mock(ResourceResolver.class); ResourceTransformer mockTransformer = Mockito.mock(ResourceTransformer.class); - this.registration.addResolver(mockResolver).addTransformer(mockTransformer); + this.registration.resourceChain(true).addResolver(mockResolver).addTransformer(mockTransformer); ResourceHttpRequestHandler handler = getHandler("/resources/**"); List resolvers = handler.getResourceResolvers(); @@ -129,8 +126,8 @@ public class ResourceHandlerRegistryTests { } @Test - public void noCacheResourceChain() throws Exception { - this.registration.enableDevMode(); + public void resourceChainWithoutCaching() throws Exception { + this.registration.resourceChain(false); ResourceHttpRequestHandler handler = getHandler("/resources/**"); List resolvers = handler.getResourceResolvers(); @@ -142,24 +139,19 @@ public class ResourceHandlerRegistryTests { } @Test - public void versionResourceChain() throws Exception { + public void resourceChainWithVersionResolver() throws Exception { VersionResourceResolver versionResolver = new VersionResourceResolver() .addFixedVersionStrategy("fixed", "/**/*.js") .addContentVersionStrategy("/**"); - this.registration.addResolver(versionResolver).addTransformer(new AppCacheManifestTransfomer()); + this.registration.resourceChain(true).addResolver(versionResolver) + .addTransformer(new AppCacheManifestTransfomer()); ResourceHttpRequestHandler handler = getHandler("/resources/**"); List resolvers = handler.getResourceResolvers(); assertThat(resolvers.toString(), resolvers, Matchers.hasSize(3)); assertThat(resolvers.get(0), Matchers.instanceOf(CachingResourceResolver.class)); - assertThat(resolvers.get(1), Matchers.instanceOf(VersionResourceResolver.class)); - DirectFieldAccessor fieldAccessor = new DirectFieldAccessor(resolvers.get(1)); - @SuppressWarnings("unchecked") - Map strategies = - (Map) fieldAccessor.getPropertyValue("versionStrategyMap"); - assertNotNull(strategies.get("/**/*.js")); - assertNotNull(strategies.get("/**")); + assertThat(resolvers.get(1), Matchers.sameInstance(versionResolver)); assertThat(resolvers.get(2), Matchers.instanceOf(PathResourceResolver.class)); List transformers = handler.getResourceTransformers(); @@ -170,33 +162,35 @@ public class ResourceHandlerRegistryTests { } @Test - public void customResourceChain() throws Exception { - VersionResourceResolver versionResolver = new VersionResourceResolver() - .addFixedVersionStrategy("fixed", "/**/*.js") - .addContentVersionStrategy("/**"); - + public void resourceChainWithOverrides() throws Exception { CachingResourceResolver cachingResolver = Mockito.mock(CachingResourceResolver.class); + VersionResourceResolver versionResolver = Mockito.mock(VersionResourceResolver.class); + PathResourceResolver pathResourceResolver = new PathResourceResolver(); CachingResourceTransformer cachingTransformer = Mockito.mock(CachingResourceTransformer.class); - this.registration - .addResolver(cachingResolver) - .addResolver(versionResolver) - .addResolver(new CustomPathResourceResolver()) - .addTransformer(cachingTransformer) - .addTransformer(new AppCacheManifestTransfomer()) - .setCachePeriod(3600); + AppCacheManifestTransfomer appCacheTransformer = Mockito.mock(AppCacheManifestTransfomer.class); + CssLinkResourceTransformer cssLinkTransformer = new CssLinkResourceTransformer(); + + this.registration.setCachePeriod(3600) + .resourceChain(false) + .addResolver(cachingResolver) + .addResolver(versionResolver) + .addResolver(pathResourceResolver) + .addTransformer(cachingTransformer) + .addTransformer(appCacheTransformer) + .addTransformer(cssLinkTransformer); ResourceHttpRequestHandler handler = getHandler("/resources/**"); List resolvers = handler.getResourceResolvers(); assertThat(resolvers.toString(), resolvers, Matchers.hasSize(3)); - assertThat(resolvers.get(0), Matchers.equalTo(cachingResolver)); - assertThat(resolvers.get(1), Matchers.instanceOf(VersionResourceResolver.class)); - assertThat(resolvers.get(2), Matchers.instanceOf(CustomPathResourceResolver.class)); + assertThat(resolvers.get(0), Matchers.sameInstance(cachingResolver)); + assertThat(resolvers.get(1), Matchers.sameInstance(versionResolver)); + assertThat(resolvers.get(2), Matchers.sameInstance(pathResourceResolver)); List transformers = handler.getResourceTransformers(); assertThat(transformers, Matchers.hasSize(3)); - assertThat(transformers.get(0), Matchers.equalTo(cachingTransformer)); - assertThat(transformers.get(1), Matchers.instanceOf(CssLinkResourceTransformer.class)); - assertThat(transformers.get(2), Matchers.instanceOf(AppCacheManifestTransfomer.class)); + assertThat(transformers.get(0), Matchers.sameInstance(cachingTransformer)); + assertThat(transformers.get(1), Matchers.sameInstance(appCacheTransformer)); + assertThat(transformers.get(2), Matchers.sameInstance(cssLinkTransformer)); } private ResourceHttpRequestHandler getHandler(String pathPattern) { @@ -204,9 +198,4 @@ public class ResourceHandlerRegistryTests { return (ResourceHttpRequestHandler) handlerMapping.getUrlMap().get(pathPattern); } - - private static class CustomPathResourceResolver extends PathResourceResolver { - - } - } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlProviderJavaConfigTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlProviderJavaConfigTests.java index bc132c30ab..7cfbcafa63 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlProviderJavaConfigTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlProviderJavaConfigTests.java @@ -113,7 +113,7 @@ public class ResourceUrlProviderJavaConfigTests { public void addResourceHandlers(ResourceHandlerRegistry registry) { registry.addResourceHandler("/resources/**") .addResourceLocations("classpath:org/springframework/web/servlet/resource/test/") - .addResolver(new VersionResourceResolver().addContentVersionStrategy("/**")); + .resourceChain(true).addResolver(new VersionResourceResolver().addContentVersionStrategy("/**")); } }