From b1158aa913900b03155712536dee85802624e99f Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 23 Apr 2014 15:42:50 -0400 Subject: [PATCH] Polish logging in resource handling --- .../resource/AbstractResourceResolver.java | 65 +++++++++++++++++++ .../DefaultResourceResolverChain.java | 27 +------- .../resource/FingerprintResourceResolver.java | 56 ++++++++++------ .../resource/GzipResourceResolver.java | 11 ++-- .../resource/PathResourceResolver.java | 26 ++++---- .../resource/PrefixResourceResolver.java | 16 +++-- .../resource/PublicResourceUrlProvider.java | 25 ++++--- .../resource/ResourceHttpRequestHandler.java | 21 +++--- 8 files changed, 159 insertions(+), 88 deletions(-) create mode 100644 spring-webmvc/src/main/java/org/springframework/web/servlet/resource/AbstractResourceResolver.java diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/AbstractResourceResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/AbstractResourceResolver.java new file mode 100644 index 0000000000..a3a50721a7 --- /dev/null +++ b/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 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 locations, ResourceResolverChain chain); + + @Override + public String resolvePublicUrlPath(String resourceUrlPath, List 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 locations, ResourceResolverChain chain); + +} diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/DefaultResourceResolverChain.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/DefaultResourceResolverChain.java index 0d2f71cb04..4f52610398 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/DefaultResourceResolverChain.java +++ b/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 resolvers = new ArrayList(); 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); - } - } - } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/FingerprintResourceResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/FingerprintResourceResolver.java index 98f301957c..c6d444f45e 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/FingerprintResourceResolver.java +++ b/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 locations, - ResourceResolverChain chain) { + protected Resource resolveResourceInternal(HttpServletRequest request, String requestPath, + List 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 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 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; - } - } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/GzipResourceResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/GzipResourceResolver.java index 150a9e1106..57f0c80991 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/GzipResourceResolver.java +++ b/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 locations, - ResourceResolverChain chain) { + protected Resource resolveResourceInternal(HttpServletRequest request, String requestPath, + List 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 locations, + protected String resolvePublicUrlPathInternal(String resourceUrlPath, List locations, ResourceResolverChain chain) { + return chain.resolvePublicUrlPath(resourceUrlPath, locations); } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PathResourceResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PathResourceResolver.java index 5bdbd35f85..347e2c3e84 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PathResourceResolver.java +++ b/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 locations, - ResourceResolverChain chain) { + protected Resource resolveResourceInternal(HttpServletRequest request, String requestPath, + List locations, ResourceResolverChain chain) { + return getResource(requestPath, locations); } @Override - public String resolvePublicUrlPath(String resourceUrlPath, List locations, + protected String resolvePublicUrlPathInternal(String resourceUrlPath, List locations, ResourceResolverChain chain) { - return (getResource(resourceUrlPath, locations) != null) ? resourceUrlPath : null; + + return (getResource(resourceUrlPath, locations) != null ? resourceUrlPath : null); } private Resource getResource(String path, List 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; diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PrefixResourceResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PrefixResourceResolver.java index 6f964ae782..208a5b587b 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PrefixResourceResolver.java +++ b/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 locations, - ResourceResolverChain chain) { + protected Resource resolveResourceInternal(HttpServletRequest request, String requestPath, + List 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 locations, + protected String resolvePublicUrlPathInternal(String resourceUrlPath, List locations, ResourceResolverChain chain) { + String baseUrl = chain.resolvePublicUrlPath(resourceUrlPath, locations); return this.prefix + (baseUrl.startsWith("/") ? baseUrl : "/" + baseUrl); } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PublicResourceUrlProvider.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PublicResourceUrlProvider.java index e3928292b5..65aca87afe 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PublicResourceUrlProvider.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PublicResourceUrlProvider.java @@ -139,9 +139,9 @@ public class PublicResourceUrlProvider implements ApplicationListener