From 378c72e9b6e3f231eca7d3eb87e3e9358b89c816 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 19 Jul 2017 23:03:37 +0200 Subject: [PATCH] Polish + minor refactoring of ResourceUrlProvider --- .../server/reactive/ServerHttpResponse.java | 2 +- .../resource/ResourceTransformerSupport.java | 2 +- .../resource/ResourceUrlProvider.java | 135 ++++++------------ .../resource/ResourceUrlProviderTests.java | 24 ++-- 4 files changed, 60 insertions(+), 103 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpResponse.java b/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpResponse.java index 5060c136ec4..7eea41af1d4 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpResponse.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpResponse.java @@ -64,7 +64,7 @@ public interface ServerHttpResponse extends ReactiveHttpOutputMessage { * HTML template libraries to use consistently for all URLs emitted by * the application. Doing so enables the registration of URL encoders via * {@link #registerUrlEncoder} that can insert an id for authentication, - * a nonce for CSRF protection, a version for a static resource, etc. + * a nonce for CSRF protection, or other. * @param url the URL to encode * @return the encoded URL or the same */ diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceTransformerSupport.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceTransformerSupport.java index 7e5e46ce0cf..8beaf67c5aa 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceTransformerSupport.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceTransformerSupport.java @@ -76,7 +76,7 @@ public abstract class ResourceTransformerSupport implements ResourceTransformer if (resourcePath.startsWith("/")) { // full resource path ResourceUrlProvider urlProvider = getResourceUrlProvider(); - return (urlProvider != null ? urlProvider.getForRequestUrl(exchange, resourcePath) : Mono.empty()); + return (urlProvider != null ? urlProvider.getForUriString(resourcePath, exchange) : Mono.empty()); } else { // try resolving as relative path diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java index 883fbad9c06..4ba842b09f6 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java @@ -33,8 +33,6 @@ import org.springframework.context.event.ContextRefreshedEvent; import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.http.server.reactive.PathContainer; import org.springframework.http.server.reactive.ServerHttpRequest; -import org.springframework.lang.Nullable; -import org.springframework.util.Assert; import org.springframework.util.StringUtils; import org.springframework.web.reactive.handler.SimpleUrlHandlerMapping; import org.springframework.web.server.ServerWebExchange; @@ -57,50 +55,49 @@ public class ResourceUrlProvider implements ApplicationListener handlerMap = new LinkedHashMap<>(); - private boolean autodetect = true; - - - public ResourceUrlProvider() { - this(new PathPatternParser()); - } - - public ResourceUrlProvider(PathPatternParser patternParser) { - Assert.notNull(patternParser, "'patternParser' is required."); - this.patternParser = patternParser; - } - /** * Return a read-only view of the resource handler mappings either manually - * configured or auto-detected when the Spring {@code ApplicationContext} - * is refreshed. + * configured or auto-detected from Spring configuration. */ public Map getHandlerMap() { return Collections.unmodifiableMap(this.handlerMap); } + /** - * Return {@code false} if resource mappings were manually configured, - * {@code true} otherwise. + * Manually configure resource handler mappings. + *

Note: by default resource mappings are auto-detected + * from the Spring {@code ApplicationContext}. If this property is used, + * auto-detection is turned off. */ - public boolean isAutodetect() { - return this.autodetect; + public void registerHandlers(Map handlerMap) { + this.handlerMap.clear(); + handlerMap.forEach((rawPattern, resourceWebHandler) -> { + rawPattern = prependLeadingSlash(rawPattern); + PathPattern pattern = this.patternParser.parse(rawPattern); + this.handlerMap.put(pattern, resourceWebHandler); + }); } + private static String prependLeadingSlash(String pattern) { + if (StringUtils.hasLength(pattern) && !pattern.startsWith("/")) { + return "/" + pattern; + } + else { + return pattern; + } + } @Override public void onApplicationEvent(ContextRefreshedEvent event) { - if (isAutodetect()) { - this.handlerMap.clear(); + if (this.handlerMap.isEmpty()) { detectResourceHandlers(event.getApplicationContext()); - if (!this.handlerMap.isEmpty()) { - this.autodetect = false; - } - else if(logger.isDebugEnabled()) { + if(logger.isDebugEnabled()) { logger.debug("No resource handling mappings found"); } } @@ -110,10 +107,10 @@ public class ResourceUrlProvider implements ApplicationListener map = context.getBeansOfType(SimpleUrlHandlerMapping.class); - List handlerMappings = new ArrayList<>(map.values()); - AnnotationAwareOrderComparator.sort(handlerMappings); + List mappings = new ArrayList<>(map.values()); + AnnotationAwareOrderComparator.sort(mappings); - handlerMappings.forEach(mapping -> { + mappings.forEach(mapping -> { mapping.getHandlerMap().forEach((pattern, handler) -> { if (handler instanceof ResourceWebHandler) { ResourceWebHandler resourceHandler = (ResourceWebHandler) handler; @@ -128,85 +125,45 @@ public class ResourceUrlProvider implements ApplicationListenerNote: by default resource mappings are auto-detected - * from the Spring {@code ApplicationContext}. However if this property is - * used, the auto-detection is turned off. - */ - public void registerHandlers(@Nullable Map handlerMap) { - if (handlerMap == null) { - return; - } - this.handlerMap.clear(); - handlerMap.forEach((rawPattern, resourceWebHandler) -> { - rawPattern = prependLeadingSlash(rawPattern); - PathPattern pattern = this.patternParser.parse(rawPattern); - this.handlerMap.put(pattern, resourceWebHandler); - }); - this.autodetect = false; - } - - private static String prependLeadingSlash(String pattern) { - if (StringUtils.hasLength(pattern) && !pattern.startsWith("/")) { - return "/" + pattern; - } - else { - return pattern; - } - } - /** - * A variation on {@link #getForLookupPath(PathContainer)} that accepts a - * full request URL path and returns the full request URL path to expose - * for public use. + * Get the public resource URL for the given URI string. + *

The URI string is expected to be a path and if it contains a query or + * fragment those will be preserved in the resulting public resource URL. + * @param uriString the URI string to transform * @param exchange the current exchange - * @param requestUrl the request URL path to resolve - * @return the resolved public URL path, or empty if unresolved + * @return the resolved public resource URL path, or empty if unresolved */ - public final Mono getForRequestUrl(ServerWebExchange exchange, String requestUrl) { + public final Mono getForUriString(String uriString, ServerWebExchange exchange) { if (logger.isTraceEnabled()) { - logger.trace("Getting resource URL for request URL \"" + requestUrl + "\""); + logger.trace("Getting resource URL for request URL \"" + uriString + "\""); } ServerHttpRequest request = exchange.getRequest(); - int queryIndex = getQueryIndex(requestUrl); - String lookupPath = requestUrl.substring(0, queryIndex); - String query = requestUrl.substring(queryIndex); + int queryIndex = getQueryIndex(uriString); + String lookupPath = uriString.substring(0, queryIndex); + String query = uriString.substring(queryIndex); PathContainer parsedLookupPath = PathContainer.parseUrlPath(lookupPath); - return getForLookupPath(parsedLookupPath).map(resolvedPath -> + if (logger.isTraceEnabled()) { + logger.trace("Getting resource URL for lookup path \"" + lookupPath + "\""); + } + return resolveResourceUrl(parsedLookupPath).map(resolvedPath -> request.getPath().contextPath().value() + resolvedPath + query); } - private int getQueryIndex(String lookupPath) { - int suffixIndex = lookupPath.length(); - int queryIndex = lookupPath.indexOf("?"); + private int getQueryIndex(String path) { + int suffixIndex = path.length(); + int queryIndex = path.indexOf("?"); if (queryIndex > 0) { suffixIndex = queryIndex; } - int hashIndex = lookupPath.indexOf("#"); + int hashIndex = path.indexOf("#"); if (hashIndex > 0) { suffixIndex = Math.min(suffixIndex, hashIndex); } return suffixIndex; } - /** - * Compare the given path against configured resource handler mappings and - * if a match is found use the {@code ResourceResolver} chain of the matched - * {@code ResourceHttpRequestHandler} to resolve the URL path to expose for - * public use. - *

It is expected that the given path is what Spring uses for - * request mapping purposes. - *

If several handler mappings match, the handler used will be the one - * configured with the most specific pattern. - * @param lookupPath the lookup path to check - * @return the resolved public URL path, or empty if unresolved - */ - public final Mono getForLookupPath(PathContainer lookupPath) { - if (logger.isTraceEnabled()) { - logger.trace("Getting resource URL for lookup path \"" + lookupPath + "\""); - } + private Mono resolveResourceUrl(PathContainer lookupPath) { return this.handlerMap.entrySet().stream() .filter(entry -> entry.getKey().matches(lookupPath)) .sorted(Comparator.comparing(Map.Entry::getKey)) diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceUrlProviderTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceUrlProviderTests.java index 7211abcaa42..2c38db47e15 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceUrlProviderTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceUrlProviderTests.java @@ -32,7 +32,6 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.Resource; -import org.springframework.http.server.reactive.PathContainer; import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; import org.springframework.mock.web.test.MockServletContext; import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; @@ -41,7 +40,6 @@ import org.springframework.web.server.ServerWebExchange; import org.springframework.web.util.pattern.PathPattern; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; @@ -74,9 +72,10 @@ public class ResourceUrlProviderTests { @Test public void getStaticResourceUrl() { - PathContainer path = PathContainer.parsePath("/resources/foo.css"); - String url = this.urlProvider.getForLookupPath(path).block(Duration.ofSeconds(5)); - assertEquals("/resources/foo.css", url); + ServerWebExchange exchange = MockServerHttpRequest.get("/").toExchange(); + String uriString = "/resources/foo.css"; + String actual = this.urlProvider.getForUriString(uriString, exchange).block(Duration.ofSeconds(5)); + assertEquals(uriString, actual); } @Test // SPR-13374 @@ -84,11 +83,11 @@ public class ResourceUrlProviderTests { ServerWebExchange exchange = MockServerHttpRequest.get("/").toExchange(); String url = "/resources/foo.css?foo=bar&url=http://example.org"; - String resolvedUrl = this.urlProvider.getForRequestUrl(exchange, url).block(Duration.ofSeconds(5)); + String resolvedUrl = this.urlProvider.getForUriString(url, exchange).block(Duration.ofSeconds(5)); assertEquals(url, resolvedUrl); url = "/resources/foo.css#hash"; - resolvedUrl = this.urlProvider.getForRequestUrl(exchange, url).block(Duration.ofSeconds(5)); + resolvedUrl = this.urlProvider.getForUriString(url, exchange).block(Duration.ofSeconds(5)); assertEquals(url, resolvedUrl); } @@ -104,8 +103,9 @@ public class ResourceUrlProviderTests { resolvers.add(new PathResourceResolver()); this.handler.setResourceResolvers(resolvers); - PathContainer path = PathContainer.parsePath("/resources/foo.css"); - String url = this.urlProvider.getForLookupPath(path).block(Duration.ofSeconds(5)); + ServerWebExchange exchange = MockServerHttpRequest.get("/").toExchange(); + String path = "/resources/foo.css"; + String url = this.urlProvider.getForUriString(path, exchange).block(Duration.ofSeconds(5)); assertEquals("/resources/foo-e36d2e05253c6c7085a91522ce43a0b4.css", url); } @@ -126,8 +126,9 @@ public class ResourceUrlProviderTests { this.handlerMap.put("/resources/*.css", otherHandler); this.urlProvider.registerHandlers(this.handlerMap); - PathContainer path = PathContainer.parsePath("/resources/foo.css"); - String url = this.urlProvider.getForLookupPath(path).block(Duration.ofSeconds(5)); + ServerWebExchange exchange = MockServerHttpRequest.get("/").toExchange(); + String path = "/resources/foo.css"; + String url = this.urlProvider.getForUriString(path, exchange).block(Duration.ofSeconds(5)); assertEquals("/resources/foo-e36d2e05253c6c7085a91522ce43a0b4.css", url); } @@ -140,7 +141,6 @@ public class ResourceUrlProviderTests { ResourceUrlProvider urlProviderBean = context.getBean(ResourceUrlProvider.class); assertThat(urlProviderBean.getHandlerMap(), Matchers.hasKey(pattern("/resources/**"))); - assertFalse(urlProviderBean.isAutodetect()); }