From d2685dfe672ab9be43bff4a73a15f3c3a56839ea Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 2 Jun 2017 15:15:42 -0400 Subject: [PATCH] Add static factory/accessor methods to LookupPath Issue: SPR-15397 --- .../UrlBasedCorsConfigurationSource.java | 6 +-- .../web/server/support/LookupPath.java | 38 ++++++++++++++++++- .../UrlBasedCorsConfigurationSourceTests.java | 11 ++---- .../handler/AbstractHandlerMapping.java | 15 -------- .../handler/AbstractUrlHandlerMapping.java | 2 +- .../resource/ResourceUrlProvider.java | 4 +- .../condition/PatternsRequestCondition.java | 11 ++---- .../method/AbstractHandlerMethodMapping.java | 2 +- .../view/ViewResolutionResultHandler.java | 4 +- .../PatternsRequestConditionTests.java | 3 +- .../condition/RequestMappingInfoTests.java | 23 +++++------ .../ViewResolutionResultHandlerTests.java | 15 +++----- 12 files changed, 64 insertions(+), 70 deletions(-) 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 737b5fdc6e8..39c37727024 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 @@ -24,8 +24,8 @@ import org.springframework.util.Assert; import org.springframework.util.PathMatcher; import org.springframework.web.cors.CorsConfiguration; import org.springframework.web.server.ServerWebExchange; -import org.springframework.web.util.pattern.ParsingPathMatcher; import org.springframework.web.server.support.LookupPath; +import org.springframework.web.util.pattern.ParsingPathMatcher; /** * Provide a per reactive request {@link CorsConfiguration} instance based on a @@ -80,9 +80,7 @@ public class UrlBasedCorsConfigurationSource implements CorsConfigurationSource @Override public CorsConfiguration getCorsConfiguration(ServerWebExchange exchange) { - String lookupPath = exchange.getAttribute(LookupPath.LOOKUP_PATH_ATTRIBUTE) - .map(LookupPath::getPath) - .orElseThrow(() -> new IllegalStateException("No LookupPath attribute.")); + String lookupPath = LookupPath.getCurrent(exchange).getPath(); for (Map.Entry entry : this.corsConfigurations.entrySet()) { if (this.pathMatcher.match(entry.getKey(), lookupPath)) { return entry.getValue(); diff --git a/spring-web/src/main/java/org/springframework/web/server/support/LookupPath.java b/spring-web/src/main/java/org/springframework/web/server/support/LookupPath.java index 3bc7a9b5a17..be5e247ad9a 100644 --- a/spring-web/src/main/java/org/springframework/web/server/support/LookupPath.java +++ b/spring-web/src/main/java/org/springframework/web/server/support/LookupPath.java @@ -17,17 +17,23 @@ package org.springframework.web.server.support; import org.springframework.lang.Nullable; +import org.springframework.web.server.ServerWebExchange; /** * Lookup path information of an incoming HTTP request. * * @author Brian Clozel + * @author Rossen Stoyanchev * @since 5.0 * @see HttpRequestPathHelper */ public final class LookupPath { - public static final String LOOKUP_PATH_ATTRIBUTE = LookupPath.class.getName(); + /** + * Name of request attribute under which the LookupPath is stored via + * {@link #getOrCreate} and accessed via {@link #getCurrent}. + */ + public static final String LOOKUP_PATH_ATTRIBUTE_NAME = LookupPath.class.getName(); private final String path; @@ -70,4 +76,34 @@ public final class LookupPath { } } + + /** + * Get the LookupPath for the current request from the request attribute + * {@link #LOOKUP_PATH_ATTRIBUTE_NAME} or otherwise create and stored it + * under that attribute for subsequent use. + * @param exchange the current exchange + * @param pathHelper the pathHelper to create the LookupPath with + * @return the LookupPath for the current request + */ + public static LookupPath getOrCreate(ServerWebExchange exchange, HttpRequestPathHelper pathHelper) { + return exchange.getAttribute(LookupPath.LOOKUP_PATH_ATTRIBUTE_NAME) + .orElseGet(() -> { + LookupPath lookupPath = pathHelper.getLookupPathForRequest(exchange); + exchange.getAttributes().put(LookupPath.LOOKUP_PATH_ATTRIBUTE_NAME, lookupPath); + return lookupPath; + }); + } + + /** + * Get the LookupPath for the current request from the request attribute + * {@link #LOOKUP_PATH_ATTRIBUTE_NAME} or raise an {@link IllegalStateException} + * if not found. + * @param exchange the current exchange + * @return the LookupPath, never {@code null} + */ + public static LookupPath getCurrent(ServerWebExchange exchange) { + return exchange.getAttribute(LookupPath.LOOKUP_PATH_ATTRIBUTE_NAME) + .orElseThrow(() -> new IllegalStateException("No LookupPath attribute.")); + } + } diff --git a/spring-web/src/test/java/org/springframework/web/cors/reactive/UrlBasedCorsConfigurationSourceTests.java b/spring-web/src/test/java/org/springframework/web/cors/reactive/UrlBasedCorsConfigurationSourceTests.java index 26c98ea9eee..ae74e368141 100644 --- a/spring-web/src/test/java/org/springframework/web/cors/reactive/UrlBasedCorsConfigurationSourceTests.java +++ b/spring-web/src/test/java/org/springframework/web/cors/reactive/UrlBasedCorsConfigurationSourceTests.java @@ -41,7 +41,7 @@ public class UrlBasedCorsConfigurationSourceTests { @Test public void empty() { ServerWebExchange exchange = MockServerHttpRequest.get("/bar/test.html").toExchange(); - initLookupPath(exchange); + LookupPath.getOrCreate(exchange, new HttpRequestPathHelper()); assertNull(this.configSource.getCorsConfiguration(exchange)); } @@ -51,11 +51,11 @@ public class UrlBasedCorsConfigurationSourceTests { this.configSource.registerCorsConfiguration("/bar/**", config); ServerWebExchange exchange = MockServerHttpRequest.get("/foo/test.html").toExchange(); - initLookupPath(exchange); + LookupPath.getOrCreate(exchange, new HttpRequestPathHelper()); assertNull(this.configSource.getCorsConfiguration(exchange)); exchange = MockServerHttpRequest.get("/bar/test.html").toExchange(); - initLookupPath(exchange); + LookupPath.getOrCreate(exchange, new HttpRequestPathHelper()); assertEquals(config, this.configSource.getCorsConfiguration(exchange)); } @@ -64,9 +64,4 @@ public class UrlBasedCorsConfigurationSourceTests { this.configSource.getCorsConfigurations().put("/**", new CorsConfiguration()); } - private void initLookupPath(ServerWebExchange exchange) { - exchange.getAttributes().put(LookupPath.LOOKUP_PATH_ATTRIBUTE, - new HttpRequestPathHelper().getLookupPathForRequest(exchange)); - } - } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractHandlerMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractHandlerMapping.java index cc83e7e625d..dbc362c4516 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractHandlerMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractHandlerMapping.java @@ -17,7 +17,6 @@ package org.springframework.web.reactive.handler; import java.util.Map; -import java.util.Optional; import reactor.core.publisher.Mono; @@ -33,7 +32,6 @@ import org.springframework.web.cors.reactive.CorsUtils; import org.springframework.web.cors.reactive.DefaultCorsProcessor; import org.springframework.web.cors.reactive.UrlBasedCorsConfigurationSource; import org.springframework.web.reactive.HandlerMapping; -import org.springframework.web.server.support.LookupPath; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebHandler; import org.springframework.web.server.support.HttpRequestPathHelper; @@ -174,19 +172,6 @@ public abstract class AbstractHandlerMapping extends ApplicationObjectSupport im }); } - protected LookupPath getLookupPath(ServerWebExchange exchange) { - return exchange.getAttribute(LookupPath.LOOKUP_PATH_ATTRIBUTE) - .orElseGet(() -> { - LookupPath lookupPath = createLookupPath(exchange); - exchange.getAttributes().put(LookupPath.LOOKUP_PATH_ATTRIBUTE, lookupPath); - return lookupPath; - }); - } - - protected LookupPath createLookupPath(ServerWebExchange exchange) { - return getPathHelper().getLookupPathForRequest(exchange); - } - /** * Look up a handler for the given request, returning an empty {@code Mono} * if no specific one is found. This method is called by {@link #getHandler}. diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractUrlHandlerMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractUrlHandlerMapping.java index c353b4376a9..bad1b32e871 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractUrlHandlerMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractUrlHandlerMapping.java @@ -101,7 +101,7 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { @Override public Mono getHandlerInternal(ServerWebExchange exchange) { - LookupPath lookupPath = getLookupPath(exchange); + LookupPath lookupPath = LookupPath.getOrCreate(exchange, getPathHelper()); Object handler; try { handler = lookupHandler(lookupPath, exchange); 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 e157ab096ac..3fb06e32637 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 @@ -185,9 +185,7 @@ public class ResourceUrlProvider implements ApplicationListenergetAttribute(LookupPath.LOOKUP_PATH_ATTRIBUTE) - .orElseGet(() -> getPathHelper().getLookupPathForRequest(exchange)); + LookupPath lookupPath = LookupPath.getOrCreate(exchange, getPathHelper()); return requestPath.indexOf(lookupPath.getPath()); } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/PatternsRequestCondition.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/PatternsRequestCondition.java index 8de476fdd66..b1bd60bcfd8 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/PatternsRequestCondition.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/PatternsRequestCondition.java @@ -187,7 +187,7 @@ public final class PatternsRequestCondition extends AbstractRequestCondition matches = getMatchingPatterns(lookupPath); return matches.isEmpty() ? null : @@ -195,11 +195,6 @@ public final class PatternsRequestCondition extends AbstractRequestConditiongetAttribute(LookupPath.LOOKUP_PATH_ATTRIBUTE) - .orElseThrow(() -> new IllegalStateException("No LookupPath attribute.")); - } - /** * Find the patterns matching the given lookup path. Invoking this method should * yield results equivalent to those of calling @@ -263,8 +258,8 @@ public final class PatternsRequestCondition extends AbstractRequestCondition patternComparator = this.pathMatcher.getPatternComparator(lookupPath.getPath()); + String path = LookupPath.getCurrent(exchange).getPath(); + Comparator patternComparator = this.pathMatcher.getPatternComparator(path); Iterator iterator = this.patterns.iterator(); Iterator iteratorOther = other.patterns.iterator(); while (iterator.hasNext() && iteratorOther.hasNext()) { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java index 0b686282020..c6cbb1429fa 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java @@ -257,7 +257,7 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap */ @Override public Mono getHandlerInternal(ServerWebExchange exchange) { - LookupPath lookupPath = getLookupPath(exchange); + LookupPath lookupPath = LookupPath.getOrCreate(exchange, getPathHelper()); if (logger.isDebugEnabled()) { logger.debug("Looking up handler method for path " + lookupPath); } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java index c70ee6bff29..30483c70510 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java @@ -257,9 +257,7 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport * Use the request path the leading and trailing slash stripped. */ private String getDefaultViewName(ServerWebExchange exchange) { - String path = exchange.getAttribute(LookupPath.LOOKUP_PATH_ATTRIBUTE) - .map(LookupPath::getPath) - .orElseThrow(() -> new IllegalStateException("No LookupPath attribute.")); + String path = LookupPath.getCurrent(exchange).getPath(); if (path.startsWith("/")) { path = path.substring(1); } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/PatternsRequestConditionTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/PatternsRequestConditionTests.java index d29f4890109..13b1c78a8db 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/PatternsRequestConditionTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/PatternsRequestConditionTests.java @@ -222,8 +222,7 @@ public class PatternsRequestConditionTests { private MockServerWebExchange initExchange(String path) { MockServerWebExchange exchange = get(path).toExchange(); - exchange.getAttributes().put(LookupPath.LOOKUP_PATH_ATTRIBUTE, - new HttpRequestPathHelper().getLookupPathForRequest(exchange)); + LookupPath.getOrCreate(exchange, new HttpRequestPathHelper()); return exchange; } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/RequestMappingInfoTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/RequestMappingInfoTests.java index a20a0de5f48..ae0badb2e4b 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/RequestMappingInfoTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/RequestMappingInfoTests.java @@ -29,9 +29,9 @@ import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; import org.springframework.mock.http.server.reactive.test.MockServerWebExchange; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.reactive.result.method.RequestMappingInfo; +import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.support.HttpRequestPathHelper; import org.springframework.web.server.support.LookupPath; -import org.springframework.web.server.ServerWebExchange; import static java.util.Arrays.asList; import static org.junit.Assert.assertEquals; @@ -48,7 +48,7 @@ import static org.springframework.web.reactive.result.method.RequestMappingInfo. */ public class RequestMappingInfoTests { - // TODO: CORS pre-flight (see @Ignored) + // TODO: CORS pre-flight (see @Ignore) @Test @@ -67,7 +67,7 @@ public class RequestMappingInfoTests { @Test public void matchPatternsCondition() { MockServerWebExchange exchange = MockServerHttpRequest.get("/foo").toExchange(); - initLookupPath(exchange); + LookupPath.getOrCreate(exchange, new HttpRequestPathHelper()); RequestMappingInfo info = paths("/foo*", "/bar").build(); RequestMappingInfo expected = paths("/foo*").build(); @@ -83,7 +83,7 @@ public class RequestMappingInfoTests { @Test public void matchParamsCondition() { ServerWebExchange exchange = MockServerHttpRequest.get("/foo?foo=bar").toExchange(); - initLookupPath(exchange); + LookupPath.getOrCreate(exchange, new HttpRequestPathHelper()); RequestMappingInfo info = paths("/foo").params("foo=bar").build(); RequestMappingInfo match = info.getMatchingCondition(exchange); @@ -99,7 +99,7 @@ public class RequestMappingInfoTests { @Test public void matchHeadersCondition() { ServerWebExchange exchange = MockServerHttpRequest.get("/foo").header("foo", "bar").toExchange(); - initLookupPath(exchange); + LookupPath.getOrCreate(exchange, new HttpRequestPathHelper()); RequestMappingInfo info = paths("/foo").headers("foo=bar").build(); RequestMappingInfo match = info.getMatchingCondition(exchange); @@ -115,7 +115,7 @@ public class RequestMappingInfoTests { @Test public void matchConsumesCondition() { ServerWebExchange exchange = MockServerHttpRequest.post("/foo").contentType(MediaType.TEXT_PLAIN).toExchange(); - initLookupPath(exchange); + LookupPath.getOrCreate(exchange, new HttpRequestPathHelper()); RequestMappingInfo info = paths("/foo").consumes("text/plain").build(); RequestMappingInfo match = info.getMatchingCondition(exchange); @@ -131,7 +131,7 @@ public class RequestMappingInfoTests { @Test public void matchProducesCondition() { ServerWebExchange exchange = MockServerHttpRequest.get("/foo").accept(MediaType.TEXT_PLAIN).toExchange(); - initLookupPath(exchange); + LookupPath.getOrCreate(exchange, new HttpRequestPathHelper()); RequestMappingInfo info = paths("/foo").produces("text/plain").build(); RequestMappingInfo match = info.getMatchingCondition(exchange); @@ -147,7 +147,7 @@ public class RequestMappingInfoTests { @Test public void matchCustomCondition() { ServerWebExchange exchange = MockServerHttpRequest.get("/foo?foo=bar").toExchange(); - initLookupPath(exchange); + LookupPath.getOrCreate(exchange, new HttpRequestPathHelper()); RequestMappingInfo info = paths("/foo").params("foo=bar").build(); RequestMappingInfo match = info.getMatchingCondition(exchange); @@ -169,7 +169,7 @@ public class RequestMappingInfoTests { RequestMappingInfo oneMethodOneParam = paths().methods(RequestMethod.GET).params("foo").build(); ServerWebExchange exchange = MockServerHttpRequest.get("/foo").toExchange(); - initLookupPath(exchange); + LookupPath.getOrCreate(exchange, new HttpRequestPathHelper()); Comparator comparator = (info, otherInfo) -> info.compareTo(otherInfo, exchange); List list = asList(none, oneMethod, oneMethodOneParam); @@ -279,9 +279,4 @@ public class RequestMappingInfoTests { assertNull("Pre-flight should match the ACCESS_CONTROL_REQUEST_METHOD", match); } - private void initLookupPath(ServerWebExchange exchange) { - exchange.getAttributes().put(LookupPath.LOOKUP_PATH_ATTRIBUTE, - new HttpRequestPathHelper().getLookupPathForRequest(exchange)); - } - } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java index 49753dd2062..59fcacbb5d9 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java @@ -219,17 +219,17 @@ public class ViewResolutionResultHandlerTests { ViewResolutionResultHandler handler = resultHandler(new TestViewResolver("account")); MockServerWebExchange exchange = get("/account").toExchange(); - initLookupPath(exchange); + LookupPath.getOrCreate(exchange, new HttpRequestPathHelper()); handler.handleResult(exchange, result).block(Duration.ofMillis(5000)); assertResponseBody(exchange, "account: {id=123}"); exchange = get("/account/").toExchange(); - initLookupPath(exchange); + LookupPath.getOrCreate(exchange, new HttpRequestPathHelper()); handler.handleResult(exchange, result).block(Duration.ofMillis(5000)); assertResponseBody(exchange, "account: {id=123}"); exchange = get("/account.123").toExchange(); - initLookupPath(exchange); + LookupPath.getOrCreate(exchange, new HttpRequestPathHelper()); handler.handleResult(exchange, result).block(Duration.ofMillis(5000)); assertResponseBody(exchange, "account: {id=123}"); } @@ -256,7 +256,7 @@ public class ViewResolutionResultHandlerTests { HandlerResult handlerResult = new HandlerResult(new Object(), value, returnType, this.bindingContext); MockServerWebExchange exchange = get("/account").accept(APPLICATION_JSON).toExchange(); - initLookupPath(exchange); + LookupPath.getOrCreate(exchange, new HttpRequestPathHelper()); TestView defaultView = new TestView("jsonView", APPLICATION_JSON); @@ -307,11 +307,6 @@ public class ViewResolutionResultHandlerTests { assertEquals("/", response.getHeaders().getLocation().toString()); } - private void initLookupPath(ServerWebExchange exchange) { - exchange.getAttributes().put(LookupPath.LOOKUP_PATH_ATTRIBUTE, - new HttpRequestPathHelper().getLookupPathForRequest(exchange)); - } - private ViewResolutionResultHandler resultHandler(ViewResolver... resolvers) { return resultHandler(Collections.emptyList(), resolvers); @@ -333,7 +328,7 @@ public class ViewResolutionResultHandlerTests { model.addAttribute("id", "123"); HandlerResult result = new HandlerResult(new Object(), returnValue, returnType, this.bindingContext); MockServerWebExchange exchange = get(path).toExchange(); - initLookupPath(exchange); + LookupPath.getOrCreate(exchange, new HttpRequestPathHelper()); resultHandler(resolvers).handleResult(exchange, result).block(Duration.ofSeconds(5)); assertResponseBody(exchange, responseBody); return exchange;