Refactor ServerWebExchange/ServerHttpRequest builders

ServerWebExchange.Builder has an additional Consumer-style shortcut
method that accepts a builder for modifying the request.

ServerWebExchange and ServerHttpRequest builders have fewer methods,
more use-case focused vs matching directly to properties.

spring-web-reactive/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolverTests.java

@@ -15,6 +15,7 @@
  */
 package org.springframework.web.reactive.result.method.annotation;
 
+import java.net.URISyntaxException;
 import java.util.Map;
 import java.util.function.Function;
 
@@ -29,10 +30,9 @@ import org.springframework.core.MethodParameter;
 import org.springframework.core.ReactiveAdapterRegistry;
 import org.springframework.core.ResolvableType;
 import org.springframework.http.HttpMethod;
+import org.springframework.http.MediaType;
 import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
 import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse;
-import org.springframework.util.LinkedMultiValueMap;
-import org.springframework.util.MultiValueMap;
 import org.springframework.validation.BindingResult;
 import org.springframework.validation.annotation.Validated;
 import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean;
@@ -41,18 +41,19 @@ import org.springframework.web.bind.support.ConfigurableWebBindingInitializer;
 import org.springframework.web.bind.support.WebExchangeBindException;
 import org.springframework.web.reactive.BindingContext;
 import org.springframework.web.reactive.result.ResolvableMethod;
-import org.springframework.web.reactive.result.method.HandlerMethodArgumentResolver;
 import org.springframework.web.server.ServerWebExchange;
 import org.springframework.web.server.adapter.DefaultServerWebExchange;
 import org.springframework.web.server.session.MockWebSessionManager;
+import org.springframework.web.server.session.WebSessionManager;
 
 import static junit.framework.TestCase.assertNotNull;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
