Restore attributes for failed AND/OR request predicates

This commit restores the attributes when either of the predicates in
an AND/OR conjunction is false.

Issue: SPR-17210

Author: poutsma

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

@@ -23,6 +23,7 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.EnumSet;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -349,6 +350,11 @@ private static void traceMatch(String prefix, Object desired, @Nullable Object a
 		}
 	}
 
+	private static void restoreAttributes(ServerRequest request, Map<String, Object> attributes) {
+		request.attributes().clear();
+		request.attributes().putAll(attributes);
+	}
+
 
 	private static class HttpMethodPredicate implements RequestPredicate {
 
@@ -518,8 +524,14 @@ public AndRequestPredicate(RequestPredicate left, RequestPredicate right) {
 		}
 
 		@Override
-		public boolean test(ServerRequest t) {
-			return (this.left.test(t) && this.right.test(t));
+		public boolean test(ServerRequest request) {
+			Map<String, Object> oldAttributes = new HashMap<>(request.attributes());
+
+			if (this.left.test(request) && this.right.test(request)) {
+				return true;
+			}
+			restoreAttributes(request, oldAttributes);
+			return false;
 		}
 
 		@Override
@@ -533,7 +545,6 @@ public String toString() {
 		}
 	}
 
-
 	/**
 	 * {@link RequestPredicate} for where either {@code left} or {@code right} predicates
 	 * may match.
@@ -552,8 +563,20 @@ public OrRequestPredicate(RequestPredicate left, RequestPredicate right) {
 		}
 
 		@Override
-		public boolean test(ServerRequest t) {
-			return (this.left.test(t) || this.right.test(t));
+		public boolean test(ServerRequest request) {
+			Map<String, Object> oldAttributes = new HashMap<>(request.attributes());
+
+			if (this.left.test(request)) {
+				return true;
+			}
+			else {
+				restoreAttributes(request, oldAttributes);
+				if (this.right.test(request)) {
+					return true;
+				}
+			}
+			restoreAttributes(request, oldAttributes);
+			return false;
 		}
 
 		@Override

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

@@ -16,16 +16,23 @@
 
 package org.springframework.web.reactive.function.server;
 
+import java.util.Map;
+
 import org.junit.Test;
 import reactor.core.publisher.Mono;
 
+import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
 import org.springframework.web.client.RestTemplate;
 
 import static org.junit.Assert.*;
-import static org.springframework.web.reactive.function.server.RequestPredicates.*;
-import static org.springframework.web.reactive.function.server.RouterFunctions.*;
+import static org.springframework.web.reactive.function.server.RequestPredicates.GET;
+import static org.springframework.web.reactive.function.server.RequestPredicates.all;
+import static org.springframework.web.reactive.function.server.RequestPredicates.method;
+import static org.springframework.web.reactive.function.server.RequestPredicates.path;
+import static org.springframework.web.reactive.function.server.RouterFunctions.nest;
+import static org.springframework.web.reactive.function.server.RouterFunctions.route;
 
 /**
  * @author Arjen Poutsma
@@ -45,7 +52,8 @@ protected RouterFunction<?> routerFunction() {
 					route(GET("/bar"), nestedHandler::variables).and(
 					nest(GET("/{bar}"),
 								route(GET("/{baz}"), nestedHandler::variables))))
-				.andRoute(GET("/{qux}/quux"), nestedHandler::variables);
+				.andRoute(path("/{qux}/quux").and(method(HttpMethod.GET)), nestedHandler::variables)
+				.andRoute(all(), nestedHandler::variables);
 	}
 
 
@@ -89,7 +97,7 @@ public void parentVariables() {
 
 	// SPR 16692
 	@Test
-	public void removeFailedPathVariables() {
+	public void removeFailedNestedPathVariables() {
 		ResponseEntity<String> result =
 				restTemplate.getForEntity("http://localhost:" + port + "/qux/quux", String.class);
 
@@ -98,6 +106,17 @@ public void removeFailedPathVariables() {
 
 	}
 
+	// SPR 17210
+	@Test
+	public void removeFailedPathVariablesAnd() {
+		ResponseEntity<String> result =
+				restTemplate.postForEntity("http://localhost:" + port + "/qux/quux", "", String.class);
+
+		assertEquals(HttpStatus.OK, result.getStatusCode());
+		assertEquals("{}", result.getBody());
+
+	}
+
 
 	private static class NestedHandler {
 
@@ -109,11 +128,15 @@ public Mono<ServerResponse> baz(ServerRequest request) {
 			return ServerResponse.ok().syncBody("baz");
 		}
 
+		@SuppressWarnings("unchecked")
 		public Mono<ServerResponse> variables(ServerRequest request) {
-			assertEquals(request.pathVariables(),
-					request.attributes().get(RouterFunctions.URI_TEMPLATE_VARIABLES_ATTRIBUTE));
+			Map<String, String> pathVariables = request.pathVariables();
+			Map<String, String> attributePathVariables =
+					(Map<String, String>) request.attributes().get(RouterFunctions.URI_TEMPLATE_VARIABLES_ATTRIBUTE);
+			assertTrue( (pathVariables.equals(attributePathVariables))
+					|| (pathVariables.isEmpty() && (attributePathVariables == null)));
 
-			Mono<String> responseBody = Mono.just(request.pathVariables().toString());
+			Mono<String> responseBody = Mono.just(pathVariables.toString());
 			return ServerResponse.ok().body(responseBody, String.class);
 		}