Browse Source

Fix CachingResourceResolver key generation

When used in combination with GzipResourceResolver, the
CachingResourceResolver does not properly cache results, since it only
takes the request path as a input for cache key generation.

Here's an example of that behavior:

1. an HTTP client requests a resource with `Accept-Encoding: gzip`, so
the GzipResourceResolver can resolve a gzipped resource.
2. the configured CachingResourceResolver caches that resource.
3. another HTTP client requests the same resource, but it does not
support gzip encoding; the previously cached gzipped resource is still
returned.

This commit uses the presence/absence of gzip encoding support as an
input in cache keys generation.

Issue: SPR-12982
pull/1271/head
Brian Clozel 11 years ago
parent
commit
bb3f26483b
  1. 17
      spring-webmvc/src/main/java/org/springframework/web/servlet/resource/CachingResourceResolver.java
  2. 41
      spring-webmvc/src/test/java/org/springframework/web/servlet/resource/CachingResourceResolverTests.java
  3. 38
      spring-webmvc/src/test/java/org/springframework/web/servlet/resource/GzipResourceResolverTests.java

17
spring-webmvc/src/main/java/org/springframework/web/servlet/resource/CachingResourceResolver.java

@ -1,5 +1,5 @@ @@ -1,5 +1,5 @@
/*
* Copyright 2002-2014 the original author or authors.
* Copyright 2002-2015 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,6 +30,7 @@ import org.springframework.util.Assert; @@ -30,6 +30,7 @@ import org.springframework.util.Assert;
* delegates to the resolver chain and saves the result in the cache.
*
* @author Rossen Stoyanchev
* @author Brian Clozel
* @since 4.1
*/
public class CachingResourceResolver extends AbstractResourceResolver {
@ -61,7 +62,7 @@ public class CachingResourceResolver extends AbstractResourceResolver { @@ -61,7 +62,7 @@ public class CachingResourceResolver extends AbstractResourceResolver {
protected Resource resolveResourceInternal(HttpServletRequest request, String requestPath,
List<? extends Resource> locations, ResourceResolverChain chain) {
String key = RESOLVED_RESOURCE_CACHE_KEY_PREFIX + requestPath;
String key = computeKey(request, requestPath);
Resource resource = this.cache.get(key, Resource.class);
if (resource != null) {
@ -82,6 +83,18 @@ public class CachingResourceResolver extends AbstractResourceResolver { @@ -82,6 +83,18 @@ public class CachingResourceResolver extends AbstractResourceResolver {
return resource;
}
protected String computeKey(HttpServletRequest request, String requestPath) {
StringBuilder key = new StringBuilder(RESOLVED_RESOURCE_CACHE_KEY_PREFIX);
key.append(requestPath);
if(request != null) {
String encoding = request.getHeader("Accept-Encoding");
if(encoding != null && encoding.contains("gzip")) {
key.append("+encoding=gzip");
}
}
return key.toString();
}
@Override
protected String resolveUrlPathInternal(String resourceUrlPath,
List<? extends Resource> locations, ResourceResolverChain chain) {

41
spring-webmvc/src/test/java/org/springframework/web/servlet/resource/CachingResourceResolverTests.java

@ -1,5 +1,5 @@ @@ -1,5 +1,5 @@
/*
* Copyright 2002-2014 the original author or authors.
* Copyright 2002-2015 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.
@ -27,6 +27,7 @@ import org.springframework.cache.Cache; @@ -27,6 +27,7 @@ import org.springframework.cache.Cache;
import org.springframework.cache.concurrent.ConcurrentMapCache;
import org.springframework.core.io.ClassPathResource;
import org.springframework.core.io.Resource;
import org.springframework.mock.web.test.MockHttpServletRequest;
import static org.junit.Assert.*;
@ -108,4 +109,42 @@ public class CachingResourceResolverTests { @@ -108,4 +109,42 @@ public class CachingResourceResolverTests {
assertNull(this.chain.resolveUrlPath("invalid.css", this.locations));
}
@Test
public void resolveResourceAcceptEncodingInCacheKey() {
String file = "bar.css";
MockHttpServletRequest request = new MockHttpServletRequest("GET", file);
request.addHeader("Accept-Encoding", "gzip");
Resource expected = this.chain.resolveResource(request, file, this.locations);
String cacheKey = CachingResourceResolver.RESOLVED_RESOURCE_CACHE_KEY_PREFIX + file + "+encoding=gzip";
assertEquals(expected, this.cache.get(cacheKey).get());
}
@Test
public void resolveResourceNoAcceptEncodingInCacheKey() {
String file = "bar.css";
MockHttpServletRequest request = new MockHttpServletRequest("GET", file);
Resource expected = this.chain.resolveResource(request, file, this.locations);
String cacheKey = CachingResourceResolver.RESOLVED_RESOURCE_CACHE_KEY_PREFIX + file;
assertEquals(expected, this.cache.get(cacheKey).get());
}
@Test
public void resolveResourceMatchingEncoding() {
Resource resource = Mockito.mock(Resource.class);
Resource gzResource = Mockito.mock(Resource.class);
this.cache.put(CachingResourceResolver.RESOLVED_RESOURCE_CACHE_KEY_PREFIX + "bar.css", resource);
this.cache.put(CachingResourceResolver.RESOLVED_RESOURCE_CACHE_KEY_PREFIX + "bar.css+encoding=gzip", gzResource);
MockHttpServletRequest request = new MockHttpServletRequest("GET", "bar.css");
assertSame(resource, this.chain.resolveResource(request,"bar.css", this.locations));
request = new MockHttpServletRequest("GET", "bar.css");
request.addHeader("Accept-Encoding", "gzip");
assertSame(gzResource, this.chain.resolveResource(request, "bar.css", this.locations));
}
}

38
spring-webmvc/src/test/java/org/springframework/web/servlet/resource/GzipResourceResolverTests.java

@ -1,5 +1,5 @@ @@ -1,5 +1,5 @@
/*
* Copyright 2002-2013 the original author or authors.
* Copyright 2002-2015 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.
@ -28,6 +28,8 @@ import org.junit.Before; @@ -28,6 +28,8 @@ import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
import org.springframework.cache.Cache;
import org.springframework.cache.concurrent.ConcurrentMapCache;
import org.springframework.core.io.ClassPathResource;
import org.springframework.core.io.FileSystemResource;
import org.springframework.core.io.Resource;
@ -38,6 +40,8 @@ import static org.junit.Assert.*; @@ -38,6 +40,8 @@ import static org.junit.Assert.*;
/**
* Unit tests for
* {@link org.springframework.web.servlet.resource.GzipResourceResolver}.
*
* @author Jeremy Grelle
*/
@ -47,6 +51,8 @@ public class GzipResourceResolverTests { @@ -47,6 +51,8 @@ public class GzipResourceResolverTests {
private List<Resource> locations;
private Cache cache;
@BeforeClass
public static void createGzippedResources() throws IOException {
Resource location = new ClassPathResource("test/", GzipResourceResolverTests.class);
@ -71,12 +77,15 @@ public class GzipResourceResolverTests { @@ -71,12 +77,15 @@ public class GzipResourceResolverTests {
@Before
public void setUp() {
this.cache = new ConcurrentMapCache("resourceCache");
Map<String, VersionStrategy> versionStrategyMap = new HashMap<>();
versionStrategyMap.put("/**", new ContentVersionStrategy());
VersionResourceResolver versionResolver = new VersionResourceResolver();
versionResolver.setStrategyMap(versionStrategyMap);
List<ResourceResolver> resolvers = new ArrayList<ResourceResolver>();
resolvers.add(new CachingResourceResolver(this.cache));
resolvers.add(new GzipResourceResolver());
resolvers.add(versionResolver);
resolvers.add(new PathResourceResolver());
@ -96,7 +105,7 @@ public class GzipResourceResolverTests { @@ -96,7 +105,7 @@ public class GzipResourceResolverTests {
Resource resolved = resolver.resolveResource(request, file, locations);
assertEquals(resource.getDescription(), resolved.getDescription());
assertEquals(new ClassPathResource("test/"+file).getFilename(), resolved.getFilename());
assertEquals(new ClassPathResource("test/" + file).getFilename(), resolved.getFilename());
assertTrue("Expected " + resolved + " to be of type " + EncodedResource.class,
resolved instanceof EncodedResource);
}
@ -115,4 +124,29 @@ public class GzipResourceResolverTests { @@ -115,4 +124,29 @@ public class GzipResourceResolverTests {
assertTrue("Expected " + resolved + " to be of type " + EncodedResource.class,
resolved instanceof EncodedResource);
}
@Test
public void resolveFromCacheWithEncodingVariants() throws IOException {
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/js/foo.js");
request.addHeader("Accept-Encoding", "gzip");
String file = "js/foo.js";
String gzFile = file+".gz";
Resource resource = new ClassPathResource("test/"+file, getClass());
Resource gzResource = new ClassPathResource("test/"+gzFile, getClass());
// resolved resource is now cached in CachingResourceResolver
Resource resolved = resolver.resolveResource(request, file, locations);
assertEquals(gzResource.getDescription(), resolved.getDescription());
assertEquals(new ClassPathResource("test/" + file).getFilename(), resolved.getFilename());
assertTrue("Expected " + resolved + " to be of type " + EncodedResource.class,
resolved instanceof EncodedResource);
request = new MockHttpServletRequest("GET", "/js/foo.js");
resolved = resolver.resolveResource(request, file, locations);
assertEquals(resource.getDescription(), resolved.getDescription());
assertEquals(new ClassPathResource("test/" + file).getFilename(), resolved.getFilename());
assertFalse("Expected " + resolved + " to *not* be of type " + EncodedResource.class,
resolved instanceof EncodedResource);
}
}

Loading…
Cancel
Save