Partially revert RequestPredicates attribute handling

This commit partially reverts 39786e4790
and c5c843696b, as the approach taken did
not take into account request predicates that query request attributes,
including path variables.

Closes gh-31732

spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RequestPredicates.java

@@ -27,7 +27,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.function.Predicate;
@@ -315,7 +314,28 @@ public abstract class RequestPredicates {
 		else {
 			return newPattern;
 		}
+	}
 
+	private static <K, V> Map<K, V> mergeMaps(Map<K, V> left, Map<K, V> right) {
+		if (left.isEmpty()) {
+			if (right.isEmpty()) {
+				return Collections.emptyMap();
+			}
+			else {
+				return right;
+			}
+		}
+		else {
+			if (right.isEmpty()) {
+				return left;
+			}
+			else {
+				Map<K, V> result = new LinkedHashMap<>(left.size() + right.size());
+				result.putAll(left);
+				result.putAll(right);
+				return result;
+			}
+		}
 	}
 
 
@@ -452,7 +472,7 @@ public abstract class RequestPredicates {
 			Result result = testInternal(request);
 			boolean value = result.value();
 			if (value) {
-				result.modify(request);
+				result.modifyAttributes(request.attributes());
 			}
 			return value;
 		}
@@ -470,12 +490,12 @@ public abstract class RequestPredicates {
 			private final boolean value;
 
 			@Nullable
-			private final Consumer<ServerRequest> modify;
+			private final Consumer<Map<String, Object>> modifyAttributes;
 
 
-			private Result(boolean value, @Nullable Consumer<ServerRequest> modify) {
+			private Result(boolean value, @Nullable Consumer<Map<String, Object>> modifyAttributes) {
 				this.value = value;
-				this.modify = modify;
+				this.modifyAttributes = modifyAttributes;
 			}
 
 
@@ -483,12 +503,12 @@ public abstract class RequestPredicates {
 				return of(value, null);
 			}
 
-			public static Result of(boolean value, @Nullable Consumer<ServerRequest> commit) {
-				if (commit == null) {
+			public static Result of(boolean value, @Nullable Consumer<Map<String, Object>> modifyAttributes) {
+				if (modifyAttributes == null) {
 					return value ? TRUE : FALSE;
 				}
 				else {
-					return new Result(value, commit);
+					return new Result(value, modifyAttributes);
 				}
 			}
 
@@ -497,11 +517,15 @@ public abstract class RequestPredicates {
 				return this.value;
 			}
 
-			public void modify(ServerRequest request) {
-				if (this.modify != null) {
-					this.modify.accept(request);
+			public void modifyAttributes(Map<String, Object> attributes) {
+				if (this.modifyAttributes != null) {
+					this.modifyAttributes.accept(attributes);
 				}
 			}
+
+			public boolean modifiesAttributes() {
+				return this.modifyAttributes != null;
+			}
 		}
 
 	}
@@ -575,29 +599,32 @@ public abstract class RequestPredicates {
 			PathPattern.PathMatchInfo info = this.pattern.matchAndExtract(pathContainer);
 			traceMatch("Pattern", this.pattern.getPatternString(), request.path(), info != null);
 			if (info != null) {
-				return Result.of(true, serverRequest -> mergeAttributes(serverRequest, info.getUriVariables()));
+				return Result.of(true, attributes -> modifyAttributes(attributes, request, info.getUriVariables()));
 			}
 			else {
 				return Result.of(false);
 			}
 		}
 
-		private void mergeAttributes(ServerRequest request, Map<String, String> variables) {
-			Map<String, Object> attributes = request.attributes();
-			Map<String, String> pathVariables = mergePathVariables(request.pathVariables(), variables);
+		private void modifyAttributes(Map<String, Object> attributes, ServerRequest request,
+				Map<String, String> variables) {
+
+			Map<String, String> pathVariables = mergeMaps(request.pathVariables(), variables);
+
 			attributes.put(RouterFunctions.URI_TEMPLATE_VARIABLES_ATTRIBUTE,
 					Collections.unmodifiableMap(pathVariables));
 
 			PathPattern pattern = mergePatterns(
 					(PathPattern) attributes.get(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE),
 					this.pattern);
+
 			attributes.put(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE, pattern);
 		}
 
 		@Override
 		public Optional<ServerRequest> nest(ServerRequest request) {
 			return Optional.ofNullable(this.pattern.matchStartOfPath(request.requestPath().pathWithinApplication()))
-					.map(info -> new SubPathServerRequestWrapper(request, info, this.pattern));
+					.map(info -> new NestedPathPatternServerRequestWrapper(request, info, this.pattern));
 		}
 
 		@Override
@@ -861,13 +888,23 @@ public abstract class RequestPredicates {
 			if (!leftResult.value()) {
 				return leftResult;
 			}
-			Result rightResult = this.rightModifying.testInternal(request);
+			// ensure that attributes (and uri variables) set in left and available in right
+			ServerRequest rightRequest;
+			if (leftResult.modifiesAttributes()) {
+				Map<String, Object> leftAttributes = new LinkedHashMap<>(2);
+				leftResult.modifyAttributes(leftAttributes);
+				rightRequest = new ExtendedAttributesServerRequestWrapper(request, leftAttributes);
+			}
+			else {
+				rightRequest = request;
+			}
+			Result rightResult = this.rightModifying.testInternal(rightRequest);
 			if (!rightResult.value()) {
 				return rightResult;
 			}
-			return Result.of(true, serverRequest -> {
-				leftResult.modify(serverRequest);
-				rightResult.modify(serverRequest);
+			return Result.of(true, attributes -> {
+				leftResult.modifyAttributes(attributes);
+				rightResult.modifyAttributes(attributes);
 			});
 		}
 
@@ -923,7 +960,7 @@ public abstract class RequestPredicates {
 		@Override
 		protected Result testInternal(ServerRequest request) {
 			Result result = this.delegateModifying.testInternal(request);
-			return Result.of(!result.value(), result::modify);
+			return Result.of(!result.value(), result::modifyAttributes);
 		}
 
 		@Override
@@ -1020,188 +1057,210 @@ public abstract class RequestPredicates {
 	}
 
 
-	private static class SubPathServerRequestWrapper implements ServerRequest {
-
-		private final ServerRequest request;
-
-		private final RequestPath requestPath;
-
-		private final Map<String, Object> attributes;
-
-		public SubPathServerRequestWrapper(ServerRequest request,
-				PathPattern.PathRemainingMatchInfo info, PathPattern pattern) {
-			this.request = request;
-			this.requestPath = requestPath(request.requestPath(), info);
-			this.attributes = mergeAttributes(request, info.getUriVariables(), pattern);
-		}
 
-		private static RequestPath requestPath(RequestPath original, PathPattern.PathRemainingMatchInfo info) {
-			StringBuilder contextPath = new StringBuilder(original.contextPath().value());
-			contextPath.append(info.getPathMatched().value());
-			int length = contextPath.length();
-			if (length > 0 && contextPath.charAt(length - 1) == '/') {
-				contextPath.setLength(length - 1);
-			}
-			return original.modifyContextPath(contextPath.toString());
-		}
+	private abstract static class DelegatingServerRequest implements ServerRequest {
 
-		private static Map<String, Object> mergeAttributes(ServerRequest request,
-		Map<String, String> pathVariables, PathPattern pattern) {
-			Map<String, Object> result = new ConcurrentHashMap<>(request.attributes());
+		private final ServerRequest delegate;
 
-			result.put(RouterFunctions.URI_TEMPLATE_VARIABLES_ATTRIBUTE,
-					mergePathVariables(request.pathVariables(), pathVariables));
 
-			pattern = mergePatterns(
-					(PathPattern) request.attributes().get(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE),
-					pattern);
-			result.put(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE, pattern);
-			return result;
+		protected DelegatingServerRequest(ServerRequest delegate) {
+			Assert.notNull(delegate, "Delegate must not be null");
+			this.delegate = delegate;
 		}
 
 		@Override
 		public HttpMethod method() {
-			return this.request.method();
+			return this.delegate.method();
 		}
 
 		@Override
 		@Deprecated
 		public String methodName() {
-			return this.request.methodName();
+			return this.delegate.methodName();
 		}
 
 		@Override
 		public URI uri() {
-			return this.request.uri();
+			return this.delegate.uri();
 		}
 
 		@Override
 		public UriBuilder uriBuilder() {
-			return this.request.uriBuilder();
-		}
-
-		@Override
-		public RequestPath requestPath() {
-			return this.requestPath;
+			return this.delegate.uriBuilder();
 		}
 
 		@Override
 		public Headers headers() {
-			return this.request.headers();
+			return this.delegate.headers();
 		}
 
 		@Override
 		public MultiValueMap<String, HttpCookie> cookies() {
-			return this.request.cookies();
+			return this.delegate.cookies();
 		}
 
 		@Override
 		public Optional<InetSocketAddress> remoteAddress() {
-			return this.request.remoteAddress();
+			return this.delegate.remoteAddress();
 		}
 
 		@Override
 		public Optional<InetSocketAddress> localAddress() {
-			return this.request.localAddress();
+			return this.delegate.localAddress();
 		}
 
 		@Override
 		public List<HttpMessageReader<?>> messageReaders() {
-			return this.request.messageReaders();
+			return this.delegate.messageReaders();
 		}
 
 		@Override
 		public <T> T body(BodyExtractor<T, ? super ServerHttpRequest> extractor) {
-			return this.request.body(extractor);
+			return this.delegate.body(extractor);
 		}
 
 		@Override
 		public <T> T body(BodyExtractor<T, ? super ServerHttpRequest> extractor, Map<String, Object> hints) {
-			return this.request.body(extractor, hints);
+			return this.delegate.body(extractor, hints);
 		}
 
 		@Override
 		public <T> Mono<T> bodyToMono(Class<? extends T> elementClass) {
-			return this.request.bodyToMono(elementClass);
+			return this.delegate.bodyToMono(elementClass);
 		}
 
 		@Override
 		public <T> Mono<T> bodyToMono(ParameterizedTypeReference<T> typeReference) {
-			return this.request.bodyToMono(typeReference);
+			return this.delegate.bodyToMono(typeReference);
 		}
 
 		@Override
 		public <T> Flux<T> bodyToFlux(Class<? extends T> elementClass) {
-			return this.request.bodyToFlux(elementClass);
+			return this.delegate.bodyToFlux(elementClass);
 		}
 
 		@Override
 		public <T> Flux<T> bodyToFlux(ParameterizedTypeReference<T> typeReference) {
-			return this.request.bodyToFlux(typeReference);
-		}
-
-		@Override
-		public <T> Mono<T> bind(Class<T> bindType) {
-			return this.request.bind(bindType);
+			return this.delegate.bodyToFlux(typeReference);
 		}
 
 		@Override
 		public <T> Mono<T> bind(Class<T> bindType, Consumer<WebDataBinder> dataBinderCustomizer) {
-			return this.request.bind(bindType, dataBinderCustomizer);
+			return this.delegate.bind(bindType, dataBinderCustomizer);
 		}
 
 		@Override
 		public Map<String, Object> attributes() {
-			return this.attributes;
-		}
-
-		@Override
-		public Optional<String> queryParam(String name) {
-			return this.request.queryParam(name);
+			return this.delegate.attributes();
 		}
 
 		@Override
 		public MultiValueMap<String, String> queryParams() {
-			return this.request.queryParams();
+			return this.delegate.queryParams();
 		}
 
 		@Override
-		@SuppressWarnings("unchecked")
 		public Map<String, String> pathVariables() {
-			return (Map<String, String>) this.attributes.getOrDefault(
-					RouterFunctions.URI_TEMPLATE_VARIABLES_ATTRIBUTE, Collections.emptyMap());
-
+			return this.delegate.pathVariables();
 		}
 
 		@Override
 		public Mono<WebSession> session() {
-			return this.request.session();
+			return this.delegate.session();
 		}
 
 		@Override
 		public Mono<? extends Principal> principal() {
-			return this.request.principal();
+			return this.delegate.principal();
 		}
 
 		@Override
 		public Mono<MultiValueMap<String, String>> formData() {
-			return this.request.formData();
+			return this.delegate.formData();
 		}
 
 		@Override
 		public Mono<MultiValueMap<String, Part>> multipartData() {
-			return this.request.multipartData();
+			return this.delegate.multipartData();
 		}
 
 		@Override
 		public ServerWebExchange exchange() {
-			return this.request.exchange();
+			return this.delegate.exchange();
 		}
 
 		@Override
 		public String toString() {
-			return method() + " " + path();
+			return this.delegate.toString();
 		}
 	}
 
+
+	private static class ExtendedAttributesServerRequestWrapper extends DelegatingServerRequest {
+
+		private final Map<String, Object> attributes;
+
+
+		public ExtendedAttributesServerRequestWrapper(ServerRequest delegate, Map<String, Object> newAttributes) {
+			super(delegate);
+			Assert.notNull(newAttributes, "NewAttributes must not be null");
+			this.attributes = mergeMaps(delegate.attributes(), newAttributes);
+		}
+
+
+		@Override
+		public Map<String, Object> attributes() {
+			return this.attributes;
+		}
+
+		@Override
+		@SuppressWarnings("unchecked")
+		public Map<String, String> pathVariables() {
+			return (Map<String, String>) this.attributes.getOrDefault(
+					RouterFunctions.URI_TEMPLATE_VARIABLES_ATTRIBUTE, Collections.emptyMap());
+		}
+	}
+
+
+	private static class NestedPathPatternServerRequestWrapper extends ExtendedAttributesServerRequestWrapper {
+
+		private final RequestPath requestPath;
+
+
+		public NestedPathPatternServerRequestWrapper(ServerRequest request,
+				PathPattern.PathRemainingMatchInfo info, PathPattern pattern) {
+			super(request, mergeAttributes(request, info.getUriVariables(), pattern));
+			this.requestPath = requestPath(request.requestPath(), info);
+		}
+
+		private static Map<String, Object> mergeAttributes(ServerRequest request, Map<String, String> newPathVariables,
+				PathPattern newPathPattern) {
+
+
+			Map<String, String> oldPathVariables = request.pathVariables();
+			PathPattern oldPathPattern = (PathPattern) request.attribute(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE)
+					.orElse(null);
+
+			Map<String, Object> result = new LinkedHashMap<>(2);
+			result.put(RouterFunctions.URI_TEMPLATE_VARIABLES_ATTRIBUTE, mergeMaps(oldPathVariables, newPathVariables));
+			result.put(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE, mergePatterns(oldPathPattern, newPathPattern));
+			return result;
+		}
+
+		private static RequestPath requestPath(RequestPath original, PathPattern.PathRemainingMatchInfo info) {
+			StringBuilder contextPath = new StringBuilder(original.contextPath().value());
+			contextPath.append(info.getPathMatched().value());
+			int length = contextPath.length();
+			if (length > 0 && contextPath.charAt(length - 1) == '/') {
+				contextPath.setLength(length - 1);
+			}
+			return original.modifyContextPath(contextPath.toString());
+		}
+
+
+		@Override
+		public RequestPath requestPath() {
+			return this.requestPath;
+		}
+	}
 }

spring-webflux/src/test/java/org/springframework/web/reactive/function/server/NestedRouteIntegrationTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2023 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.
@@ -90,7 +90,11 @@ class NestedRouteIntegrationTests extends AbstractRouterFunctionIntegrationTests
 				restTemplate.getForEntity("http://localhost:" + port + "/1/2/3", String.class);
 
 		assertThat(result.getStatusCode()).isEqualTo(HttpStatus.OK);
-		assertThat(result.getBody()).isEqualTo("/{foo}/{bar}/{baz}\n{foo=1, bar=2, baz=3}");
+		String body = result.getBody();
+		assertThat(body).startsWith("/{foo}/{bar}/{baz}");
+		assertThat(body).contains("foo=1");
+		assertThat(body).contains("bar=2");
+		assertThat(body).contains("baz=3");
 	}
 
 	// SPR-16868

spring-webflux/src/test/java/org/springframework/web/reactive/function/server/RequestPredicateAttributesTests.java

@@ -200,7 +200,7 @@ public class RequestPredicateAttributesTests {
 
 		@Override
 		protected Result testInternal(ServerRequest request) {
-			return Result.of(this.result, serverRequest -> serverRequest.attributes().put(this.key, this.value));
+			return Result.of(this.result, attributes -> attributes.put(this.key, this.value));
 		}
 	}
 

spring-webmvc/src/main/java/org/springframework/web/servlet/function/RequestPredicates.java

@@ -29,7 +29,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.function.Predicate;
@@ -294,19 +293,6 @@ public abstract class RequestPredicates {
 		}
 	}
 
-	private static Map<String, String> mergePathVariables(Map<String, String> oldVariables,
-			Map<String, String> newVariables) {
-
-		if (!newVariables.isEmpty()) {
-			Map<String, String> mergedVariables = new LinkedHashMap<>(oldVariables);
-			mergedVariables.putAll(newVariables);
-			return mergedVariables;
-		}
-		else {
-			return oldVariables;
-		}
-	}
-
 	private static PathPattern mergePatterns(@Nullable PathPattern oldPattern, PathPattern newPattern) {
 		if (oldPattern != null) {
 			return oldPattern.combine(newPattern);
@@ -314,7 +300,28 @@ public abstract class RequestPredicates {
 		else {
 			return newPattern;
 		}
+	}
 
+	private static <K, V> Map<K, V> mergeMaps(Map<K, V> left, Map<K, V> right) {
+		if (left.isEmpty()) {
+			if (right.isEmpty()) {
+				return Collections.emptyMap();
+			}
+			else {
+				return right;
+			}
+		}
+		else {
+			if (right.isEmpty()) {
+				return left;
+			}
+			else {
+				Map<K, V> result = new LinkedHashMap<>(left.size() + right.size());
+				result.putAll(left);
+				result.putAll(right);
+				return result;
+			}
+		}
 	}
 
 
@@ -451,7 +458,7 @@ public abstract class RequestPredicates {
 			Result result = testInternal(request);
 			boolean value = result.value();
 			if (value) {
-				result.modify(request);
+				result.modifyAttributes(request.attributes());
 			}
 			return value;
 		}
@@ -469,12 +476,12 @@ public abstract class RequestPredicates {
 			private final boolean value;
 
 			@Nullable
-			private final Consumer<ServerRequest> modify;
+			private final Consumer<Map<String, Object>> modifyAttributes;
 
 
-			private Result(boolean value, @Nullable Consumer<ServerRequest> modify) {
+			private Result(boolean value, @Nullable Consumer<Map<String, Object>> modifyAttributes) {
 				this.value = value;
-				this.modify = modify;
+				this.modifyAttributes = modifyAttributes;
 			}
 
 
@@ -482,12 +489,12 @@ public abstract class RequestPredicates {
 				return of(value, null);
 			}
 
-			public static Result of(boolean value, @Nullable Consumer<ServerRequest> commit) {
-				if (commit == null) {
+			public static Result of(boolean value, @Nullable Consumer<Map<String, Object>> modifyAttributes) {
+				if (modifyAttributes == null) {
 					return value ? TRUE : FALSE;
 				}
 				else {
-					return new Result(value, commit);
+					return new Result(value, modifyAttributes);
 				}
 			}
 
@@ -496,11 +503,15 @@ public abstract class RequestPredicates {
 				return this.value;
 			}
 
-			public void modify(ServerRequest request) {
-				if (this.modify != null) {
-					this.modify.accept(request);
+			public void modifyAttributes(Map<String, Object> attributes) {
+				if (this.modifyAttributes != null) {
+					this.modifyAttributes.accept(attributes);
 				}
 			}
+
+			public boolean modifiesAttributes() {
+				return this.modifyAttributes != null;
+			}
 		}
 
 	}
@@ -574,29 +585,32 @@ public abstract class RequestPredicates {
 			PathPattern.PathMatchInfo info = this.pattern.matchAndExtract(pathContainer);
 			traceMatch("Pattern", this.pattern.getPatternString(), request.path(), info != null);
 			if (info != null) {
-				return Result.of(true, serverRequest -> mergeAttributes(serverRequest, info.getUriVariables()));
+				return Result.of(true, attributes -> modifyAttributes(attributes, request, info.getUriVariables()));
 			}
 			else {
 				return Result.of(false);
 			}
 		}
 
-		private void mergeAttributes(ServerRequest request, Map<String, String> variables) {
-			Map<String, Object> attributes = request.attributes();
-			Map<String, String> pathVariables = mergePathVariables(request.pathVariables(), variables);
+		private void modifyAttributes(Map<String, Object> attributes, ServerRequest request,
+				Map<String, String> variables) {
+
+			Map<String, String> pathVariables = mergeMaps(request.pathVariables(), variables);
+
 			attributes.put(RouterFunctions.URI_TEMPLATE_VARIABLES_ATTRIBUTE,
 					Collections.unmodifiableMap(pathVariables));
 
 			PathPattern pattern = mergePatterns(
 					(PathPattern) attributes.get(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE),
 					this.pattern);
+
 			attributes.put(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE, pattern);
 		}
 
 		@Override
 		public Optional<ServerRequest> nest(ServerRequest request) {
 			return Optional.ofNullable(this.pattern.matchStartOfPath(request.requestPath().pathWithinApplication()))
-					.map(info -> new SubPathServerRequestWrapper(request, info, this.pattern));
+					.map(info -> new NestedPathPatternServerRequestWrapper(request, info, this.pattern));
 		}
 
 		@Override
@@ -860,13 +874,23 @@ public abstract class RequestPredicates {
 			if (!leftResult.value()) {
 				return leftResult;
 			}
-			Result rightResult = this.rightModifying.testInternal(request);
+			// ensure that attributes (and uri variables) set in left and available in right
+			ServerRequest rightRequest;
+			if (leftResult.modifiesAttributes()) {
+				Map<String, Object> leftAttributes = new LinkedHashMap<>(2);
+				leftResult.modifyAttributes(leftAttributes);
+				rightRequest = new ExtendedAttributesServerRequestWrapper(request, leftAttributes);
+			}
+			else {
+				rightRequest = request;
+			}
+			Result rightResult = this.rightModifying.testInternal(rightRequest);
 			if (!rightResult.value()) {
 				return rightResult;
 			}
-			return Result.of(true, serverRequest -> {
-				leftResult.modify(serverRequest);
-				rightResult.modify(serverRequest);
+			return Result.of(true, attributes -> {
+				leftResult.modifyAttributes(attributes);
+				rightResult.modifyAttributes(attributes);
 			});
 		}
 
@@ -922,7 +946,7 @@ public abstract class RequestPredicates {
 		@Override
 		protected Result testInternal(ServerRequest request) {
 			Result result = this.delegateModifying.testInternal(request);
-			return Result.of(!result.value(), result::modify);
+			return Result.of(!result.value(), result::modifyAttributes);
 		}
 
 		@Override
@@ -1019,178 +1043,210 @@ public abstract class RequestPredicates {
 	}
 
 
-	private static class SubPathServerRequestWrapper implements ServerRequest {
-
-		private final ServerRequest request;
-
-		private final RequestPath requestPath;
-
-		private final Map<String, Object> attributes;
-
-		public SubPathServerRequestWrapper(ServerRequest request,
-				PathPattern.PathRemainingMatchInfo info, PathPattern pattern) {
-			this.request = request;
-			this.requestPath = requestPath(request.requestPath(), info);
-			this.attributes = mergeAttributes(request, info.getUriVariables(), pattern);
-		}
 
-		private static RequestPath requestPath(RequestPath original, PathPattern.PathRemainingMatchInfo info) {
-			StringBuilder contextPath = new StringBuilder(original.contextPath().value());
-			contextPath.append(info.getPathMatched().value());
-			int length = contextPath.length();
-			if (length > 0 && contextPath.charAt(length - 1) == '/') {
-				contextPath.setLength(length - 1);
-			}
-			return original.modifyContextPath(contextPath.toString());
-		}
+	private abstract static class DelegatingServerRequest implements ServerRequest {
 
-		private static Map<String, Object> mergeAttributes(ServerRequest request,
-		Map<String, String> pathVariables, PathPattern pattern) {
-			Map<String, Object> result = new ConcurrentHashMap<>(request.attributes());
+		private final ServerRequest delegate;
 
-			result.put(RouterFunctions.URI_TEMPLATE_VARIABLES_ATTRIBUTE,
-					mergePathVariables(request.pathVariables(), pathVariables));
 
-			pattern = mergePatterns(
-					(PathPattern) request.attributes().get(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE),
-					pattern);
-			result.put(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE, pattern);
-			return result;
+		protected DelegatingServerRequest(ServerRequest delegate) {
+			Assert.notNull(delegate, "Delegate must not be null");
+			this.delegate = delegate;
 		}
 
 		@Override
 		public HttpMethod method() {
-			return this.request.method();
+			return this.delegate.method();
 		}
 
 		@Override
 		@Deprecated
 		public String methodName() {
-			return this.request.methodName();
+			return this.delegate.methodName();
 		}
 
 		@Override
 		public URI uri() {
-			return this.request.uri();
+			return this.delegate.uri();
 		}
 
 		@Override
 		public UriBuilder uriBuilder() {
-			return this.request.uriBuilder();
+			return this.delegate.uriBuilder();
+		}
+
+		@Override
+		public String path() {
+			return this.delegate.path();
 		}
 
 		@Override
 		public RequestPath requestPath() {
-			return this.requestPath;
+			return this.delegate.requestPath();
 		}
 
 		@Override
 		public Headers headers() {
-			return this.request.headers();
+			return this.delegate.headers();
 		}
 
 		@Override
 		public MultiValueMap<String, Cookie> cookies() {
-			return this.request.cookies();
+			return this.delegate.cookies();
 		}
 
 		@Override
 		public Optional<InetSocketAddress> remoteAddress() {
-			return this.request.remoteAddress();
+			return this.delegate.remoteAddress();
 		}
 
 		@Override
 		public List<HttpMessageConverter<?>> messageConverters() {
-			return this.request.messageConverters();
+			return this.delegate.messageConverters();
 		}
 
 		@Override
 		public <T> T body(Class<T> bodyType) throws ServletException, IOException {
-			return this.request.body(bodyType);
+			return this.delegate.body(bodyType);
 		}
 
 		@Override
-		public <T> T body(ParameterizedTypeReference<T> bodyType)
-				throws ServletException, IOException {
-			return this.request.body(bodyType);
+		public <T> T body(ParameterizedTypeReference<T> bodyType) throws ServletException, IOException {
+			return this.delegate.body(bodyType);
 		}
 
 		@Override
 		public <T> T bind(Class<T> bindType) throws BindException {
-			return this.request.bind(bindType);
+			return this.delegate.bind(bindType);
 		}
 
 		@Override
 		public <T> T bind(Class<T> bindType, Consumer<WebDataBinder> dataBinderCustomizer) throws BindException {
-			return this.request.bind(bindType, dataBinderCustomizer);
-		}
-
-		@Override
-		public Optional<Object> attribute(String name) {
-			return this.request.attribute(name);
+			return this.delegate.bind(bindType, dataBinderCustomizer);
 		}
 
 		@Override
 		public Map<String, Object> attributes() {
-			return this.attributes;
-		}
-
-		@Override
-		public Optional<String> param(String name) {
-			return this.request.param(name);
+			return this.delegate.attributes();
 		}
 
 		@Override
 		public MultiValueMap<String, String> params() {
-			return this.request.params();
+			return this.delegate.params();
 		}
 
 		@Override
 		public MultiValueMap<String, Part> multipartData() throws IOException, ServletException {
-			return this.request.multipartData();
+			return this.delegate.multipartData();
 		}
 
 		@Override
-		@SuppressWarnings("unchecked")
 		public Map<String, String> pathVariables() {
-			return (Map<String, String>) this.attributes.getOrDefault(
-					RouterFunctions.URI_TEMPLATE_VARIABLES_ATTRIBUTE, Collections.emptyMap());
+			return this.delegate.pathVariables();
 		}
 
 		@Override
 		public HttpSession session() {
-			return this.request.session();
+			return this.delegate.session();
 		}
 
 		@Override
 		public Optional<Principal> principal() {
-			return this.request.principal();
+			return this.delegate.principal();
 		}
 
 		@Override
 		public HttpServletRequest servletRequest() {
-			return this.request.servletRequest();
+			return this.delegate.servletRequest();
 		}
 
 		@Override
 		public Optional<ServerResponse> checkNotModified(Instant lastModified) {
-			return this.request.checkNotModified(lastModified);
+			return this.delegate.checkNotModified(lastModified);
 		}
 
 		@Override
 		public Optional<ServerResponse> checkNotModified(String etag) {
-			return this.request.checkNotModified(etag);
+			return this.delegate.checkNotModified(etag);
 		}
 
 		@Override
 		public Optional<ServerResponse> checkNotModified(Instant lastModified, String etag) {
-			return this.request.checkNotModified(lastModified, etag);
+			return this.delegate.checkNotModified(lastModified, etag);
 		}
 
 		@Override
 		public String toString() {
-			return method() + " " + path();
+			return this.delegate.toString();
+		}
+	}
+
+
+	private static class ExtendedAttributesServerRequestWrapper extends DelegatingServerRequest {
+
+		private final Map<String, Object> attributes;
+
+
+		public ExtendedAttributesServerRequestWrapper(ServerRequest delegate, Map<String, Object> newAttributes) {
+			super(delegate);
+			Assert.notNull(newAttributes, "NewAttributes must not be null");
+			this.attributes = mergeMaps(delegate.attributes(), newAttributes);
+		}
+
+
+		@Override
+		public Map<String, Object> attributes() {
+			return this.attributes;
+		}
+
+		@Override
+		@SuppressWarnings("unchecked")
+		public Map<String, String> pathVariables() {
+			return (Map<String, String>) this.attributes.getOrDefault(
+					RouterFunctions.URI_TEMPLATE_VARIABLES_ATTRIBUTE, Collections.emptyMap());
 		}
 	}
 
+
+	private static class NestedPathPatternServerRequestWrapper extends ExtendedAttributesServerRequestWrapper {
+
+		private final RequestPath requestPath;
+
+
+		public NestedPathPatternServerRequestWrapper(ServerRequest request,
+				PathPattern.PathRemainingMatchInfo info, PathPattern pattern) {
+			super(request, mergeAttributes(request, info.getUriVariables(), pattern));
+			this.requestPath = requestPath(request.requestPath(), info);
+		}
+
+		private static Map<String, Object> mergeAttributes(ServerRequest request, Map<String, String> newPathVariables,
+				PathPattern newPathPattern) {
+
+
+			Map<String, String> oldPathVariables = request.pathVariables();
+			PathPattern oldPathPattern = (PathPattern) request.attribute(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE)
+					.orElse(null);
+
+			Map<String, Object> result = new LinkedHashMap<>(2);
+			result.put(RouterFunctions.URI_TEMPLATE_VARIABLES_ATTRIBUTE, mergeMaps(oldPathVariables, newPathVariables));
+			result.put(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE, mergePatterns(oldPathPattern, newPathPattern));
+			return result;
+		}
+
+		private static RequestPath requestPath(RequestPath original, PathPattern.PathRemainingMatchInfo info) {
+			StringBuilder contextPath = new StringBuilder(original.contextPath().value());
+			contextPath.append(info.getPathMatched().value());
+			int length = contextPath.length();
+			if (length > 0 && contextPath.charAt(length - 1) == '/') {
+				contextPath.setLength(length - 1);
+			}
+			return original.modifyContextPath(contextPath.toString());
+		}
+
+
+		@Override
+		public RequestPath requestPath() {
+			return this.requestPath;
+		}
+	}
 }

spring-webmvc/src/test/java/org/springframework/web/servlet/function/RouterFunctionsTests.java

@@ -114,4 +114,18 @@ public class RouterFunctionsTests {
 		assertThat(resultHandlerFunction).contains(handlerFunction);
 	}
 
+	@Test
+	public void composedPathVariable() {
+		HandlerFunction<ServerResponse> handlerFunction = request -> ServerResponse.ok().build();
+		RequestPredicate requestPredicate = RequestPredicates.path("/{foo}").and(
+				request -> request.pathVariable("foo").equals("bar"));
+		RouterFunction<ServerResponse> routerFunction = RouterFunctions.route(requestPredicate, handlerFunction);
+
+		MockHttpServletRequest servletRequest = new MockHttpServletRequest("GET", "/bar");
+		ServerRequest request = new DefaultServerRequest(servletRequest, Collections.emptyList());
+		Optional<HandlerFunction<ServerResponse>> resultHandlerFunction = routerFunction.route(request);
+		assertThat(resultHandlerFunction).isPresent();
+		assertThat(resultHandlerFunction).contains(handlerFunction);
+	}
+
 }