From e0674386e31065f8262593e2642f6634f8aea7a9 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 19 May 2020 20:04:30 +0200 Subject: [PATCH] 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 gh-25106 --- .../function/client/ClientResponse.java | 41 ++++++++++--------- .../client/DefaultClientResponseBuilder.java | 6 +-- .../server/DefaultServerRequestBuilder.java | 32 +++++++++------ .../function/server/ServerRequest.java | 22 +++++----- .../DefaultServerRequestBuilderTests.java | 16 ++++++-- 5 files changed, 68 insertions(+), 49 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientResponse.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientResponse.java index 801de88faf5c..692ae20c55bf 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientResponse.java +++ b/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 cookies(); @@ -148,7 +148,7 @@ public interface ClientResponse { Flux bodyToFlux(ParameterizedTypeReference 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 + *

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 header(String headerName); /** - * Return the headers as a {@link HttpHeaders} instance. + * Return the headers as an {@link HttpHeaders} instance. */ HttpHeaders asHttpHeaders(); } @@ -318,14 +318,14 @@ 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 */ @@ -333,7 +333,7 @@ interface Builder { /** * 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) @@ -341,11 +341,11 @@ interface Builder { 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. + *

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. + *

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> cookiesConsumer); /** - * Set the body of the response. Calling this methods will + * Set the body of the response. + *

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 body); /** * Set the body of the response to the UTF-8 encoded bytes of the given string. - * Calling this methods will + *

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); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilder.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilder.java index 3c43175d04f3..78751e00fce1 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilder.java +++ b/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(); } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerRequestBuilder.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerRequestBuilder.java index a33ce8b7eb82..794dab654af4 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultServerRequestBuilder.java +++ b/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 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 attributes; + private final ServerWebExchange delegate; private final Mono> formDataMono; private final Mono> multipartDataMono; - public DelegatingServerWebExchange( - ServerHttpRequest request, ServerWebExchange delegate, List> messageReaders) { + DelegatingServerWebExchange(ServerHttpRequest request, Map attributes, + ServerWebExchange delegate, List> 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> initMultipartData(ServerHttpReq } return EMPTY_MULTIPART_DATA; } + @Override public ServerHttpRequest getRequest() { return this.request; } + @Override + public Map getAttributes() { + return this.attributes; + } + @Override public Mono> getFormData() { return this.formDataMono; @@ -381,11 +391,6 @@ public ServerHttpResponse getResponse() { return this.delegate.getResponse(); } - @Override - public Map getAttributes() { - return this.delegate.getAttributes(); - } - @Override public Mono getSession() { return this.delegate.getSession(); @@ -442,4 +447,5 @@ public String getLogPrefix() { return this.delegate.getLogPrefix(); } } + } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/ServerRequest.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/ServerRequest.java index 20ee3944c897..1aeb4b917ec0 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/ServerRequest.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/ServerRequest.java @@ -127,7 +127,7 @@ default PathContainer pathContainer() { Optional 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 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 + *

If not modified, this method returns a response with corresponding * status code and headers, otherwise an empty result. *

Typical usage: * 

@@ -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 checkNotModified(Instant lastModified) {
@@ -326,7 +326,7 @@ default Mono 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
+	 * 
If not modified, this method returns a response with corresponding
 	 * status code and headers, otherwise an empty result.
 	 * 
Typical usage:
 	 * 
@@ -350,7 +350,7 @@ default Mono 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 checkNotModified(String etag) {
@@ -362,7 +362,7 @@ default Mono 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
+	 * 
If not modified, this method returns a response with corresponding
 	 * status code and headers, otherwise an empty result.
 	 * 
Typical usage:
 	 * 
@@ -406,8 +406,10 @@ static ServerRequest create(ServerWebExchange exchange, List 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.
 		 * 
Returns an empty list if no header values are found.
 		 * @param headerName the header name
 		 */
 		List 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.
 		 * 
Returns {@code null} if no header values are found.
 		 * @param headerName the header name
 		 * @since 5.2.5
diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/DefaultServerRequestBuilderTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/DefaultServerRequestBuilderTests.java
index 8e4f483d5986..1b2506d20b41 100644
--- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/DefaultServerRequestBuilderTests.java
+++ b/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 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")