From d303c8a20f70f623ebdf6861868041f8664de403 Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Thu, 18 Oct 2018 14:48:45 +0200 Subject: [PATCH] Store PathPattern instead of String in attributes This commit changes the attributes stored under RouterFunctions.MATCHING_PATTERN_ATTRIBUTE and HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE from a String to a PathPattern, similar to what annotated controllers set. Issue: SPR-17395 --- .../function/server/RequestPredicates.java | 24 +++---- .../function/server/RouterFunctions.java | 2 +- .../server/support/RouterFunctionMapping.java | 5 +- .../DispatcherHandlerIntegrationTests.java | 69 +++++++++++++++++-- .../server/NestedRouteIntegrationTests.java | 12 ++-- 5 files changed, 87 insertions(+), 25 deletions(-) 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 eda84732e8..d8396ea7c5 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 @@ -369,12 +369,9 @@ public abstract class RequestPredicates { } } - private static String mergePatterns(@Nullable String oldPattern, String newPattern) { + private static PathPattern mergePatterns(@Nullable PathPattern oldPattern, PathPattern newPattern) { if (oldPattern != null) { - if (oldPattern.endsWith("/") && newPattern.startsWith("/")) { - oldPattern = oldPattern.substring(0, oldPattern.length() - 1); - } - return oldPattern + newPattern; + return oldPattern.combine(newPattern); } else { return newPattern; @@ -429,10 +426,9 @@ public abstract class RequestPredicates { public boolean test(ServerRequest request) { PathContainer pathContainer = request.pathContainer(); PathPattern.PathMatchInfo info = this.pattern.matchAndExtract(pathContainer); - String patternString = this.pattern.getPatternString(); - traceMatch("Pattern", patternString, request.path(), info != null); + traceMatch("Pattern", this.pattern.getPatternString(), request.path(), info != null); if (info != null) { - mergeAttributes(request, info.getUriVariables(), patternString); + mergeAttributes(request, info.getUriVariables(), this.pattern); return true; } else { @@ -441,13 +437,13 @@ public abstract class RequestPredicates { } private static void mergeAttributes(ServerRequest request, Map variables, - String pattern) { + PathPattern pattern) { Map pathVariables = mergePathVariables(request.pathVariables(), variables); request.attributes().put(RouterFunctions.URI_TEMPLATE_VARIABLES_ATTRIBUTE, Collections.unmodifiableMap(pathVariables)); pattern = mergePatterns( - (String) request.attributes().get(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE), + (PathPattern) request.attributes().get(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE), pattern); request.attributes().put(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE, pattern); } @@ -455,7 +451,7 @@ public abstract class RequestPredicates { @Override public Optional nest(ServerRequest request) { return Optional.ofNullable(this.pattern.matchStartOfPath(request.pathContainer())) - .map(info -> new SubPathServerRequestWrapper(request, info, this.pattern.getPatternString())); + .map(info -> new SubPathServerRequestWrapper(request, info, this.pattern)); } @Override @@ -662,21 +658,21 @@ public abstract class RequestPredicates { private final Map attributes; public SubPathServerRequestWrapper(ServerRequest request, - PathPattern.PathRemainingMatchInfo info, String pattern) { + PathPattern.PathRemainingMatchInfo info, PathPattern pattern) { this.request = request; this.pathContainer = new SubPathContainer(info.getPathRemaining()); this.attributes = mergeAttributes(request, info.getUriVariables(), pattern); } private static Map mergeAttributes(ServerRequest request, - Map pathVariables, String pattern) { + Map pathVariables, PathPattern pattern) { Map result = new ConcurrentHashMap<>(request.attributes()); result.put(RouterFunctions.URI_TEMPLATE_VARIABLES_ATTRIBUTE, mergePathVariables(request.pathVariables(), pathVariables)); pattern = mergePatterns( - (String) request.attributes().get(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE), + (PathPattern) request.attributes().get(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE), pattern); result.put(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE, pattern); return result; diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RouterFunctions.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RouterFunctions.java index af111a1907..b45caea311 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RouterFunctions.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RouterFunctions.java @@ -71,7 +71,7 @@ public abstract class RouterFunctions { /** * Name of the {@link ServerWebExchange#getAttributes() attribute} that - * contains the matching pattern. + * contains the matching pattern, as a {@link org.springframework.web.util.pattern.PathPattern}. */ public static final String MATCHING_PATTERN_ATTRIBUTE = RouterFunctions.class.getName() + ".matchingPattern"; diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/support/RouterFunctionMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/support/RouterFunctionMapping.java index c219f9941d..4c291e180d 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/support/RouterFunctionMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/support/RouterFunctionMapping.java @@ -34,6 +34,7 @@ import org.springframework.web.reactive.function.server.RouterFunctions; import org.springframework.web.reactive.function.server.ServerRequest; import org.springframework.web.reactive.handler.AbstractHandlerMapping; import org.springframework.web.server.ServerWebExchange; +import org.springframework.web.util.pattern.PathPattern; /** * {@code HandlerMapping} implementation that supports {@link RouterFunction RouterFunctions}. @@ -159,8 +160,8 @@ public class RouterFunctionMapping extends AbstractHandlerMapping implements Ini attributes.put(RouterFunctions.REQUEST_ATTRIBUTE, serverRequest); attributes.put(BEST_MATCHING_HANDLER_ATTRIBUTE, handlerFunction); - String matchingPattern = - (String) attributes.get(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE); + PathPattern matchingPattern = + (PathPattern) attributes.get(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE); if (matchingPattern != null) { attributes.put(BEST_MATCHING_PATTERN_ATTRIBUTE, matchingPattern); } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/DispatcherHandlerIntegrationTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/DispatcherHandlerIntegrationTests.java index 136c2820af..7ef150614d 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/DispatcherHandlerIntegrationTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/DispatcherHandlerIntegrationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -17,6 +17,7 @@ package org.springframework.web.reactive.function.server; import java.util.List; +import java.util.Map; import org.junit.Test; import reactor.core.publisher.Flux; @@ -36,12 +37,15 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.client.RestTemplate; import org.springframework.web.reactive.DispatcherHandler; +import org.springframework.web.reactive.HandlerMapping; import org.springframework.web.reactive.config.EnableWebFlux; import org.springframework.web.server.adapter.WebHttpHandlerBuilder; +import org.springframework.web.util.pattern.PathPattern; import static org.junit.Assert.*; -import static org.springframework.web.reactive.function.BodyInserters.*; -import static org.springframework.web.reactive.function.server.RouterFunctions.*; +import static org.springframework.web.reactive.function.BodyInserters.fromPublisher; +import static org.springframework.web.reactive.function.server.RouterFunctions.nest; +import static org.springframework.web.reactive.function.server.RouterFunctions.route; /** * Tests the use of {@link HandlerFunction} and {@link RouterFunction} in a @@ -101,6 +105,15 @@ public class DispatcherHandlerIntegrationTests extends AbstractHttpHandlerIntegr assertEquals("John", result.getBody().getName()); } + @Test + public void attributes() { + ResponseEntity result = + this.restTemplate + .getForEntity("http://localhost:" + this.port + "/attributes/bar", String.class); + + assertEquals(HttpStatus.OK, result.getStatusCode()); + } + @EnableWebFlux @Configuration @@ -116,6 +129,12 @@ public class DispatcherHandlerIntegrationTests extends AbstractHttpHandlerIntegr return new PersonController(); } + @Bean + public AttributesHandler attributesHandler() { + return new AttributesHandler(); + } + + @Bean public RouterFunction> monoRouterFunction(PersonHandler personHandler) { return route(RequestPredicates.GET("/mono"), personHandler::mono); @@ -126,6 +145,12 @@ public class DispatcherHandlerIntegrationTests extends AbstractHttpHandlerIntegr return route(RequestPredicates.GET("/flux"), personHandler::flux); } + @Bean + public RouterFunction attributesRouterFunction(AttributesHandler attributesHandler) { + return nest(RequestPredicates.GET("/attributes"), + route(RequestPredicates.GET("/{foo}"), attributesHandler::attributes)); + } + } @@ -145,8 +170,44 @@ public class DispatcherHandlerIntegrationTests extends AbstractHttpHandlerIntegr } + private static class AttributesHandler { + + @SuppressWarnings("unchecked") + public Mono attributes(ServerRequest request) { + assertTrue(request.attributes().containsKey(RouterFunctions.REQUEST_ATTRIBUTE)); + assertTrue(request.attributes() + .containsKey(HandlerMapping.BEST_MATCHING_HANDLER_ATTRIBUTE)); + + Map pathVariables = + (Map) request.attributes().get(RouterFunctions.URI_TEMPLATE_VARIABLES_ATTRIBUTE); + assertNotNull(pathVariables); + assertEquals(1, pathVariables.size()); + assertEquals("bar", pathVariables.get("foo")); + + pathVariables = + (Map) request.attributes().get(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE); + assertNotNull(pathVariables); + assertEquals(1, pathVariables.size()); + assertEquals("bar", pathVariables.get("foo")); + + + PathPattern pattern = + (PathPattern) request.attributes().get(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE); + assertNotNull(pattern); + assertEquals("/attributes/{foo}", pattern.getPatternString()); + + pattern = (PathPattern) request.attributes() + .get(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE); + assertNotNull(pattern); + assertEquals("/attributes/{foo}", pattern.getPatternString()); + + return ServerResponse.ok().build(); + } + + } + @Controller - private static class PersonController { + public static class PersonController { @RequestMapping("/controller") @ResponseBody diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/NestedRouteIntegrationTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/NestedRouteIntegrationTests.java index 32df4d1ddc..664117d4f2 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/NestedRouteIntegrationTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/NestedRouteIntegrationTests.java @@ -25,7 +25,9 @@ import reactor.core.publisher.Mono; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; +import org.springframework.lang.Nullable; import org.springframework.web.client.RestTemplate; +import org.springframework.web.util.pattern.PathPattern; import static org.junit.Assert.*; import static org.springframework.web.reactive.function.server.RequestPredicates.GET; @@ -122,7 +124,7 @@ public class NestedRouteIntegrationTests extends AbstractRouterFunctionIntegrati private static class NestedHandler { public Mono pattern(ServerRequest request) { - String pattern = matchingPattern(request); + String pattern = matchingPattern(request).getPatternString(); return ServerResponse.ok().syncBody(pattern); } @@ -134,7 +136,8 @@ public class NestedRouteIntegrationTests extends AbstractRouterFunctionIntegrati assertTrue( (pathVariables.equals(attributePathVariables)) || (pathVariables.isEmpty() && (attributePathVariables == null))); - String pattern = matchingPattern(request); + PathPattern pathPattern = matchingPattern(request); + String pattern = pathPattern != null ? pathPattern.getPatternString() : ""; Flux responseBody; if (!pattern.isEmpty()) { responseBody = Flux.just(pattern, "\n", pathVariables.toString()); @@ -144,8 +147,9 @@ public class NestedRouteIntegrationTests extends AbstractRouterFunctionIntegrati return ServerResponse.ok().body(responseBody, String.class); } - private String matchingPattern(ServerRequest request) { - return (String) request.attributes().getOrDefault(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE, ""); + @Nullable + private PathPattern matchingPattern(ServerRequest request) { + return (PathPattern) request.attributes().get(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE); } } -- GitLab