-import static org.springframework.core.ResolvableType.*;
-import static org.springframework.util.Assert.*;
+import static org.springframework.core.ResolvableType.forClass;
+import static org.springframework.core.ResolvableType.forClassWithGenerics;
+import static org.springframework.util.Assert.isTrue;
 
 
 /**
@@ -61,28 +62,18 @@ import static org.springframework.util.Assert.*;
  */
 public class ModelAttributeMethodArgumentResolverTests {
 
-	private ServerWebExchange exchange;
-
-	private final MockServerHttpRequest request = new MockServerHttpRequest(HttpMethod.POST, "/path");
-
-	private final MultiValueMap<String, String> formData = new LinkedMultiValueMap<>();
-
-	private BindingContext bindingContext;
+	private BindingContext bindContext;
 
 	private ResolvableMethod testMethod = ResolvableMethod.onClass(this.getClass()).name("handle");
 
 
 	@Before
 	public void setUp() throws Exception {
-		MockServerHttpResponse response = new MockServerHttpResponse();
-		this.exchange = new DefaultServerWebExchange(this.request, response, new MockWebSessionManager());
-		this.exchange = this.exchange.mutate().formData(Mono.just(this.formData)).build();
-
 		LocalValidatorFactoryBean validator = new LocalValidatorFactoryBean();
 		validator.afterPropertiesSet();
 		ConfigurableWebBindingInitializer initializer = new ConfigurableWebBindingInitializer();
 		initializer.setValidator(validator);
-		this.bindingContext = new BindingContext(initializer);
+		this.bindContext = new BindingContext(initializer);
 	}
 
 
@@ -156,49 +147,49 @@ public class ModelAttributeMethodArgumentResolverTests {
 	public void bindExisting() throws Exception {
 		Foo foo = new Foo();
 		foo.setName("Jim");
-		this.bindingContext.getModel().addAttribute(foo);
+		this.bindContext.getModel().addAttribute(foo);
 
 		testBindFoo(forClass(Foo.class), value -> {
 			assertEquals(Foo.class, value.getClass());
 			return (Foo) value;
 		});
 
-		assertSame(foo, this.bindingContext.getModel().asMap().get("foo"));
+		assertSame(foo, this.bindContext.getModel().asMap().get("foo"));
 	}
 
 	@Test
 	public void bindExistingMono() throws Exception {
 		Foo foo = new Foo();
 		foo.setName("Jim");
-		this.bindingContext.getModel().addAttribute("foo", Mono.just(foo));
+		this.bindContext.getModel().addAttribute("foo", Mono.just(foo));
 
 		testBindFoo(forClass(Foo.class), value -> {
 			assertEquals(Foo.class, value.getClass());
 			return (Foo) value;
 		});
 
-		assertSame(foo, this.bindingContext.getModel().asMap().get("foo"));
+		assertSame(foo, this.bindContext.getModel().asMap().get("foo"));
 	}
 
 	@Test
 	public void bindExistingSingle() throws Exception {
 		Foo foo = new Foo();
 		foo.setName("Jim");
-		this.bindingContext.getModel().addAttribute("foo", Single.just(foo));
+		this.bindContext.getModel().addAttribute("foo", Single.just(foo));
 
 		testBindFoo(forClass(Foo.class), value -> {
 			assertEquals(Foo.class, value.getClass());
 			return (Foo) value;
 		});
 
-		assertSame(foo, this.bindingContext.getModel().asMap().get("foo"));
+		assertSame(foo, this.bindContext.getModel().asMap().get("foo"));
 	}
 
 	@Test
 	public void bindExistingMonoToMono() throws Exception {
 		Foo foo = new Foo();
 		foo.setName("Jim");
-		this.bindingContext.getModel().addAttribute("foo", Mono.just(foo));
+		this.bindContext.getModel().addAttribute("foo", Mono.just(foo));
 
 		testBindFoo(forClassWithGenerics(Mono.class, Foo.class), mono -> {
 			assertTrue(mono.getClass().getName(), mono instanceof Mono);
@@ -238,14 +229,11 @@ public class ModelAttributeMethodArgumentResolverTests {
 	}
 
 
-	private void testBindFoo(ResolvableType type, Function<Object, Foo> valueExtractor) {
-
-		this.formData.add("name", "Robert");
-		this.formData.add("age", "25");
+	private void testBindFoo(ResolvableType type, Function<Object, Foo> valueExtractor) throws Exception {
 
 		Object value = createResolver()
-				.resolveArgument(parameter(type), this.bindingContext, this.exchange)
-				.blockMillis(5000);
+				.resolveArgument(parameter(type), this.bindContext, exchange("name=Robert&age=25"))
+				.blockMillis(0);
 
 		Foo foo = valueExtractor.apply(value);
 		assertEquals("Robert", foo.getName());
@@ -253,19 +241,18 @@ public class ModelAttributeMethodArgumentResolverTests {
 		String key = "foo";
 		String bindingResultKey = BindingResult.MODEL_KEY_PREFIX + key;
 
-		Map<String, Object> map = bindingContext.getModel().asMap();
+		Map<String, Object> map = bindContext.getModel().asMap();
 		assertEquals(map.toString(), 2, map.size());
 		assertSame(foo, map.get(key));
 		assertNotNull(map.get(bindingResultKey));
 		assertTrue(map.get(bindingResultKey) instanceof BindingResult);
 	}
 
-	private void testValidationError(ResolvableType type, Function<Mono<?>, Mono<?>> valueMonoExtractor) {
-
-		this.formData.add("age", "invalid");
+	private void testValidationError(ResolvableType type, Function<Mono<?>, Mono<?>> valueMonoExtractor)
+			throws URISyntaxException {
 
-		HandlerMethodArgumentResolver resolver = createResolver();
-		Mono<?> mono = resolver.resolveArgument(parameter(type), this.bindingContext, this.exchange);
+		ServerWebExchange exchange = exchange("age=invalid");
+		Mono<?> mono = createResolver().resolveArgument(parameter(type), this.bindContext, exchange);
 
 		mono = valueMonoExtractor.apply(mono);
 
@@ -294,6 +281,15 @@ public class ModelAttributeMethodArgumentResolverTests {
 				parameter -> !parameter.hasParameterAnnotations());
 	}
 
+	private ServerWebExchange exchange(String formData) throws URISyntaxException {
+		MockServerHttpRequest request = new MockServerHttpRequest(HttpMethod.GET, "/");
+		request.getHeaders().setContentType(MediaType.APPLICATION_FORM_URLENCODED);
+		request.setBody(formData);
+		MockServerHttpResponse response = new MockServerHttpResponse();
+		WebSessionManager manager = new MockWebSessionManager();
+		return new DefaultServerWebExchange(request, response, manager);
+	}
+
 
 	@SuppressWarnings("unused")
 	void handle(

spring-web/src/main/java/org/springframework/http/server/reactive/DefaultServerHttpRequestBuilder.java

@@ -17,14 +17,9 @@ package org.springframework.http.server.reactive;
 
 import java.net.URI;
 
-import reactor.core.publisher.Flux;
-
-import org.springframework.core.io.buffer.DataBuffer;
-import org.springframework.http.HttpCookie;
-import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpMethod;
 import org.springframework.util.Assert;
-import org.springframework.util.MultiValueMap;
+import org.springframework.web.util.UriComponentsBuilder;
 
 /**
  * Package private default implementation of {@link ServerHttpRequest.Builder}.
@@ -36,21 +31,12 @@ class DefaultServerHttpRequestBuilder implements ServerHttpRequest.Builder {
 
 	private final ServerHttpRequest delegate;
 
-
 	private HttpMethod httpMethod;
 
-	private URI uri;
+	private String path;
 
 	private String contextPath;
 
-	private MultiValueMap<String, String> queryParams;
-
-	private HttpHeaders headers;
-
-	private MultiValueMap<String, HttpCookie> cookies;
-
-	private Flux<DataBuffer> body;
-
 
 	public DefaultServerHttpRequestBuilder(ServerHttpRequest delegate) {
 		Assert.notNull(delegate, "ServerHttpRequest delegate is required.");
@@ -65,8 +51,8 @@ class DefaultServerHttpRequestBuilder implements ServerHttpRequest.Builder {
 	}
 
 	@Override
-	public ServerHttpRequest.Builder uri(URI uri) {
-		this.uri = uri;
+	public ServerHttpRequest.Builder path(String path) {
+		this.path = path;
 		return this;
 	}
 
@@ -76,34 +62,14 @@ class DefaultServerHttpRequestBuilder implements ServerHttpRequest.Builder {
 		return this;
 	}
 
-	@Override
-	public ServerHttpRequest.Builder queryParams(MultiValueMap<String, String> queryParams) {
-		this.queryParams = queryParams;
-		return this;
-	}
-
-	@Override
-	public ServerHttpRequest.Builder headers(HttpHeaders headers) {
-		this.headers = headers;
-		return this;
-	}
-
-	@Override
-	public ServerHttpRequest.Builder cookies(MultiValueMap<String, HttpCookie> cookies) {
-		this.cookies = cookies;
-		return this;
-	}
-
-	@Override
-	public ServerHttpRequest.Builder body(Flux<DataBuffer> body) {
-		this.body = body;
-		return this;
-	}
-
 	@Override
 	public ServerHttpRequest build() {
-		return new MutativeDecorator(this.delegate, this.httpMethod, this.uri, this.contextPath,
-				this.queryParams, this.headers, this.cookies, this.body);
+		URI uri = null;
+		if (this.path != null) {
+			uri = this.delegate.getURI();
+			uri = UriComponentsBuilder.fromUri(uri).replacePath(this.path).build(true).toUri();
+		}
+		return new MutativeDecorator(this.delegate, this.httpMethod, uri, this.contextPath);
 	}
 
 
@@ -119,27 +85,14 @@ class DefaultServerHttpRequestBuilder implements ServerHttpRequest.Builder {
 
 		private final String contextPath;
 
-		private final MultiValueMap<String, String> queryParams;
-
-		private final HttpHeaders headers;
 
-		private final MultiValueMap<String, HttpCookie> cookies;
-
-		private final Flux<DataBuffer> body;
-
-
-		public MutativeDecorator(ServerHttpRequest delegate, HttpMethod httpMethod, URI uri,
-				String contextPath, MultiValueMap<String, String> queryParams, HttpHeaders headers,
-				MultiValueMap<String, HttpCookie> cookies, Flux<DataBuffer> body) {
+		public MutativeDecorator(ServerHttpRequest delegate, HttpMethod httpMethod,
+				URI uri, String contextPath) {
 
 			super(delegate);
 			this.httpMethod = httpMethod;
 			this.uri = uri;
 			this.contextPath = contextPath;
-			this.queryParams = queryParams;
-			this.headers = headers;
-			this.cookies = cookies;
-			this.body = body;
 		}
 
 		@Override
@@ -157,25 +110,6 @@ class DefaultServerHttpRequestBuilder implements ServerHttpRequest.Builder {
 			return (this.contextPath != null ? this.contextPath : super.getContextPath());
 		}
 
-		@Override
-		public MultiValueMap<String, String> getQueryParams() {
-			return (this.queryParams != null ? this.queryParams : super.getQueryParams());
-		}
-
-		@Override
-		public HttpHeaders getHeaders() {
-			return (this.headers != null ? this.headers : super.getHeaders());
-		}
-
-		@Override
-		public MultiValueMap<String, HttpCookie> getCookies() {
-			return (this.cookies != null ? this.cookies : super.getCookies());
-		}
-
-		@Override
-		public Flux<DataBuffer> getBody() {
-			return (this.body != null ? this.body : super.getBody());
-		}
 	}
 
 }

spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpRequest.java

@@ -61,9 +61,9 @@ public interface ServerHttpRequest extends HttpRequest, ReactiveHttpInputMessage
 
 
 	/**
-	 * Return a builder to mutate properties of this request. The resulting
-	 * new request is an immutable {@link ServerHttpRequestDecorator decorator}
-	 * around the current exchange instance returning mutated values.
+	 * Return a builder to mutate properties of this request by wrapping it
+	 * with {@link ServerHttpRequestDecorator} and returning either mutated
+	 * values or delegating back to this instance.
 	 */
 	default ServerHttpRequest.Builder mutate() {
 		return new DefaultServerHttpRequestBuilder(this);
@@ -71,51 +71,29 @@ public interface ServerHttpRequest extends HttpRequest, ReactiveHttpInputMessage
 
 
 	/**
-	 * Builder for mutating properties of a {@link ServerHttpRequest}.
+	 * Builder for mutating an existing {@link ServerHttpRequest}.
 	 */
 	interface Builder {
 
 		/**
-		 * Set the HTTP method.
+		 * Set the HTTP method to return.
 		 */
 		Builder method(HttpMethod httpMethod);
 
 		/**
-		 * Set the request URI.
+		 * Set the request URI to return.
 		 */
-		Builder uri(URI uri);
-
+		Builder path(String path);
 
 		/**
-		 * Set the contextPath for the request.
+		 * Set the contextPath to return.
 		 */
 		Builder contextPath(String contextPath);
 
 		/**
-		 * Set the query params to return.
-		 */
-		Builder queryParams(MultiValueMap<String, String> queryParams);
-
-		/**
-		 * Set the headers to use.
-		 */
-		Builder headers(HttpHeaders headers);
-
-		/**
-		 * Set the cookies to use.
-		 */
-		Builder cookies(MultiValueMap<String, HttpCookie> cookies);
-
-		/**
-		 * Set the body to return.
-		 */
-		Builder body(Flux<DataBuffer> body);
-
-		/**
-		 * Build an immutable wrapper that returning the mutated properties.
+		 * Build a {@link ServerHttpRequest} decorator with the mutated properties.
 		 */
 		ServerHttpRequest build();
-
 	}
 
 }

spring-web/src/main/java/org/springframework/web/server/DefaultServerWebExchangeBuilder.java

@@ -16,13 +16,13 @@
 package org.springframework.web.server;
 
 import java.security.Principal;
+import java.util.function.Consumer;
 
 import reactor.core.publisher.Mono;
 
 import org.springframework.http.server.reactive.ServerHttpRequest;
 import org.springframework.http.server.reactive.ServerHttpResponse;
 import org.springframework.util.Assert;
-import org.springframework.util.MultiValueMap;
 
 /**
  * Package private implementation of {@link ServerWebExchange.Builder}.
@@ -38,19 +38,22 @@ class DefaultServerWebExchangeBuilder implements ServerWebExchange.Builder {
 
 	private ServerHttpResponse response;
 
-	private Mono<Principal> user;
+	private Mono<Principal> principalMono;
 
-	private Mono<WebSession> session;
 
-	private Mono<MultiValueMap<String, String>> formData;
-
-
-	public DefaultServerWebExchangeBuilder(ServerWebExchange delegate) {
+	DefaultServerWebExchangeBuilder(ServerWebExchange delegate) {
 		Assert.notNull(delegate, "'delegate' is required.");
 		this.delegate = delegate;
 	}
 
 
+	@Override
+	public ServerWebExchange.Builder request(Consumer<ServerHttpRequest.Builder> consumer) {
+		ServerHttpRequest.Builder builder = this.delegate.getRequest().mutate();
+		consumer.accept(builder);
+		return request(builder.build());
+	}
+
 	@Override
 	public ServerWebExchange.Builder request(ServerHttpRequest request) {
 		this.request = request;
@@ -64,27 +67,14 @@ class DefaultServerWebExchangeBuilder implements ServerWebExchange.Builder {
 	}
 
 	@Override
-	public ServerWebExchange.Builder principal(Mono<Principal> user) {
-		this.user = user;
-		return this;
-	}
-
-	@Override
-	public ServerWebExchange.Builder session(Mono<WebSession> session) {
-		this.session = session;
-		return this;
-	}
-
-	@Override
-	public ServerWebExchange.Builder formData(Mono<MultiValueMap<String, String>> formData) {
-		this.formData = formData;
+	public ServerWebExchange.Builder principal(Mono<Principal> principalMono) {
+		this.principalMono = principalMono;
 		return this;
 	}
 
 	@Override
 	public ServerWebExchange build() {
-		return new MutativeDecorator(this.delegate, this.request, this.response,
-				this.user, this.session, this.formData);
+		return new MutativeDecorator(this.delegate, this.request, this.response, this.principalMono);
 	}
 
 
@@ -98,23 +88,16 @@ class DefaultServerWebExchangeBuilder implements ServerWebExchange.Builder {
 
 		private final ServerHttpResponse response;
 
-		private final Mono<Principal> userMono;
-
-		private final Mono<WebSession> session;
+		private final Mono<Principal> principalMono;
 
-		private final Mono<MultiValueMap<String, String>> formData;
 
-
-		public MutativeDecorator(ServerWebExchange delegate,
-				ServerHttpRequest request, ServerHttpResponse response, Mono<Principal> user,
-				Mono<WebSession> session, Mono<MultiValueMap<String, String>> formData) {
+		public MutativeDecorator(ServerWebExchange delegate, ServerHttpRequest request,
+				ServerHttpResponse response, Mono<Principal> principalMono) {
 
 			super(delegate);
 			this.request = request;
 			this.response = response;
-			this.userMono = user;
-			this.session = session;
-			this.formData = formData;
+			this.principalMono = principalMono;
 		}
 
 
@@ -128,20 +111,11 @@ class DefaultServerWebExchangeBuilder implements ServerWebExchange.Builder {
 			return (this.response != null ? this.response : getDelegate().getResponse());
 		}
 
-		@Override
-		public Mono<WebSession> getSession() {
-			return (this.session != null ? this.session : getDelegate().getSession());
-		}
-
 		@SuppressWarnings("unchecked")
 		@Override
 		public <T extends Principal> Mono<T> getPrincipal() {
-			return (this.userMono != null ? (Mono<T>) this.userMono : getDelegate().getPrincipal());
-		}
-
-		@Override
-		public Mono<MultiValueMap<String, String>> getFormData() {
-			return (this.formData != null ? this.formData : getDelegate().getFormData());
+			return (this.principalMono != null ?
+					(Mono<T>) this.principalMono : getDelegate().getPrincipal());
 		}
 	}
 

spring-web/src/main/java/org/springframework/web/server/ServerWebExchange.java

@@ -20,6 +20,7 @@ import java.security.Principal;
 import java.time.Instant;
 import java.util.Map;
 import java.util.Optional;
+import java.util.function.Consumer;
 
 import reactor.core.publisher.Mono;
 
@@ -134,9 +135,9 @@ public interface ServerWebExchange {
 
 
 	/**
-	 * Return a builder to mutate properties of this exchange. The resulting
-	 * new exchange is an immutable {@link ServerWebExchangeDecorator decorator}
-	 * around the current exchange instance returning mutated values.
+	 * Return a builder to mutate properties of this exchange by wrapping it
+	 * with {@link ServerWebExchangeDecorator} and returning either mutated
+	 * values or delegating back to this instance.
 	 */
 	default Builder mutate() {
 		return new DefaultServerWebExchangeBuilder(this);
@@ -144,40 +145,52 @@ public interface ServerWebExchange {
 
 
 	/**
-	 * Builder for mutating the properties of a {@link ServerWebExchange}.
+	 * Builder for mutating an existing {@link ServerWebExchange}.
+	 * Removes the need
 	 */
 	interface Builder {
 
 		/**
-		 * Set the request to use.
+		 * Configure a consumer to modify the current request using a builder.
+		 * <p>Effectively this:
+		 * <pre>
+		 * exchange.mutate().request(builder-> builder.method(HttpMethod.PUT));
+		 *
+		 * // vs...
+		 *
+		 * ServerHttpRequest request = exchange.getRequest().mutate()
+		 *     .method(HttpMethod.PUT)
+		 *     .build();
+		 *
+		 * exchange.mutate().request(request);
+		 * </pre>
+		 * @see ServerHttpRequest#mutate()
 		 */
-		Builder request(ServerHttpRequest request);
-
-		/**
-		 * Set the response to use.
-		 */
-		Builder response(ServerHttpResponse response);
+		Builder request(Consumer<ServerHttpRequest.Builder> requestBuilderConsumer);
 
 		/**
-		 * Set the principal to use.
+		 * Set the request to use especially when there is a need to override
+		 * {@link ServerHttpRequest} methods. To simply mutate request properties
+		 * see {@link #request(Consumer)} instead.
+		 * @see org.springframework.http.server.reactive.ServerHttpRequestDecorator
 		 */
-		Builder principal(Mono<Principal> user);
+		Builder request(ServerHttpRequest request);
 
 		/**
-		 * Set the session to use.
+		 * Set the response to use.
+		 * @see org.springframework.http.server.reactive.ServerHttpResponseDecorator
 		 */
-		Builder session(Mono<WebSession> session);
+		Builder response(ServerHttpResponse response);
 
 		/**
-		 * Set the form data.
+		 * Set the {@code Mono<Principal>} to return for this exchange.
 		 */
-		Builder formData(Mono<MultiValueMap<String, String>> formData);
+		Builder principal(Mono<Principal> principalMono);
 
 		/**
-		 * Build an immutable wrapper that returning the mutated properties.
+		 * Build a {@link ServerWebExchange} decorator with the mutated properties.
 		 */
 		ServerWebExchange build();
-
 	}
 
 }

spring-web/src/test/java/org/springframework/http/server/reactive/HttpHandlerAdapterSupportTests.java

@@ -18,7 +18,6 @@ package org.springframework.http.server.reactive;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.LinkedHashMap;
 import java.util.Map;
 
 import org.junit.Test;