From 52d4b83dbaa5f4c77cf8e6bb4405a7f72ef8de4d Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Mon, 4 Dec 2023 10:06:28 +0100 Subject: [PATCH] Partially revert RequestPredicates attribute handling This commit partially reverts 39786e479080a337691a3a7d0490547e37a900e3 and c5c843696b50a16077cb6027da127b937c0df6a9, as the approach taken did not take into account request predicates that query request attributes, including path variables. Closes gh-31732 --- .../function/server/RequestPredicates.java | 419 +++++++++-------- .../server/NestedRouteIntegrationTests.java | 8 +- .../RequestPredicateAttributesTests.java | 2 +- .../servlet/function/RequestPredicates.java | 422 ++++++++++-------- .../function/RouterFunctionsTests.java | 14 + 5 files changed, 499 insertions(+), 366 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 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); + } + }