Preserve ETag HTTP header for versioned resources

Prior to this change, a resource handler chain configured with a
`VersionResourceResolver` would add the resource version to the request
attributes when serving that resource. This approach would not work when
a `CachingResourceResolver` is configured and the resource is already
cached. Indeed, that code path is not executed when the resource is
resolved from the cache.

This commit adds a new `VersionedResource` interface that's used by the
`VersionResourceResolver`, adding a `getVersion()` method that returns
the version string for that resource. This way, the version information
is cached with the resource itself and the request attributes are no
longer used for this.

Issue: SPR-13817

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -266,7 +266,6 @@ public class ResourceHttpRequestHandler extends WebContentGenerator
 		}
 
 		if (request.getHeader(HttpHeaders.RANGE) == null) {
-			setETagHeader(request, response);
 			setHeaders(response, resource, mediaType);
 			writeContent(response, resource);
 		}
@@ -406,21 +405,6 @@ public class ResourceHttpRequestHandler extends WebContentGenerator
 		return mediaType;
 	}
 
-	/**
-	 * Set the ETag header if the version string of the served resource is present.
-	 * Version strings can be resolved by {@link VersionStrategy} implementations and then
-	 * set as a request attribute by {@link VersionResourceResolver}.
-	 * @param request current servlet request
-	 * @param response current servlet response
-	 * @see VersionResourceResolver
-	 */
-	protected void setETagHeader(HttpServletRequest request, HttpServletResponse response) {
-		String versionString = (String) request.getAttribute(VersionResourceResolver.RESOURCE_VERSION_ATTRIBUTE);
-		if (versionString != null) {
-			response.setHeader(HttpHeaders.ETAG, "\"" + versionString + "\"");
-		}
-	}
-
 	/**
 	 * Set headers on the given servlet response.
 	 * Called for GET requests as well as HEAD requests.
@@ -435,15 +419,15 @@ public class ResourceHttpRequestHandler extends WebContentGenerator
 			throw new IOException("Resource content too long (beyond Integer.MAX_VALUE): " + resource);
 		}
 		response.setContentLength((int) length);
-
 		if (mediaType != null) {
 			response.setContentType(mediaType.toString());
 		}
-
 		if (resource instanceof EncodedResource) {
 			response.setHeader(HttpHeaders.CONTENT_ENCODING, ((EncodedResource) resource).getContentEncoding());
 		}
-
+		if (resource instanceof VersionedResource) {
+			response.setHeader(HttpHeaders.ETAG, "\"" + ((VersionedResource) resource).getVersion() + "\"");
+		}
 		response.setHeader(HttpHeaders.ACCEPT_RANGES, "bytes");
 	}
 

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -16,14 +16,21 @@
 
 package org.springframework.web.servlet.resource;
 
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URI;
+import java.net.URL;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+
 import javax.servlet.http.HttpServletRequest;
 
+import org.springframework.core.io.AbstractResource;
 import org.springframework.core.io.Resource;
 import org.springframework.util.AntPathMatcher;
 import org.springframework.util.StringUtils;
@@ -55,10 +62,6 @@ import org.springframework.util.StringUtils;
  */
 public class VersionResourceResolver extends AbstractResourceResolver {
 
-	public static final String RESOURCE_VERSION_ATTRIBUTE =
-			VersionResourceResolver.class.getName() + ".resourceVersion";
-
-
 	private AntPathMatcher pathMatcher = new AntPathMatcher();
 
 	/** Map from path pattern -> VersionStrategy */
@@ -168,12 +171,9 @@ public class VersionResourceResolver extends AbstractResourceResolver {
 		String actualVersion = versionStrategy.getResourceVersion(baseResource);
 		if (candidateVersion.equals(actualVersion)) {
 			if (logger.isTraceEnabled()) {
-				logger.trace("Resource matches extracted version ["+ candidateVersion + "]");
+				logger.trace("Resource matches extracted version [" + candidateVersion + "]");
 			}
-			if (request != null) {
-				request.setAttribute(RESOURCE_VERSION_ATTRIBUTE, candidateVersion);
-			}
-			return baseResource;
+			return new FileNameVersionedResource(baseResource, candidateVersion);
 		}
 		else {
 			if (logger.isTraceEnabled()) {
@@ -225,4 +225,82 @@ public class VersionResourceResolver extends AbstractResourceResolver {
 		return null;
 	}
 
+	private class FileNameVersionedResource extends AbstractResource implements VersionedResource {
+
+		private final Resource original;
+
+		private final String version;
+
+		public FileNameVersionedResource(Resource original, String version) {
+			this.original = original;
+			this.version = version;
+		}
+
+		@Override
+		public boolean exists() {
+			return this.original.exists();
+		}
+
+		@Override
+		public boolean isReadable() {
+			return this.original.isReadable();
+		}
+
+		@Override
+		public boolean isOpen() {
+			return this.original.isOpen();
+		}
+
+		@Override
+		public URL getURL() throws IOException {
+			return this.original.getURL();
+		}
+
+		@Override
+		public URI getURI() throws IOException {
+			return this.original.getURI();
+		}
+
+		@Override
+		public File getFile() throws IOException {
+			return this.original.getFile();
+		}
+
+		@Override
+		public String getFilename() {
+			return this.original.getFilename();
+		}
+
+		@Override
+		public long contentLength() throws IOException {
+			return this.original.contentLength();
+		}
+
+		@Override
+		public long lastModified() throws IOException {
+			return this.original.lastModified();
+		}
+
+		@Override
+		public Resource createRelative(String relativePath) throws IOException {
+			return this.original.createRelative(relativePath);
+		}
+
+		@Override
+		public String getDescription() {
+			return original.getDescription();
+		}
+
+		@Override
+		public InputStream getInputStream() throws IOException {
+			return original.getInputStream();
+		}
+
+		@Override
+		public String getVersion() {
+			return this.version;
+		}
+
+	}
+
 }

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

@@ -0,0 +1,32 @@
+/*
+ * Copyright 2002-2016 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.resource;
+
+import org.springframework.core.io.Resource;
+
+/**
+ * Interface for a resource descriptor that describes its version
+ * with a version string that can be derived from its content and/or metadata.
+ *
+ * @author Brian Clozel
+ * @since 4.2
+ * @see VersionResourceResolver
+ */
+public interface VersionedResource extends Resource {
+
+	String getVersion();
+}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -87,7 +87,6 @@ public class ResourceHttpRequestHandlerTests {
 	@Test
 	public void getResource() throws Exception {
 		this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "foo.css");
-		this.request.setAttribute(VersionResourceResolver.RESOURCE_VERSION_ATTRIBUTE, "versionString");
 		this.handler.handleRequest(this.request, this.response);
 
 		assertEquals("text/css", this.response.getContentType());
@@ -95,7 +94,6 @@ public class ResourceHttpRequestHandlerTests {
 		assertEquals("max-age=3600", this.response.getHeader("Cache-Control"));
 		assertTrue(this.response.containsHeader("Last-Modified"));
 		assertEquals(this.response.getHeader("Last-Modified"), resourceLastModifiedDate("test/foo.css"));
-		assertEquals("\"versionString\"", this.response.getHeader("ETag"));
 		assertEquals("h1 { color:red; }", this.response.getContentAsString());
 	}
 
@@ -110,6 +108,19 @@ public class ResourceHttpRequestHandlerTests {
 		assertEquals(this.response.getHeader("Last-Modified"), resourceLastModifiedDate("test/foo.css"));
 	}
 
+	@Test
+	public void getVersionedResource() throws Exception {
+		VersionResourceResolver versionResolver = new VersionResourceResolver()
+				.addFixedVersionStrategy("versionString", "/**");
+		this.handler.setResourceResolvers(Arrays.asList(versionResolver, new PathResourceResolver()));
+		this.handler.afterPropertiesSet();
+
+		this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "versionString/foo.css");
+		this.handler.handleRequest(this.request, this.response);
+
+		assertEquals("\"versionString\"", this.response.getHeader("ETag"));
+	}
+
 	@Test
 	@SuppressWarnings("deprecation")
 	public void getResourceHttp10BehaviorCache() throws Exception {

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 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,7 @@ import org.springframework.core.io.ClassPathResource;
 import org.springframework.core.io.Resource;
 import org.springframework.mock.web.test.MockHttpServletRequest;
 
+import static org.hamcrest.Matchers.instanceOf;
 import static org.junit.Assert.*;
 import static org.mockito.BDDMockito.*;
 
@@ -146,9 +147,10 @@ public class VersionResourceResolverTests {
 		this.resolver
 				.setStrategyMap(Collections.singletonMap("/**", this.versionStrategy));
 		Resource actual = this.resolver.resolveResourceInternal(request, versionFile, this.locations, this.chain);
-		assertEquals(expected, actual);
+		assertEquals(expected.getFilename(), actual.getFilename());
 		verify(this.versionStrategy, times(1)).getResourceVersion(expected);
-		assertEquals(version, request.getAttribute(VersionResourceResolver.RESOURCE_VERSION_ATTRIBUTE));
+		assertThat(actual, instanceOf(VersionedResource.class));
+		assertEquals(version, ((VersionedResource)actual).getVersion());
 	}
 
 	@Test