Honor attributes configured in ServerRequest.from() builder

Prior to this commit, if attributes were configured in the builder
returned by `ServerRequest.from(...)`, those attributes were not
available in the `ServerRequest` built by the builder. In addition, any
attributes in the original `ServerRequest` supplied to
`ServerRequest.from(...)` were also ignored.

This commit addresses this issue by ensuring that the attributes
configured via DefaultServerRequestBuilder are used as the attributes
in the resulting `ServerRequest`.

This commit also polishes the Javadoc in `ServerRequest` and
`ClientResponse` and avoids the use of lambda expressions in the
constructors for `DefaultServerRequestBuilder` and
`DefaultClientResponseBuilder`.

Closes spring-projectsgh-25106

Author: sbrannen

spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientResponse.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -82,7 +82,7 @@ public interface ClientResponse {
 	Headers headers();
 
 	/**
-	 * Return cookies of this response.
+	 * Return the cookies of this response.
 	 */
 	MultiValueMap<String, ResponseCookie> cookies();
 
@@ -253,7 +253,7 @@ interface Headers {
 		List<String> header(String headerName);
 
 		/**
-		 * Return the headers as a {@link HttpHeaders} instance.
+		 * Return the headers as an {@link HttpHeaders} instance.
 		 */
 		HttpHeaders asHttpHeaders();
 	}
@@ -266,34 +266,34 @@ interface Builder {
 
 		/**
 		 * Set the status code of the response.
-		 * @param statusCode the new status code.
+		 * @param statusCode the new status code
 		 * @return this builder
 		 */
 		Builder statusCode(HttpStatus statusCode);
 
 		/**
 		 * Set the raw status code of the response.
-		 * @param statusCode the new status code.
+		 * @param statusCode the new status code
 		 * @return this builder
 		 * @since 5.1.9
 		 */
 		Builder rawStatusCode(int statusCode);
 
 		/**
 		 * Add the given header value(s) under the given name.
-		 * @param headerName  the header name
+		 * @param headerName the header name
 		 * @param headerValues the header value(s)
 		 * @return this builder
 		 * @see HttpHeaders#add(String, String)
 		 */
 		Builder header(String headerName, String... headerValues);
 
 		/**
-		 * Manipulate this response's headers with the given consumer. The
-		 * headers provided to the consumer are "live", so that the consumer can be used to
-		 * {@linkplain HttpHeaders#set(String, String) overwrite} existing header values,
-		 * {@linkplain HttpHeaders#remove(Object) remove} values, or use any of the other
-		 * {@link HttpHeaders} methods.
+		 * Manipulate this response's headers with the given consumer.
+		 * <p>The headers provided to the consumer are "live", so that the consumer
+		 * can be used to {@linkplain HttpHeaders#set(String, String) overwrite}
+		 * existing header values, {@linkplain HttpHeaders#remove(Object) remove}
+		 * values, or use any of the other {@link HttpHeaders} methods.
 		 * @param headersConsumer a function that consumes the {@code HttpHeaders}
 		 * @return this builder
 		 */
@@ -308,9 +308,9 @@ interface Builder {
 		Builder cookie(String name, String... values);
 
 		/**
-		 * Manipulate this response's cookies with the given consumer. The
-		 * map provided to the consumer is "live", so that the consumer can be used to
-		 * {@linkplain MultiValueMap#set(Object, Object) overwrite} existing header values,
+		 * Manipulate this response's cookies with the given consumer.
+		 * <p>The map provided to the consumer is "live", so that the consumer can be used to
+		 * {@linkplain MultiValueMap#set(Object, Object) overwrite} existing cookie values,
 		 * {@linkplain MultiValueMap#remove(Object) remove} values, or use any of the other
 		 * {@link MultiValueMap} methods.
 		 * @param cookiesConsumer a function that consumes the cookies map
@@ -319,20 +319,21 @@ interface Builder {
 		Builder cookies(Consumer<MultiValueMap<String, ResponseCookie>> cookiesConsumer);
 
 		/**
-		 * Set the body of the response. Calling this methods will
+		 * Set the body of the response.
+		 * <p>Calling this methods will
 		 * {@linkplain org.springframework.core.io.buffer.DataBufferUtils#release(DataBuffer) release}
 		 * the existing body of the builder.
-		 * @param body the new body.
+		 * @param body the new body
 		 * @return this builder
 		 */
 		Builder body(Flux<DataBuffer> body);
 
 		/**
 		 * Set the body of the response to the UTF-8 encoded bytes of the given string.
-		 * Calling this methods will
+		 * <p>Calling this methods will
 		 * {@linkplain org.springframework.core.io.buffer.DataBufferUtils#release(DataBuffer) release}
 		 * the existing body of the builder.
-		 * @param body the new body.
+		 * @param body the new body
 		 * @return this builder
 		 */
 		Builder body(String body);

spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilder.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -53,17 +53,17 @@ final class DefaultClientResponseBuilder implements ClientResponse.Builder {
 	private Flux<DataBuffer> body = Flux.empty();
 
 
-	public DefaultClientResponseBuilder(ExchangeStrategies strategies) {
+	DefaultClientResponseBuilder(ExchangeStrategies strategies) {
 		Assert.notNull(strategies, "ExchangeStrategies must not be null");
 		this.strategies = strategies;
 	}
 
-	public DefaultClientResponseBuilder(ClientResponse other) {
+	DefaultClientResponseBuilder(ClientResponse other) {
 		Assert.notNull(other, "ClientResponse must not be null");
 		this.strategies = other.strategies();
 		this.statusCode = other.rawStatusCode();
-		headers(headers -> headers.addAll(other.headers().asHttpHeaders()));
-		cookies(cookies -> cookies.addAll(other.cookies()));
+		this.headers.addAll(other.headers().asHttpHeaders());
+		this.cookies.addAll(other.cookies());
 	}
 
 
@@ -153,7 +153,7 @@ private static class BuiltClientHttpResponse implements ClientHttpResponse {
 
 		private final Flux<DataBuffer> body;
 
-		public BuiltClientHttpResponse(int statusCode, HttpHeaders headers,
+		BuiltClientHttpResponse(int statusCode, HttpHeaders headers,
 				MultiValueMap<String, ResponseCookie> cookies, Flux<DataBuffer> body) {
 
 			this.statusCode = statusCode;

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -63,6 +63,7 @@
  * Default {@link ServerRequest.Builder} implementation.
  *
  * @author Arjen Poutsma
+ * @author Sam Brannen
  * @since 5.1
  */
 class DefaultServerRequestBuilder implements ServerRequest.Builder {
@@ -84,15 +85,15 @@ class DefaultServerRequestBuilder implements ServerRequest.Builder {
 	private Flux<DataBuffer> body = Flux.empty();
 
 
-	public DefaultServerRequestBuilder(ServerRequest other) {
+	DefaultServerRequestBuilder(ServerRequest other) {
 		Assert.notNull(other, "ServerRequest must not be null");
 		this.messageReaders = other.messageReaders();
 		this.exchange = other.exchange();
 		this.methodName = other.methodName();
 		this.uri = other.uri();
-		headers(headers -> headers.addAll(other.headers().asHttpHeaders()));
-		cookies(cookies -> cookies.addAll(other.cookies()));
-		attributes(attributes -> attributes.putAll(other.attributes()));
+		this.headers.addAll(other.headers().asHttpHeaders());
+		this.cookies.addAll(other.cookies());
+		this.attributes.putAll(other.attributes());
 	}
 
 
@@ -180,7 +181,7 @@ public ServerRequest build() {
 		ServerHttpRequest serverHttpRequest = new BuiltServerHttpRequest(this.exchange.getRequest().getId(),
 				this.methodName, this.uri, this.headers, this.cookies, this.body);
 		ServerWebExchange exchange = new DelegatingServerWebExchange(
-				serverHttpRequest, this.exchange, this.messageReaders);
+				serverHttpRequest, this.attributes, this.exchange, this.messageReaders);
 		return new DefaultServerRequest(exchange, this.messageReaders);
 	}
 
@@ -301,16 +302,19 @@ private static class DelegatingServerWebExchange implements ServerWebExchange {
 
 		private final ServerHttpRequest request;
 
+		private final Map<String, Object> attributes;
+
 		private final ServerWebExchange delegate;
 
 		private final Mono<MultiValueMap<String, String>> formDataMono;
 
 		private final Mono<MultiValueMap<String, Part>> multipartDataMono;
 
-		public DelegatingServerWebExchange(
-				ServerHttpRequest request, ServerWebExchange delegate, List<HttpMessageReader<?>> messageReaders) {
+		DelegatingServerWebExchange(ServerHttpRequest request, Map<String, Object> attributes,
+				ServerWebExchange delegate, List<HttpMessageReader<?>> messageReaders) {
 
 			this.request = request;
+			this.attributes = attributes;
 			this.delegate = delegate;
 			this.formDataMono = initFormData(request, messageReaders);
 			this.multipartDataMono = initMultipartData(request, messageReaders);
@@ -359,11 +363,17 @@ private static Mono<MultiValueMap<String, Part>> initMultipartData(ServerHttpReq
 			}
 			return EMPTY_MULTIPART_DATA;
 		}
+
 		@Override
 		public ServerHttpRequest getRequest() {
 			return this.request;
 		}
 
+		@Override
+		public Map<String, Object> getAttributes() {
+			return this.attributes;
+		}
+
 		@Override
 		public Mono<MultiValueMap<String, String>> getFormData() {
 			return this.formDataMono;
@@ -381,11 +391,6 @@ public ServerHttpResponse getResponse() {
 			return this.delegate.getResponse();
 		}
 
-		@Override
-		public Map<String, Object> getAttributes() {
-			return this.delegate.getAttributes();
-		}
-
 		@Override
 		public Mono<WebSession> getSession() {
 			return this.delegate.getSession();
@@ -442,4 +447,5 @@ public String getLogPrefix() {
 			return this.delegate.getLogPrefix();
 		}
 	}
+
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -296,8 +296,10 @@ static ServerRequest create(ServerWebExchange exchange, List<HttpMessageReader<?
 	}
 
 	/**
-	 * Create a builder with the status, headers, and cookies of the given request.
-	 * @param other the response to copy the status, headers, and cookies from
+	 * Create a builder with the {@linkplain HttpMessageReader message readers},
+	 * method name, URI, headers, cookies, and attributes of the given request.
+	 * @param other the request to copy the message readers, method name, URI,
+	 * headers, and attributes from
 	 * @return the created builder
 	 * @since 5.1
 	 */
@@ -359,7 +361,7 @@ interface Headers {
 		List<HttpRange> range();
 
 		/**
-		 * Get the header value(s), if any, for the header of the given name.
+		 * Get the header value(s), if any, for the header with the given name.
 		 * <p>Returns an empty list if no header values are found.
 		 * @param headerName the header name
 		 */

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -17,6 +17,8 @@
 package org.springframework.web.reactive.function.server;
 
 import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.HashSet;
 
 import org.junit.Test;
 import reactor.core.publisher.Flux;
@@ -29,11 +31,15 @@
 import org.springframework.http.ResponseCookie;
 import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
 import org.springframework.mock.web.test.server.MockServerWebExchange;
+import org.springframework.web.server.ServerWebExchange;
 
 import static org.junit.Assert.*;
 
 /**
+ * Unit tests for {@link DefaultServerRequestBuilder}.
+ *
  * @author Arjen Poutsma
+ * @author Sam Brannen
  */
 public class DefaultServerRequestBuilderTests {
 
@@ -49,6 +55,7 @@ public void from() {
 
 		ServerRequest other =
 				ServerRequest.create(exchange, HandlerStrategies.withDefaults().messageReaders());
+		other.attributes().put("attr1", "value1");
 
 		Flux<DataBuffer> body = Flux.just("baz")
 				.map(s -> s.getBytes(StandardCharsets.UTF_8))
@@ -58,6 +65,8 @@ public void from() {
 				.method(HttpMethod.HEAD)
 				.headers(httpHeaders -> httpHeaders.set("foo", "baar"))
 				.cookies(cookies -> cookies.set("baz", ResponseCookie.from("baz", "quux").build()))
+				.attribute("attr2", "value2")
+				.attributes(attributes -> attributes.put("attr3", "value3"))
 				.body(body)
 				.build();
 
@@ -67,6 +76,13 @@ public void from() {
 		assertEquals(1, result.cookies().size());
 		assertEquals("quux", result.cookies().getFirst("baz").getValue());
 
+		assertEquals(4, result.attributes().size());
+		assertEquals(new HashSet<>(Arrays.asList(ServerWebExchange.LOG_ID_ATTRIBUTE, "attr1", "attr2", "attr3")),
+				result.attributes().keySet());
+		assertEquals("value1", result.attributes().get("attr1"));
+		assertEquals("value2", result.attributes().get("attr2"));
+		assertEquals("value3", result.attributes().get("attr3"));
+
 		StepVerifier.create(result.bodyToFlux(String.class))
 				.expectNext("baz")
 				.verifyComplete();