From 9640cedeaed69902d1351cee27fd3426cdc21c40 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 7 Jul 2017 19:11:13 +0200 Subject: [PATCH] Removing remaining use of PathPattern with String path --- .../http/server/reactive/PathContainer.java | 14 ++++- .../UrlBasedCorsConfigurationSource.java | 5 +- .../web/util/pattern/ParsingPathMatcher.java | 3 +- .../web/util/pattern/PathPattern.java | 34 +++-------- .../util/pattern/PathPatternMatcherTests.java | 12 +++- .../server/PathResourceLookupFunction.java | 14 ++--- .../function/server/RequestPredicates.java | 5 +- .../reactive/handler/PathPatternRegistry.java | 2 +- .../resource/ResourceUrlProvider.java | 57 +++++++++---------- .../handler/PathPatternRegistryTests.java | 12 +++- .../resource/ResourceUrlProviderTests.java | 11 +++- 11 files changed, 90 insertions(+), 79 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/PathContainer.java b/spring-web/src/main/java/org/springframework/http/server/reactive/PathContainer.java index 795ef4434f2..c6d00dc4ebb 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/PathContainer.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/PathContainer.java @@ -60,7 +60,19 @@ public interface PathContainer { * @return the sub-path */ static PathContainer subPath(PathContainer path, int index) { - return DefaultPathContainer.subPath(path, index, path.elements().size()); + return subPath(path, index, path.elements().size()); + } + + /** + * Extract a sub-path from the given start offset (inclusive) into the path + * element list and to the end offset (exclusive). + * @param path the path to extract from + * @param startIndex the start element index (inclusive) + * @param endIndex the end element index (exclusive) + * @return the sub-path + */ + static PathContainer subPath(PathContainer path, int startIndex, int endIndex) { + return DefaultPathContainer.subPath(path, startIndex, endIndex); } diff --git a/spring-web/src/main/java/org/springframework/web/cors/reactive/UrlBasedCorsConfigurationSource.java b/spring-web/src/main/java/org/springframework/web/cors/reactive/UrlBasedCorsConfigurationSource.java index 68db63fa709..11422041cc8 100644 --- a/spring-web/src/main/java/org/springframework/web/cors/reactive/UrlBasedCorsConfigurationSource.java +++ b/spring-web/src/main/java/org/springframework/web/cors/reactive/UrlBasedCorsConfigurationSource.java @@ -19,6 +19,7 @@ package org.springframework.web.cors.reactive; import java.util.LinkedHashMap; import java.util.Map; +import org.springframework.http.server.reactive.PathContainer; import org.springframework.lang.Nullable; import org.springframework.web.cors.CorsConfiguration; import org.springframework.web.server.ServerWebExchange; @@ -66,10 +67,10 @@ public class UrlBasedCorsConfigurationSource implements CorsConfigurationSource @Override public CorsConfiguration getCorsConfiguration(ServerWebExchange exchange) { - String lookupPath = exchange.getRequest().getPath().pathWithinApplication().value(); + PathContainer lookupPath = exchange.getRequest().getPath().pathWithinApplication(); return this.corsConfigurations.entrySet().stream() .filter(entry -> entry.getKey().matches(lookupPath)) - .map(entry -> entry.getValue()) + .map(Map.Entry::getValue) .findFirst() .orElse(null); } diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/ParsingPathMatcher.java b/spring-web/src/main/java/org/springframework/web/util/pattern/ParsingPathMatcher.java index 0440b8c242d..f044888091f 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/ParsingPathMatcher.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/ParsingPathMatcher.java @@ -73,7 +73,8 @@ public class ParsingPathMatcher implements PathMatcher { @Override public String extractPathWithinPattern(String pattern, String path) { PathPattern pathPattern = getPathPattern(pattern); - return pathPattern.extractPathWithinPattern(path); + PathContainer pathContainer = PathContainer.parse(path, StandardCharsets.UTF_8); + return pathPattern.extractPathWithinPattern(pathContainer).value(); } @Override diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/PathPattern.java b/spring-web/src/main/java/org/springframework/web/util/pattern/PathPattern.java index a3babd99b6a..2a86dac8e82 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/PathPattern.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/PathPattern.java @@ -158,22 +158,6 @@ public class PathPattern implements Comparable { } - // TODO: remove String-variants - - public boolean matches(String path) { - return matches(PathContainer.parse(path, StandardCharsets.UTF_8)); - } - - public PathMatchResult matchAndExtract(String path) { - return matchAndExtract(PathContainer.parse(path, StandardCharsets.UTF_8)); - } - - @Nullable - public PathRemainingMatchInfo getPathRemaining(String path) { - return getPathRemaining(PathContainer.parse(path, StandardCharsets.UTF_8)); - } - - /** * @param pathContainer the candidate path container to attempt to match against this pattern * @return true if the path matches this pattern @@ -262,13 +246,6 @@ public class PathPattern implements Comparable { } } - // TODO: implement extractPathWithinPattern natively for PathContainer - - public PathContainer extractPathWithinPattern(PathContainer path) { - String result = extractPathWithinPattern(path.value()); - return PathContainer.parse(result, StandardCharsets.UTF_8); - } - /** * Given a full path, determine the pattern-mapped part.

For example:

    *
  • '{@code /docs/cvs/commit.html}' and '{@code /docs/cvs/commit.html} -> ''
    • @@ -283,7 +260,14 @@ public class PathPattern implements Comparable { * @param path a path that matches this pattern * @return the subset of the path that is matched by pattern or "" if none of it is matched by pattern elements */ - public String extractPathWithinPattern(String path) { + public PathContainer extractPathWithinPattern(PathContainer path) { + String result = extractPathWithinPattern(path.value()); + return PathContainer.parse(result, StandardCharsets.UTF_8); + } + + // TODO: implement extractPathWithinPattern natively for PathContainer + + private String extractPathWithinPattern(String path) { // assert this.matches(path) PathElement elem = head; int separatorCount = 0; @@ -517,7 +501,7 @@ public class PathPattern implements Comparable { } /** - * A holder for the result of a {@link PathPattern#getPathRemaining(String)} call. Holds + * A holder for the result of a {@link PathPattern#getPathRemaining} call.Holds * information on the path left after the first part has successfully matched a pattern * and any variables bound in that first part that matched. */ diff --git a/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternMatcherTests.java b/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternMatcherTests.java index 36e9d8a395e..c425bff60ba 100644 --- a/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternMatcherTests.java +++ b/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternMatcherTests.java @@ -36,8 +36,14 @@ import org.springframework.util.AntPathMatcher; import org.springframework.web.util.pattern.PathPattern.PathMatchResult; import org.springframework.web.util.pattern.PathPattern.PathRemainingMatchInfo; -import static org.hamcrest.CoreMatchers.*; -import static org.junit.Assert.*; +import static org.hamcrest.CoreMatchers.containsString; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** * Exercise matching of {@link PathPattern} objects. @@ -1368,7 +1374,7 @@ public class PathPatternMatcherTests { private void checkExtractPathWithinPattern(String pattern, String path, String expected) { PathPatternParser ppp = new PathPatternParser(); PathPattern pp = ppp.parse(pattern); - String s = pp.extractPathWithinPattern(path); + String s = pp.extractPathWithinPattern(toPathContainer(path)).value(); assertEquals(expected, s); } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/PathResourceLookupFunction.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/PathResourceLookupFunction.java index 2bb5e0ac31b..5bb605ec41e 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/PathResourceLookupFunction.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/PathResourceLookupFunction.java @@ -26,6 +26,7 @@ import reactor.core.publisher.Mono; import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.Resource; import org.springframework.core.io.UrlResource; +import org.springframework.http.server.reactive.PathContainer; import org.springframework.util.ResourceUtils; import org.springframework.util.StringUtils; import org.springframework.web.util.pattern.PathPattern; @@ -54,19 +55,18 @@ class PathResourceLookupFunction implements Function apply(ServerRequest request) { - String path = processPath(request.path()); + PathContainer pathContainer = request.pathContainer(); + if (!this.pattern.matches(pathContainer)) { + return Mono.empty(); + } + pathContainer = this.pattern.extractPathWithinPattern(pathContainer); + String path = processPath(pathContainer.value()); if (path.contains("%")) { path = StringUtils.uriDecode(path, StandardCharsets.UTF_8); } if (!StringUtils.hasLength(path) || isInvalidPath(path)) { return Mono.empty(); } - if (!this.pattern.matches(path)) { - return Mono.empty(); - } - else { - path = this.pattern.extractPathWithinPattern(path); - } try { Resource resource = this.location.createRelative(path); if (resource.exists() && resource.isReadable() && isResourceUnderLocation(resource)) { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java index f16bc32a8c6..2dcbcd8e156 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java @@ -341,10 +341,11 @@ public abstract class RequestPredicates { @Override public boolean test(ServerRequest request) { - boolean match = this.pattern.matches(request.pathContainer()); + PathContainer pathContainer = request.pathContainer(); + boolean match = this.pattern.matches(pathContainer); traceMatch("Pattern", this.pattern.getPatternString(), request.path(), match); if (match) { - mergeTemplateVariables(request, this.pattern.matchAndExtract(request.path()).getUriVariables()); + mergeTemplateVariables(request, this.pattern.matchAndExtract(pathContainer).getUriVariables()); return true; } else { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/PathPatternRegistry.java b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/PathPatternRegistry.java index 64e7b2a3f72..ab03bdcea08 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/PathPatternRegistry.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/PathPatternRegistry.java @@ -109,7 +109,7 @@ public class PathPatternRegistry { * patterns first, according to the given {@code lookupPath}. * @param lookupPath the URL lookup path to be matched against */ - public SortedSet> findMatches(String lookupPath) { + public SortedSet> findMatches(PathContainer lookupPath) { return this.patternsMap.entrySet().stream() .filter(entry -> entry.getKey().matches(lookupPath)) .sorted(Comparator.comparing(Map.Entry::getKey)) 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 2090af065af..3d7f66d37a7 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 @@ -16,6 +16,7 @@ package org.springframework.web.reactive.resource; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -30,6 +31,7 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationListener; 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.web.reactive.handler.PathMatchResult; @@ -136,8 +138,9 @@ public class ResourceUrlProvider implements ApplicationListener prefix + resolvedPath + suffix); - } - - private int getLookupPathIndex(ServerWebExchange exchange) { ServerHttpRequest request = exchange.getRequest(); - String requestPath = request.getURI().getPath(); - String lookupPath = exchange.getRequest().getPath().pathWithinApplication().value(); - return requestPath.indexOf(lookupPath); + int queryIndex = getQueryIndex(requestUrl); + String lookupPath = requestUrl.substring(0, queryIndex); + String query = requestUrl.substring(queryIndex); + PathContainer parsedLookupPath = PathContainer.parse(lookupPath, StandardCharsets.UTF_8); + return getForLookupPath(parsedLookupPath).map(resolvedPath -> + request.getPath().contextPath().value() + resolvedPath + query); } - private int getEndPathIndex(String lookupPath) { + private int getQueryIndex(String lookupPath) { int suffixIndex = lookupPath.length(); int queryIndex = lookupPath.indexOf("?"); if (queryIndex > 0) { @@ -186,35 +183,33 @@ public class ResourceUrlProvider implements ApplicationListener getForLookupPath(String lookupPath) { + public final Mono getForLookupPath(PathContainer lookupPath) { if (logger.isTraceEnabled()) { logger.trace("Getting resource URL for lookup path \"" + lookupPath + "\""); } - Set> matchResults = this.patternRegistry.findMatches(lookupPath); - - if (matchResults.isEmpty()) { - return Mono.empty(); - } + Set> matches = this.patternRegistry.findMatches(lookupPath); - return Flux.fromIterable(matchResults) - .concatMap(result -> { - String pathWithinMapping = result.getPattern().extractPathWithinPattern(lookupPath); - String pathMapping = lookupPath.substring(0, lookupPath.indexOf(pathWithinMapping)); + return Flux.fromIterable(matches).next() + .flatMap(result -> { + PathContainer path = result.getPattern().extractPathWithinPattern(lookupPath); + int endIndex = lookupPath.elements().size() - path.elements().size(); + PathContainer mapping = PathContainer.subPath(lookupPath, 0, endIndex); if (logger.isTraceEnabled()) { - logger.trace("Invoking ResourceResolverChain for URL pattern \"" + result.getPattern() + "\""); + logger.trace("Invoking ResourceResolverChain for URL pattern " + + "\"" + result.getPattern() + "\""); } ResourceWebHandler handler = result.getHandler(); - ResourceResolverChain chain = new DefaultResourceResolverChain(handler.getResourceResolvers()); - return chain.resolveUrlPath(pathWithinMapping, handler.getLocations()) + List resolvers = handler.getResourceResolvers(); + ResourceResolverChain chain = new DefaultResourceResolverChain(resolvers); + return chain.resolveUrlPath(path.value(), handler.getLocations()) .map(resolvedPath -> { if (logger.isTraceEnabled()) { logger.trace("Resolved public resource URL path \"" + resolvedPath + "\""); } - return pathMapping + resolvedPath; + return mapping.value() + resolvedPath; }); - }) - .next(); + }); } } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/handler/PathPatternRegistryTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/handler/PathPatternRegistryTests.java index 69de1467cd1..62a8b7cf31c 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/handler/PathPatternRegistryTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/handler/PathPatternRegistryTests.java @@ -16,6 +16,7 @@ package org.springframework.web.reactive.handler; +import java.nio.charset.StandardCharsets; import java.util.Collection; import java.util.List; import java.util.Set; @@ -28,6 +29,7 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.springframework.http.server.reactive.PathContainer; import org.springframework.web.util.pattern.PathPattern; import org.springframework.web.util.pattern.PathPatternParser; import org.springframework.web.util.pattern.PatternParseException; @@ -73,14 +75,17 @@ public class PathPatternRegistryTests { this.registry.register("/fo?", new Object()); this.registry.register("/f?o", new Object()); - Set> matches = this.registry.findMatches("/foo"); + + PathContainer path = PathContainer.parse("/foo", StandardCharsets.UTF_8); + Set> matches = this.registry.findMatches(path); assertThat(toPatterns(matches), contains(pattern("/f?o"), pattern("/fo?"))); } @Test public void findNoMatch() { this.registry.register("/foo/{bar}", new Object()); - assertThat(this.registry.findMatches("/other"), hasSize(0)); + PathContainer path = PathContainer.parse("/other", StandardCharsets.UTF_8); + assertThat(this.registry.findMatches(path), hasSize(0)); } @Test @@ -88,7 +93,8 @@ public class PathPatternRegistryTests { this.registry.register("/foo/{*baz}", new Object()); this.registry.register("/foo/bar/baz", new Object()); this.registry.register("/foo/bar/{baz}", new Object()); - Set> matches = this.registry.findMatches("/foo/bar/baz"); + PathContainer path = PathContainer.parse("/foo/bar/baz", StandardCharsets.UTF_8); + Set> matches = this.registry.findMatches(path); assertThat(toPatterns(matches), contains(pattern("/foo/bar/baz"), pattern("/foo/bar/{baz}"), pattern("/foo/{*baz}"))); } 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 a24895a8fe1..8ae39f7a237 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 @@ -16,6 +16,7 @@ package org.springframework.web.reactive.resource; +import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.ArrayList; import java.util.HashMap; @@ -32,6 +33,7 @@ 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; @@ -73,7 +75,8 @@ public class ResourceUrlProviderTests { @Test public void getStaticResourceUrl() { - String url = this.urlProvider.getForLookupPath("/resources/foo.css").block(Duration.ofSeconds(5)); + PathContainer path = PathContainer.parse("/resources/foo.css", StandardCharsets.UTF_8); + String url = this.urlProvider.getForLookupPath(path).block(Duration.ofSeconds(5)); assertEquals("/resources/foo.css", url); } @@ -102,7 +105,8 @@ public class ResourceUrlProviderTests { resolvers.add(new PathResourceResolver()); this.handler.setResourceResolvers(resolvers); - String url = this.urlProvider.getForLookupPath("/resources/foo.css").block(Duration.ofSeconds(5)); + PathContainer path = PathContainer.parse("/resources/foo.css", StandardCharsets.UTF_8); + String url = this.urlProvider.getForLookupPath(path).block(Duration.ofSeconds(5)); assertEquals("/resources/foo-e36d2e05253c6c7085a91522ce43a0b4.css", url); } @@ -123,7 +127,8 @@ public class ResourceUrlProviderTests { this.handlerMap.put("/resources/*.css", otherHandler); this.urlProvider.setHandlerMap(this.handlerMap); - String url = this.urlProvider.getForLookupPath("/resources/foo.css").block(Duration.ofSeconds(5)); + PathContainer path = PathContainer.parse("/resources/foo.css", StandardCharsets.UTF_8); + String url = this.urlProvider.getForLookupPath(path).block(Duration.ofSeconds(5)); assertEquals("/resources/foo-e36d2e05253c6c7085a91522ce43a0b4.css", url); }