Polish logging in resource handling

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

@@ -0,0 +1,65 @@
+/*
+ * 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.resource;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.springframework.core.io.Resource;
+
+import javax.servlet.http.HttpServletRequest;
+import java.util.List;
+
+/**
+ * Base class for {@link org.springframework.web.servlet.resource.ResourceResolver}
+ * implementations.
+ *
+ * @author Rossen Stoyanchev
+ * @since 4.1
+ */
+public abstract class AbstractResourceResolver implements ResourceResolver {
+
+	protected final Log logger = LogFactory.getLog(getClass());
+
+
+	@Override
+	public Resource resolveResource(HttpServletRequest request, String requestPath,
+			List<? extends Resource> locations, ResourceResolverChain chain) {
+
+		if (logger.isTraceEnabled()) {
+			logger.trace("Resolving resource: requestPath=\"" + requestPath + "\"");
+		}
+		return resolveResourceInternal(request, requestPath, locations, chain);
+	}
+
+	protected abstract Resource resolveResourceInternal(HttpServletRequest request, String requestPath,
+			List<? extends Resource> locations, ResourceResolverChain chain);
+
+	@Override
+	public String resolvePublicUrlPath(String resourceUrlPath, List<? extends Resource> locations,
+			ResourceResolverChain chain) {
+
+		if (logger.isTraceEnabled()) {
+			logger.trace("Resolving public URL for path=\"" + resourceUrlPath + "\"");
+		}
+
+		return resolvePublicUrlPathInternal(resourceUrlPath, locations, chain);
+	}
+
+	protected abstract String resolvePublicUrlPathInternal(String resourceUrlPath,
+			List<? extends Resource> locations, ResourceResolverChain chain);
+
+}

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

