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 e63ca971f57..8e5028aa75a 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 @@ -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 Map mergeMaps(Map left, Map right) { + if (left.isEmpty()) { + if (right.isEmpty()) { + return Collections.emptyMap(); + } + else { + return right; + } + } + else { + if (right.isEmpty()) { + return left; + } + else { + Map 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 modify; + private final Consumer> modifyAttributes; - private Result(boolean value, @Nullable Consumer modify) { + private Result(boolean value, @Nullable Consumer> 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 commit) { - if (commit == null) { + public static Result of(boolean value, @Nullable Consumer> 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 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 variables) { - Map attributes = request.attributes(); - Map pathVariables = mergePathVariables(request.pathVariables(), variables); + private void modifyAttributes(Map attributes, ServerRequest request, + Map variables) { + + Map 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 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 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,19 +1057,194 @@ public abstract class RequestPredicates { } - private static class SubPathServerRequestWrapper implements ServerRequest { - private final ServerRequest request; + private abstract static class DelegatingServerRequest implements ServerRequest { - private final RequestPath requestPath; + private final ServerRequest delegate; + + + protected DelegatingServerRequest(ServerRequest delegate) { + Assert.notNull(delegate, "Delegate must not be null"); + this.delegate = delegate; + } + + @Override + public HttpMethod method() { + return this.delegate.method(); + } + + @Override + @Deprecated + public String methodName() { + return this.delegate.methodName(); + } + + @Override + public URI uri() { + return this.delegate.uri(); + } + + @Override + public UriBuilder uriBuilder() { + return this.delegate.uriBuilder(); + } + + @Override + public Headers headers() { + return this.delegate.headers(); + } + + @Override + public MultiValueMap cookies() { + return this.delegate.cookies(); + } + + @Override + public Optional remoteAddress() { + return this.delegate.remoteAddress(); + } + + @Override + public Optional localAddress() { + return this.delegate.localAddress(); + } + + @Override + public List> messageReaders() { + return this.delegate.messageReaders(); + } + + @Override + public T body(BodyExtractor extractor) { + return this.delegate.body(extractor); + } + + @Override + public T body(BodyExtractor extractor, Map hints) { + return this.delegate.body(extractor, hints); + } + + @Override + public Mono bodyToMono(Class elementClass) { + return this.delegate.bodyToMono(elementClass); + } + + @Override + public Mono bodyToMono(ParameterizedTypeReference typeReference) { + return this.delegate.bodyToMono(typeReference); + } + + @Override + public Flux bodyToFlux(Class elementClass) { + return this.delegate.bodyToFlux(elementClass); + } + + @Override + public Flux bodyToFlux(ParameterizedTypeReference typeReference) { + return this.delegate.bodyToFlux(typeReference); + } + + @Override + public Mono bind(Class bindType, Consumer dataBinderCustomizer) { + return this.delegate.bind(bindType, dataBinderCustomizer); + } + + @Override + public Map attributes() { + return this.delegate.attributes(); + } + + @Override + public MultiValueMap queryParams() { + return this.delegate.queryParams(); + } + + @Override + public Map pathVariables() { + return this.delegate.pathVariables(); + } + + @Override + public Mono session() { + return this.delegate.session(); + } + + @Override + public Mono principal() { + return this.delegate.principal(); + } + + @Override + public Mono> formData() { + return this.delegate.formData(); + } + + @Override + public Mono> multipartData() { + return this.delegate.multipartData(); + } + + @Override + public ServerWebExchange exchange() { + return this.delegate.exchange(); + } + + @Override + public String toString() { + return this.delegate.toString(); + } + } + + + private static class ExtendedAttributesServerRequestWrapper extends DelegatingServerRequest { private final Map attributes; - public SubPathServerRequestWrapper(ServerRequest request, + + public ExtendedAttributesServerRequestWrapper(ServerRequest delegate, Map newAttributes) { + super(delegate); + Assert.notNull(newAttributes, "NewAttributes must not be null"); + this.attributes = mergeMaps(delegate.attributes(), newAttributes); + } + + + @Override + public Map attributes() { + return this.attributes; + } + + @Override + @SuppressWarnings("unchecked") + public Map pathVariables() { + return (Map) 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) { - this.request = request; + super(request, mergeAttributes(request, info.getUriVariables(), pattern)); this.requestPath = requestPath(request.requestPath(), info); - this.attributes = mergeAttributes(request, info.getUriVariables(), pattern); + } + + private static Map mergeAttributes(ServerRequest request, Map newPathVariables, + PathPattern newPathPattern) { + + + Map oldPathVariables = request.pathVariables(); + PathPattern oldPathPattern = (PathPattern) request.attribute(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE) + .orElse(null); + + Map 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) { @@ -1045,163 +1257,10 @@ public abstract class RequestPredicates { return original.modifyContextPath(contextPath.toString()); } - private static Map mergeAttributes(ServerRequest request, - Map pathVariables, PathPattern pattern) { - Map result = new ConcurrentHashMap<>(request.attributes()); - - 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; - } - - @Override - public HttpMethod method() { - return this.request.method(); - } - - @Override - @Deprecated - public String methodName() { - return this.request.methodName(); - } - - @Override - public URI uri() { - return this.request.uri(); - } - - @Override - public UriBuilder uriBuilder() { - return this.request.uriBuilder(); - } @Override public RequestPath requestPath() { return this.requestPath; } - - @Override - public Headers headers() { - return this.request.headers(); - } - - @Override - public MultiValueMap cookies() { - return this.request.cookies(); - } - - @Override - public Optional remoteAddress() { - return this.request.remoteAddress(); - } - - @Override - public Optional localAddress() { - return this.request.localAddress(); - } - - @Override - public List> messageReaders() { - return this.request.messageReaders(); - } - - @Override - public T body(BodyExtractor extractor) { - return this.request.body(extractor); - } - - @Override - public T body(BodyExtractor extractor, Map hints) { - return this.request.body(extractor, hints); - } - - @Override - public Mono bodyToMono(Class elementClass) { - return this.request.bodyToMono(elementClass); - } - - @Override - public Mono bodyToMono(ParameterizedTypeReference typeReference) { - return this.request.bodyToMono(typeReference); - } - - @Override - public Flux bodyToFlux(Class elementClass) { - return this.request.bodyToFlux(elementClass); - } - - @Override - public Flux bodyToFlux(ParameterizedTypeReference typeReference) { - return this.request.bodyToFlux(typeReference); - } - - @Override - public Mono bind(Class bindType) { - return this.request.bind(bindType); - } - - @Override - public Mono bind(Class bindType, Consumer dataBinderCustomizer) { - return this.request.bind(bindType, dataBinderCustomizer); - } - - @Override - public Map attributes() { - return this.attributes; - } - - @Override - public Optional queryParam(String name) { - return this.request.queryParam(name); - } - - @Override - public MultiValueMap queryParams() { - return this.request.queryParams(); - } - - @Override - @SuppressWarnings("unchecked") - public Map pathVariables() { - return (Map) this.attributes.getOrDefault( - RouterFunctions.URI_TEMPLATE_VARIABLES_ATTRIBUTE, Collections.emptyMap()); - - } - - @Override - public Mono session() { - return this.request.session(); - } - - @Override - public Mono principal() { - return this.request.principal(); - } - - @Override - public Mono> formData() { - return this.request.formData(); - } - - @Override - public Mono> multipartData() { - return this.request.multipartData(); - } - - @Override - public ServerWebExchange exchange() { - return this.request.exchange(); - } - - @Override - public String toString() { - return method() + " " + path(); - } } - } 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 bab05a7a128..6d9d35cee97 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 @@ -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 diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/RequestPredicateAttributesTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/RequestPredicateAttributesTests.java index 8e46e1c7647..b48ca03af77 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/RequestPredicateAttributesTests.java +++ b/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)); } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/RequestPredicates.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/RequestPredicates.java index 3f98479f75c..8cca6f7d8a2 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/RequestPredicates.java +++ b/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 mergePathVariables(Map oldVariables, - Map newVariables) { - - if (!newVariables.isEmpty()) { - Map 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 Map mergeMaps(Map left, Map right) { + if (left.isEmpty()) { + if (right.isEmpty()) { + return Collections.emptyMap(); + } + else { + return right; + } + } + else { + if (right.isEmpty()) { + return left; + } + else { + Map 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 modify; + private final Consumer> modifyAttributes; - private Result(boolean value, @Nullable Consumer modify) { + private Result(boolean value, @Nullable Consumer> 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 commit) { - if (commit == null) { + public static Result of(boolean value, @Nullable Consumer> 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 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 variables) { - Map attributes = request.attributes(); - Map pathVariables = mergePathVariables(request.pathVariables(), variables); + private void modifyAttributes(Map attributes, ServerRequest request, + Map variables) { + + Map 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 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 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,19 +1043,194 @@ public abstract class RequestPredicates { } - private static class SubPathServerRequestWrapper implements ServerRequest { - private final ServerRequest request; + private abstract static class DelegatingServerRequest implements ServerRequest { - private final RequestPath requestPath; + private final ServerRequest delegate; + + + protected DelegatingServerRequest(ServerRequest delegate) { + Assert.notNull(delegate, "Delegate must not be null"); + this.delegate = delegate; + } + + @Override + public HttpMethod method() { + return this.delegate.method(); + } + + @Override + @Deprecated + public String methodName() { + return this.delegate.methodName(); + } + + @Override + public URI uri() { + return this.delegate.uri(); + } + + @Override + public UriBuilder uriBuilder() { + return this.delegate.uriBuilder(); + } + + @Override + public String path() { + return this.delegate.path(); + } + + @Override + public RequestPath requestPath() { + return this.delegate.requestPath(); + } + + @Override + public Headers headers() { + return this.delegate.headers(); + } + + @Override + public MultiValueMap cookies() { + return this.delegate.cookies(); + } + + @Override + public Optional remoteAddress() { + return this.delegate.remoteAddress(); + } + + @Override + public List> messageConverters() { + return this.delegate.messageConverters(); + } + + @Override + public T body(Class bodyType) throws ServletException, IOException { + return this.delegate.body(bodyType); + } + + @Override + public T body(ParameterizedTypeReference bodyType) throws ServletException, IOException { + return this.delegate.body(bodyType); + } + + @Override + public T bind(Class bindType) throws BindException { + return this.delegate.bind(bindType); + } + + @Override + public T bind(Class bindType, Consumer dataBinderCustomizer) throws BindException { + return this.delegate.bind(bindType, dataBinderCustomizer); + } + + @Override + public Map attributes() { + return this.delegate.attributes(); + } + + @Override + public MultiValueMap params() { + return this.delegate.params(); + } + + @Override + public MultiValueMap multipartData() throws IOException, ServletException { + return this.delegate.multipartData(); + } + + @Override + public Map pathVariables() { + return this.delegate.pathVariables(); + } + + @Override + public HttpSession session() { + return this.delegate.session(); + } + + @Override + public Optional principal() { + return this.delegate.principal(); + } + + @Override + public HttpServletRequest servletRequest() { + return this.delegate.servletRequest(); + } + + @Override + public Optional checkNotModified(Instant lastModified) { + return this.delegate.checkNotModified(lastModified); + } + + @Override + public Optional checkNotModified(String etag) { + return this.delegate.checkNotModified(etag); + } + + @Override + public Optional checkNotModified(Instant lastModified, String etag) { + return this.delegate.checkNotModified(lastModified, etag); + } + + @Override + public String toString() { + return this.delegate.toString(); + } + } + + + private static class ExtendedAttributesServerRequestWrapper extends DelegatingServerRequest { private final Map attributes; - public SubPathServerRequestWrapper(ServerRequest request, + + public ExtendedAttributesServerRequestWrapper(ServerRequest delegate, Map newAttributes) { + super(delegate); + Assert.notNull(newAttributes, "NewAttributes must not be null"); + this.attributes = mergeMaps(delegate.attributes(), newAttributes); + } + + + @Override + public Map attributes() { + return this.attributes; + } + + @Override + @SuppressWarnings("unchecked") + public Map pathVariables() { + return (Map) 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) { - this.request = request; + super(request, mergeAttributes(request, info.getUriVariables(), pattern)); this.requestPath = requestPath(request.requestPath(), info); - this.attributes = mergeAttributes(request, info.getUriVariables(), pattern); + } + + private static Map mergeAttributes(ServerRequest request, Map newPathVariables, + PathPattern newPathPattern) { + + + Map oldPathVariables = request.pathVariables(); + PathPattern oldPathPattern = (PathPattern) request.attribute(RouterFunctions.MATCHING_PATTERN_ATTRIBUTE) + .orElse(null); + + Map 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) { @@ -1044,153 +1243,10 @@ public abstract class RequestPredicates { return original.modifyContextPath(contextPath.toString()); } - private static Map mergeAttributes(ServerRequest request, - Map pathVariables, PathPattern pattern) { - Map result = new ConcurrentHashMap<>(request.attributes()); - - 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; - } - - @Override - public HttpMethod method() { - return this.request.method(); - } - - @Override - @Deprecated - public String methodName() { - return this.request.methodName(); - } - - @Override - public URI uri() { - return this.request.uri(); - } - - @Override - public UriBuilder uriBuilder() { - return this.request.uriBuilder(); - } @Override public RequestPath requestPath() { return this.requestPath; } - - @Override - public Headers headers() { - return this.request.headers(); - } - - @Override - public MultiValueMap cookies() { - return this.request.cookies(); - } - - @Override - public Optional remoteAddress() { - return this.request.remoteAddress(); - } - - @Override - public List> messageConverters() { - return this.request.messageConverters(); - } - - @Override - public T body(Class bodyType) throws ServletException, IOException { - return this.request.body(bodyType); - } - - @Override - public T body(ParameterizedTypeReference bodyType) - throws ServletException, IOException { - return this.request.body(bodyType); - } - - @Override - public T bind(Class bindType) throws BindException { - return this.request.bind(bindType); - } - - @Override - public T bind(Class bindType, Consumer dataBinderCustomizer) throws BindException { - return this.request.bind(bindType, dataBinderCustomizer); - } - - @Override - public Optional attribute(String name) { - return this.request.attribute(name); - } - - @Override - public Map attributes() { - return this.attributes; - } - - @Override - public Optional param(String name) { - return this.request.param(name); - } - - @Override - public MultiValueMap params() { - return this.request.params(); - } - - @Override - public MultiValueMap multipartData() throws IOException, ServletException { - return this.request.multipartData(); - } - - @Override - @SuppressWarnings("unchecked") - public Map pathVariables() { - return (Map) this.attributes.getOrDefault( - RouterFunctions.URI_TEMPLATE_VARIABLES_ATTRIBUTE, Collections.emptyMap()); - } - - @Override - public HttpSession session() { - return this.request.session(); - } - - @Override - public Optional principal() { - return this.request.principal(); - } - - @Override - public HttpServletRequest servletRequest() { - return this.request.servletRequest(); - } - - @Override - public Optional checkNotModified(Instant lastModified) { - return this.request.checkNotModified(lastModified); - } - - @Override - public Optional checkNotModified(String etag) { - return this.request.checkNotModified(etag); - } - - @Override - public Optional checkNotModified(Instant lastModified, String etag) { - return this.request.checkNotModified(lastModified, etag); - } - - @Override - public String toString() { - return method() + " " + path(); - } } - } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/function/RouterFunctionsTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/function/RouterFunctionsTests.java index d4dc9caf2b2..b98ba0ed72a 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/function/RouterFunctionsTests.java +++ b/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 handlerFunction = request -> ServerResponse.ok().build(); + RequestPredicate requestPredicate = RequestPredicates.path("/{foo}").and( + request -> request.pathVariable("foo").equals("bar")); + RouterFunction routerFunction = RouterFunctions.route(requestPredicate, handlerFunction); + + MockHttpServletRequest servletRequest = new MockHttpServletRequest("GET", "/bar"); + ServerRequest request = new DefaultServerRequest(servletRequest, Collections.emptyList()); + Optional> resultHandlerFunction = routerFunction.route(request); + assertThat(resultHandlerFunction).isPresent(); + assertThat(resultHandlerFunction).contains(handlerFunction); + } + }