Polish + minor refactoring of ResourceUrlProvider

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
 	 */

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

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<ContextRefreshed
 	private static final Log logger = LogFactory.getLog(ResourceUrlProvider.class);
 
 
-	private final PathPatternParser patternParser;
+	private final PathPatternParser patternParser = new PathPatternParser();
 
 	private final Map<PathPattern, ResourceWebHandler> 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<PathPattern, ResourceWebHandler> getHandlerMap() {
 		return Collections.unmodifiableMap(this.handlerMap);
 	}
 
+
 	/**
-	 * Return {@code false} if resource mappings were manually configured,
-	 * {@code true} otherwise.
+	 * Manually configure resource handler mappings.
+	 * <p><strong>Note:</strong> 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<String, ResourceWebHandler> 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<ContextRefreshed
 		logger.debug("Looking for resource handler mappings");
 
 		Map<String, SimpleUrlHandlerMapping> map = context.getBeansOfType(SimpleUrlHandlerMapping.class);
-		List<SimpleUrlHandlerMapping> handlerMappings = new ArrayList<>(map.values());
-		AnnotationAwareOrderComparator.sort(handlerMappings);
+		List<SimpleUrlHandlerMapping> 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 ApplicationListener<ContextRefreshed
 		});
 	}
 
-	/**
-	 * Manually configure the resource mappings.
-	 * <p><strong>Note:</strong> 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<String, ResourceWebHandler> 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.
+	 * <p>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<String> getForRequestUrl(ServerWebExchange exchange, String requestUrl) {
+	public final Mono<String> 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.
-	 * <p>It is expected that the given path is what Spring uses for
-	 * request mapping purposes.
-	 * <p>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<String> getForLookupPath(PathContainer lookupPath) {
-		if (logger.isTraceEnabled()) {
-			logger.trace("Getting resource URL for lookup path \"" + lookupPath + "\"");
-		}
+	private Mono<String> resolveResourceUrl(PathContainer lookupPath) {
 		return this.handlerMap.entrySet().stream()
 				.filter(entry -> entry.getKey().matches(lookupPath))
 				.sorted(Comparator.comparing(Map.Entry::getKey))

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());
 	}