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

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("/")) {
+			return oldPattern.combine(newPattern);
-				oldPattern = oldPattern.substring(0, oldPattern.length() - 1);
-			}
-			return oldPattern + 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", this.pattern.getPatternString(), request.path(), info != null);
-			traceMatch("Pattern", patternString, 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<String, String> variables,
-				String pattern) {
+				PathPattern pattern) {
 			Map<String, String> 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<ServerRequest> 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<String, Object> 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<String, Object> mergeAttributes(ServerRequest request,
-		Map<String, String> pathVariables, String pattern) {
+		Map<String, String> pathVariables, PathPattern pattern) {
 			Map<String, Object> 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;

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";

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 =
+		PathPattern matchingPattern =
-				(String) attributes.get(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE);
+				(PathPattern) attributes.get(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE);
 		if (matchingPattern != null) {
 			attributes.put(BEST_MATCHING_PATTERN_ATTRIBUTE, matchingPattern);
 		}

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.BodyInserters.fromPublisher;
-import static org.springframework.web.reactive.function.server.RouterFunctions.*;
+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<String> 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<EntityResponse<Person>> 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<ServerResponse> 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<ServerResponse> attributes(ServerRequest request) {
+			assertTrue(request.attributes().containsKey(RouterFunctions.REQUEST_ATTRIBUTE));
+			assertTrue(request.attributes()
+					.containsKey(HandlerMapping.BEST_MATCHING_HANDLER_ATTRIBUTE));
+
+			Map<String, String> pathVariables =
+					(Map<String, String>) request.attributes().get(RouterFunctions.URI_TEMPLATE_VARIABLES_ATTRIBUTE);
+			assertNotNull(pathVariables);
+			assertEquals(1, pathVariables.size());
+			assertEquals("bar", pathVariables.get("foo"));
+
+			pathVariables =
+					(Map<String, String>) 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

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<ServerResponse> 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<String> 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) {
+		@Nullable
-			return (String) request.attributes().getOrDefault(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE, "");
+		private PathPattern matchingPattern(ServerRequest request) {
+			return (PathPattern) request.attributes().get(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE);
 		}
 
 	}