@@ -38,8 +38,6 @@ import org.springframework.util.Assert;
  */
 class DefaultResourceResolverChain implements ResourceResolverChain {
 
-	private static final Log logger = LogFactory.getLog(DefaultResourceResolverChain.class);
-
 	private final List<ResourceResolver> resolvers = new ArrayList<ResourceResolver>();
 
 	private int index = -1;
@@ -59,10 +57,7 @@ class DefaultResourceResolverChain implements ResourceResolverChain {
 			return null;
 		}
 		try {
-			logBefore(resolver);
-			Resource resource = resolver.resolveResource(request, requestPath, locations, this);
-			logAfter(resolver, resource);
-			return resource;
+			return resolver.resolveResource(request, requestPath, locations, this);
 		}
 		finally {
 			this.index--;
@@ -76,10 +71,7 @@ class DefaultResourceResolverChain implements ResourceResolverChain {
 			return null;
 		}
 		try {
-			logBefore(resolver);
-			String urlPath = resolver.resolvePublicUrlPath(resourcePath, locations, this);
-			logAfter(resolver, urlPath);
-			return urlPath;
+			return resolver.resolvePublicUrlPath(resourcePath, locations, this);
 		}
 		finally {
 			this.index--;
@@ -92,9 +84,6 @@ class DefaultResourceResolverChain implements ResourceResolverChain {
 				"Current index exceeds the number of configured ResourceResolver's");
 
 		if (this.index == (this.resolvers.size() - 1)) {
-			if (logger.isTraceEnabled()) {
-				logger.trace("No more ResourceResolver's to delegate to, returning null");
-			}
 			return null;
 		}
 
@@ -102,16 +91,4 @@ class DefaultResourceResolverChain implements ResourceResolverChain {
 		return this.resolvers.get(this.index);
 	}
 
-	private void logBefore(ResourceResolver resolver) {
-		if (logger.isTraceEnabled()) {
-			logger.trace("Calling " + resolver.getClass().getSimpleName() + " index=" + this.index);
-		}
-	}
-
-	private void logAfter(ResourceResolver resolver, Object result) {
-		if (logger.isTraceEnabled()) {
-			logger.trace(resolver.getClass().getSimpleName() + " returned " + result);
-		}
-	}
-
 }

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

@@ -50,16 +50,14 @@ import org.springframework.util.StringUtils;
  * @author Sam Brannen
  * @since 4.1
  */
-public class FingerprintResourceResolver implements ResourceResolver {
-
-	private static final Log logger = LogFactory.getLog(FingerprintResourceResolver.class);
+public class FingerprintResourceResolver extends AbstractResourceResolver {
 
 	private static final Pattern pattern = Pattern.compile("-(\\S*)\\.");
 
 
 	@Override
-	public Resource resolveResource(HttpServletRequest request, String requestPath, List<? extends Resource> locations,
-			ResourceResolverChain chain) {
+	protected Resource resolveResourceInternal(HttpServletRequest request, String requestPath,
+			List<? extends Resource> locations, ResourceResolverChain chain) {
 
 		Resource resolved = chain.resolveResource(request, requestPath, locations);
 		if (resolved != null) {
@@ -68,10 +66,18 @@ public class FingerprintResourceResolver implements ResourceResolver {
 
 		String hash = extractHash(requestPath);
 		if (StringUtils.isEmpty(hash)) {
+			if (logger.isTraceEnabled()) {
+				logger.trace("No hash found");
+			}
 			return null;
 		}
 
 		String simplePath = StringUtils.delete(requestPath, "-" + hash);
+
+		if (logger.isTraceEnabled()) {
+			logger.trace("Extracted hash from path, re-resolving without hash, path=\"" + simplePath + "\"");
+		}
+
 		Resource baseResource = chain.resolveResource(request, simplePath, locations);
 		if (baseResource == null) {
 			return null;
@@ -79,14 +85,39 @@ public class FingerprintResourceResolver implements ResourceResolver {
 
 		String candidateHash = calculateHash(baseResource);
 		if (candidateHash.equals(hash)) {
+			if (logger.isTraceEnabled()) {
+				logger.trace("Calculated hash matches extracted hash");
+			}
 			return baseResource;
 		}
 		else {
-			logger.debug("Potential resource found for [" + requestPath + "], but fingerprint doesn't match.");
+			logger.trace("Potential resource found for [" + requestPath + "], but fingerprint doesn't match.");
 			return null;
 		}
 	}
 
+	@Override
+	protected String resolvePublicUrlPathInternal(String resourceUrlPath, List<? extends Resource> locations,
+			ResourceResolverChain chain) {
+
+		String baseUrl = chain.resolvePublicUrlPath(resourceUrlPath, locations);
+		if (StringUtils.hasText(baseUrl)) {
+			if (logger.isTraceEnabled()) {
+				logger.trace("Getting the original resource to calculate hash");
+			}
+			Resource original = chain.resolveResource(null, resourceUrlPath, locations);
+			String hash = calculateHash(original);
+			if (logger.isTraceEnabled()) {
+				logger.trace("Calculated hash=" + hash);
+			}
+			String baseFilename = StringUtils.stripFilenameExtension(baseUrl);
+			String extension = StringUtils.getFilenameExtension(baseUrl);
+			return baseFilename + "-" + hash + "." + extension;
+		}
+		return baseUrl;
+	}
+
+
 	private String extractHash(String path) {
 		Matcher matcher = pattern.matcher(path);
 		if (matcher.find()) {
@@ -109,17 +140,4 @@ public class FingerprintResourceResolver implements ResourceResolver {
 		}
 	}
 
-	@Override
-	public String resolvePublicUrlPath(String resourceUrlPath, List<? extends Resource> locations,
-			ResourceResolverChain chain) {
-		String baseUrl = chain.resolvePublicUrlPath(resourceUrlPath, locations);
-		if (StringUtils.hasText(baseUrl)) {
-			Resource original = chain.resolveResource(null, resourceUrlPath, locations);
-			String hash = calculateHash(original);
-			return StringUtils.stripFilenameExtension(baseUrl) + "-" + hash + "."
-					+ StringUtils.getFilenameExtension(baseUrl);
-		}
-		return baseUrl;
-	}
-
 }

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

@@ -43,14 +43,12 @@ import org.springframework.core.io.Resource;
  * @author Sam Brannen
  * @since 4.1
  */
-public class GzipResourceResolver implements ResourceResolver {
-
-	private static final Log logger = LogFactory.getLog(GzipResourceResolver.class);
+public class GzipResourceResolver extends AbstractResourceResolver {
 
 
 	@Override
-	public Resource resolveResource(HttpServletRequest request, String requestPath, List<? extends Resource> locations,
-			ResourceResolverChain chain) {
+	protected Resource resolveResourceInternal(HttpServletRequest request, String requestPath,
+			List<? extends Resource> locations, ResourceResolverChain chain) {
 
 		Resource resource = chain.resolveResource(request, requestPath, locations);
 		if ((resource == null) || !isGzipAccepted(request)) {
@@ -76,8 +74,9 @@ public class GzipResourceResolver implements ResourceResolver {
 	}
 
 	@Override
-	public String resolvePublicUrlPath(String resourceUrlPath, List<? extends Resource> locations,
+	protected String resolvePublicUrlPathInternal(String resourceUrlPath, List<? extends Resource> locations,
 			ResourceResolverChain chain) {
+
 		return chain.resolvePublicUrlPath(resourceUrlPath, locations);
 	}
 

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

@@ -37,42 +37,42 @@ import org.springframework.core.io.Resource;
  * @author Sam Brannen
  * @since 4.1
  */
-public class PathResourceResolver implements ResourceResolver {
-
-	private static final Log logger = LogFactory.getLog(PathResourceResolver.class);
+public class PathResourceResolver extends AbstractResourceResolver {
 
 
 	@Override
-	public Resource resolveResource(HttpServletRequest request, String requestPath, List<? extends Resource> locations,
-			ResourceResolverChain chain) {
+	protected Resource resolveResourceInternal(HttpServletRequest request, String requestPath,
+			List<? extends Resource> locations, ResourceResolverChain chain) {
+
 		return getResource(requestPath, locations);
 	}
 
 	@Override
-	public String resolvePublicUrlPath(String resourceUrlPath, List<? extends Resource> locations,
+	protected String resolvePublicUrlPathInternal(String resourceUrlPath, List<? extends Resource> locations,
 			ResourceResolverChain chain) {
-		return (getResource(resourceUrlPath, locations) != null) ? resourceUrlPath : null;
+
+		return (getResource(resourceUrlPath, locations) != null ? resourceUrlPath : null);
 	}
 
 	private Resource getResource(String path, List<? extends Resource> locations) {
 		for (Resource location : locations) {
 			try {
-				if (logger.isDebugEnabled()) {
-					logger.debug("Looking for \"" + path + "\" under " + location);
+				if (logger.isTraceEnabled()) {
+					logger.trace("Checking location=[" + location + "]");
 				}
 				Resource resource = location.createRelative(path);
 				if (resource.exists() && resource.isReadable()) {
-					if (logger.isDebugEnabled()) {
-						logger.debug("Resource exists and is readable");
+					if (logger.isTraceEnabled()) {
+						logger.trace("Found match");
 					}
 					return resource;
 				}
 				else if (logger.isTraceEnabled()) {
-					logger.trace("Relative resource doesn't exist or isn't readable: " + resource);
+					logger.trace("No match");
 				}
 			}
 			catch (IOException ex) {
-				logger.debug("Failed to create relative resource - trying next resource location", ex);
+				logger.trace("Failure checking for relative resource. Trying next location.", ex);
 			}
 		}
 		return null;

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

@@ -16,6 +16,8 @@
 
 package org.springframework.web.servlet.resource;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.springframework.core.io.Resource;
 import org.springframework.util.Assert;
 
@@ -39,7 +41,9 @@ import java.util.List;
  * @author Sam Brannen
  * @since 4.1
  */
-public class PrefixResourceResolver implements ResourceResolver {
+public class PrefixResourceResolver extends AbstractResourceResolver {
+
+	private static final Log logger = LogFactory.getLog(PathResourceResolver.class);
 
 	private final String prefix;
 
@@ -50,9 +54,12 @@ public class PrefixResourceResolver implements ResourceResolver {
 	}
 
 	@Override
-	public Resource resolveResource(HttpServletRequest request, String requestPath, List<? extends Resource> locations,
-			ResourceResolverChain chain) {
+	protected Resource resolveResourceInternal(HttpServletRequest request, String requestPath,
+			List<? extends Resource> locations, ResourceResolverChain chain) {
 
+		if (logger.isTraceEnabled()) {
+			logger.trace("Resolving resource: requestPath=\"" + requestPath + "\"");
+		}
 		if (requestPath.startsWith(this.prefix)) {
 			requestPath = requestPath.substring(this.prefix.length());
 		}
@@ -61,8 +68,9 @@ public class PrefixResourceResolver implements ResourceResolver {
 	}
 
 	@Override
-	public String resolvePublicUrlPath(String resourceUrlPath, List<? extends Resource> locations,
+	protected String resolvePublicUrlPathInternal(String resourceUrlPath, List<? extends Resource> locations,
 			ResourceResolverChain chain) {
+
 		String baseUrl = chain.resolvePublicUrlPath(resourceUrlPath, locations);
 		return this.prefix + (baseUrl.startsWith("/") ? baseUrl : "/" + baseUrl);
 	}

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

@@ -139,9 +139,9 @@ public class PublicResourceUrlProvider implements ApplicationListener<ContextRef
 				if (handler instanceof ResourceHttpRequestHandler) {
 					ResourceHttpRequestHandler resourceHandler = (ResourceHttpRequestHandler) handler;
 					if (logger.isDebugEnabled()) {
-						logger.debug("Found pattern=\"" + pattern + "\" mapped to locations " +
-								resourceHandler.getLocations() + " with resolvers: " +
-								resourceHandler.getResourceResolvers());
+						logger.debug("Found resource handler mapping: URL pattern=\"" + pattern + "\", " +
+								"locations=" + resourceHandler.getLocations() + ", " +
+								"resolvers=" + resourceHandler.getResourceResolvers());
 					}
 					this.handlerMap.put(pattern, resourceHandler);
 				}
@@ -160,13 +160,13 @@ public class PublicResourceUrlProvider implements ApplicationListener<ContextRef
 	 * @return the resolved public URL path or {@code null} if unresolved
 	 */
 	public final String getForRequestUrl(HttpServletRequest request, String requestUrl) {
-		if (logger.isDebugEnabled()) {
-			logger.debug("Checking requestURL=" + requestUrl);
+		if (logger.isTraceEnabled()) {
+			logger.trace("Getting resource URL for requestURL=" + requestUrl);
 		}
 
 		String pathWithinMapping = (String) request.getAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE);
 		if (pathWithinMapping == null) {
-			logger.debug("Request attribute with lookup path not found, calculating instead.");
+			logger.trace("Request attribute with lookup path not found, calculating instead.");
 			pathWithinMapping = getPathHelper().getLookupPathForRequest(request);
 		}
 
@@ -193,8 +193,8 @@ public class PublicResourceUrlProvider implements ApplicationListener<ContextRef
 	 * @return the resolved public URL path or {@code null} if unresolved
 	 */
 	public final String getForLookupPath(String lookupPath) {
-		if (logger.isDebugEnabled()) {
-			logger.debug("Checking lookup path: " + lookupPath);
+		if (logger.isTraceEnabled()) {
+			logger.trace("Getting resource URL for lookupPath=" + lookupPath);
 		}
 		for (String pattern : this.handlerMap.keySet()) {
 			if (!getPathMatcher().match(pattern, lookupPath)) {
@@ -202,9 +202,8 @@ public class PublicResourceUrlProvider implements ApplicationListener<ContextRef
 			}
 			String pathWithinMapping = getPathMatcher().extractPathWithinPattern(pattern, lookupPath);
 			String pathMapping = lookupPath.substring(0, lookupPath.indexOf(pathWithinMapping));
-			if (logger.isDebugEnabled()) {
-				logger.debug("Found matching resource mapping=\"" + pattern + "\", " +
-						"resource URL path=\"" + pathWithinMapping + "\"");
+			if (logger.isTraceEnabled()) {
+				logger.trace("Invoking ResourceResolverChain for URL pattern=\"" + pattern + "\"");
 			}
 			ResourceHttpRequestHandler handler = this.handlerMap.get(pattern);
 			ResourceResolverChain chain = handler.createResourceResolverChain();
@@ -212,8 +211,8 @@ public class PublicResourceUrlProvider implements ApplicationListener<ContextRef
 			if (resolved == null) {
 				throw new IllegalStateException("Failed to get public resource URL path for " + pathWithinMapping);
 			}
-			if (logger.isDebugEnabled()) {
-				logger.debug("Returning public resource URL path=\"" + resolved + "\"");
+			if (logger.isTraceEnabled()) {
+				logger.trace("Resolved public resource URL path=\"" + resolved + "\"");
 			}
 			return pathMapping + resolved;
 		}

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

@@ -156,7 +156,7 @@ public class ResourceHttpRequestHandler extends WebContentGenerator implements H
 		// check whether a matching resource exists
 		Resource resource = getResource(request);
 		if (resource == null) {
-			logger.debug("No matching resource found - returning 404");
+			logger.trace("No matching resource found - returning 404");
 			response.sendError(HttpServletResponse.SC_NOT_FOUND);
 			return;
 		}
@@ -164,19 +164,19 @@ public class ResourceHttpRequestHandler extends WebContentGenerator implements H
 		// check the resource's media type
 		MediaType mediaType = getMediaType(resource);
 		if (mediaType != null) {
-			if (logger.isDebugEnabled()) {
-				logger.debug("Determined media type '" + mediaType + "' for " + resource);
+			if (logger.isTraceEnabled()) {
+				logger.trace("Determined media type '" + mediaType + "' for " + resource);
 			}
 		}
 		else {
-			if (logger.isDebugEnabled()) {
-				logger.debug("No media type found for " + resource + " - not sending a content-type header");
+			if (logger.isTraceEnabled()) {
+				logger.trace("No media type found for " + resource + " - not sending a content-type header");
 			}
 		}
 
 		// header phase
 		if (new ServletWebRequest(request, response).checkNotModified(resource.lastModified())) {
-			logger.debug("Resource not modified - returning 304");
+			logger.trace("Resource not modified - returning 304");
 			return;
 		}
 		setHeaders(response, resource, mediaType);
@@ -196,8 +196,8 @@ public class ResourceHttpRequestHandler extends WebContentGenerator implements H
 					HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE + "' is not set");
 		}
 		if (!StringUtils.hasText(path) || isInvalidPath(path)) {
-			if (logger.isDebugEnabled()) {
-				logger.debug("Ignoring invalid resource path [" + path + "]");
+			if (logger.isTraceEnabled()) {
+				logger.trace("Ignoring invalid resource path [" + path + "]");
 			}
 			return null;
 		}
@@ -284,6 +284,11 @@ public class ResourceHttpRequestHandler extends WebContentGenerator implements H
 		}
 	}
 
+	@Override
+	public String toString() {
+		return "ResourceHttpRequestHandler [locations=" +
+				getLocations() + ", resolvers=" + getResourceResolvers() + "]";
+	}
 
 	/**
 	 * Inner class to avoid hard-coded JAF dependency.