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

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

@@ -98,7 +98,7 @@ public interface ClientResponse {
 	Headers headers();
 
 	/**
-	 * Return cookies of this response.
+	 * Return the cookies of this response.
 	 */
 	MultiValueMap<String, ResponseCookie> cookies();
 
@@ -148,7 +148,7 @@ public interface ClientResponse {
 	<T> Flux<T> bodyToFlux(ParameterizedTypeReference<T> elementTypeRef);
 
 	/**
-	 * Releases the body of this response.
+	 * Release the body of this response.
 	 * @return a completion signal
 	 * @since 5.2
 	 * @see org.springframework.core.io.buffer.DataBufferUtils#release(DataBuffer)
@@ -206,11 +206,11 @@ public interface ClientResponse {
 
 	/**
 	 * Return a log message prefix to use to correlate messages for this exchange.
-	 * The prefix is based on {@linkplain ClientRequest#logPrefix()}, which
+	 * <p>The prefix is based on {@linkplain ClientRequest#logPrefix()}, which
 	 * itself is based on the value of the {@link ClientRequest#LOG_ID_ATTRIBUTE
 	 * LOG_ID_ATTRIBUTE} request attribute, further surrounded with "[" and "]".
 	 * @return the log message prefix or an empty String if the
-	 * {@link ClientRequest#LOG_ID_ATTRIBUTE LOG_ID_ATTRIBUTE} is not set.
+	 * {@link ClientRequest#LOG_ID_ATTRIBUTE LOG_ID_ATTRIBUTE} is not set
 	 * @since 5.2.3
 	 */
 	String logPrefix();
@@ -305,7 +305,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();
 	}
@@ -318,34 +318,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
 		 */
@@ -360,9 +360,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
@@ -371,20 +371,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.
@@ -88,8 +88,8 @@ public 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());
 		if (other instanceof DefaultClientResponse) {
 			this.request = ((DefaultClientResponse) other).request();
 		}

spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerRequestBuilder.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.
@@ -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

@@ -127,7 +127,7 @@ default PathContainer pathContainer() {
 	Optional<InetSocketAddress> remoteAddress();
 
 	/**
-	 * Get the remote address to which this request is connected, if available.
+	 * Get the local address to which this request is connected, if available.
 	 * @since 5.2.3
 	 */
 	Optional<InetSocketAddress> localAddress();
@@ -292,7 +292,7 @@ default String pathVariable(String name) {
 	/**
 	 * Check whether the requested resource has been modified given the
 	 * supplied last-modified timestamp (as determined by the application).
-	 * If not modified, this method returns a response with corresponding
+	 * <p>If not modified, this method returns a response with corresponding
 	 * status code and headers, otherwise an empty result.
 	 * <p>Typical usage:
 	 * <pre class="code">
@@ -315,7 +315,7 @@ default String pathVariable(String name) {
 	 * @param lastModified the last-modified timestamp that the
 	 * application determined for the underlying resource
 	 * @return a corresponding response if the request qualifies as not
-	 * modified, or an empty result otherwise.
+	 * modified, or an empty result otherwise
 	 * @since 5.2.5
 	 */
 	default Mono<ServerResponse> checkNotModified(Instant lastModified) {
@@ -326,7 +326,7 @@ default Mono<ServerResponse> checkNotModified(Instant lastModified) {
 	/**
 	 * Check whether the requested resource has been modified given the
 	 * supplied {@code ETag} (entity tag), as determined by the application.
-	 * If not modified, this method returns a response with corresponding
+	 * <p>If not modified, this method returns a response with corresponding
 	 * status code and headers, otherwise an empty result.
 	 * <p>Typical usage:
 	 * <pre class="code">
@@ -350,7 +350,7 @@ default Mono<ServerResponse> checkNotModified(Instant lastModified) {
 	 * for the underlying resource. This parameter will be padded
 	 * with quotes (") if necessary.
 	 * @return a corresponding response if the request qualifies as not
-	 * modified, or an empty result otherwise.
+	 * modified, or an empty result otherwise
 	 * @since 5.2.5
 	 */
 	default Mono<ServerResponse> checkNotModified(String etag) {
@@ -362,7 +362,7 @@ default Mono<ServerResponse> checkNotModified(String etag) {
 	 * Check whether the requested resource has been modified given the
 	 * supplied {@code ETag} (entity tag) and last-modified timestamp,
 	 * as determined by the application.
-	 * If not modified, this method returns a response with corresponding
+	 * <p>If not modified, this method returns a response with corresponding
 	 * status code and headers, otherwise an empty result.
 	 * <p>Typical usage:
 	 * <pre class="code">
@@ -406,8 +406,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
 	 */
@@ -469,14 +471,14 @@ 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
 		 */
 		List<String> header(String headerName);
 
 		/**
-		 * Get the first header value, if any, for the header for the given name.
+		 * Get the first header value, if any, for the header with the given name.
 		 * <p>Returns {@code null} if no header values are found.
 		 * @param headerName the header name
 		 * @since 5.2.5

spring-webflux/src/test/java/org/springframework/web/reactive/function/server/DefaultServerRequestBuilderTests.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.
@@ -27,13 +27,18 @@
 import org.springframework.core.io.buffer.DefaultDataBufferFactory;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.ResponseCookie;
+import org.springframework.web.server.ServerWebExchange;
 import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest;
 import org.springframework.web.testfixture.server.MockServerWebExchange;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.entry;
 
 /**
+ * Unit tests for {@link DefaultServerRequestBuilder}.
+ *
  * @author Arjen Poutsma
+ * @author Sam Brannen
  */
 public class DefaultServerRequestBuilderTests {
 
@@ -49,6 +54,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,14 +64,18 @@ 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();
 
 		assertThat(result.method()).isEqualTo(HttpMethod.HEAD);
-		assertThat(result.headers().asHttpHeaders().size()).isEqualTo(1);
+		assertThat(result.headers().asHttpHeaders()).hasSize(1);
 		assertThat(result.headers().asHttpHeaders().getFirst("foo")).isEqualTo("baar");
-		assertThat(result.cookies().size()).isEqualTo(1);
+		assertThat(result.cookies()).hasSize(1);
 		assertThat(result.cookies().getFirst("baz").getValue()).isEqualTo("quux");
+		assertThat(result.attributes()).containsOnlyKeys(ServerWebExchange.LOG_ID_ATTRIBUTE, "attr1", "attr2", "attr3");
+		assertThat(result.attributes()).contains(entry("attr1", "value1"), entry("attr2", "value2"), entry("attr3", "value3"));
 
 		StepVerifier.create(result.bodyToFlux(String.class))
 				.expectNext("baz")