Polishing contribution

See gh-33697

spring-web/src/main/java/org/springframework/web/client/DefaultRestClient.java

@@ -40,7 +40,6 @@ import org.apache.commons.logging.LogFactory;
 
 import org.springframework.core.ParameterizedTypeReference;
 import org.springframework.core.ResolvableType;
-import org.springframework.http.HttpCookie;
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpRequest;
@@ -301,8 +300,6 @@ final class DefaultRestClient implements RestClient {
 
 	private class DefaultRequestBodyUriSpec implements RequestBodyUriSpec {
 
-		private static final String COOKIE_DELIMITER = "; ";
-
 		private final HttpMethod httpMethod;
 
 		@Nullable
@@ -557,10 +554,9 @@ final class DefaultRestClient implements RestClient {
 			try {
 				uri = initUri();
 				HttpHeaders headers = initHeaders();
-
 				MultiValueMap<String, String> cookies = initCookies();
 				if (!CollectionUtils.isEmpty(cookies)) {
-					headers.put(HttpHeaders.COOKIE, List.of(cookiesToHeaderValue(cookies)));
+					headers.set(HttpHeaders.COOKIE, serializeCookies(cookies));
 				}
 
 				ClientHttpRequest clientRequest = createRequest(uri);
@@ -638,25 +634,36 @@ final class DefaultRestClient implements RestClient {
 		}
 
 		private MultiValueMap<String, String> initCookies() {
-			MultiValueMap<String, String> mergedCookies = new LinkedMultiValueMap<>();
-
-			if(!CollectionUtils.isEmpty(defaultCookies)) {
-				mergedCookies.putAll(defaultCookies);
+			MultiValueMap<String, String> defaultCookies = DefaultRestClient.this.defaultCookies;
+			if (CollectionUtils.isEmpty(this.cookies)) {
+				return (defaultCookies != null ? defaultCookies : new LinkedMultiValueMap<>());
 			}
-
-			if(!CollectionUtils.isEmpty(this.cookies)) {
-				mergedCookies.putAll(this.cookies);
+			else if (CollectionUtils.isEmpty(defaultCookies)) {
+				return this.cookies;
+			}
+			else {
+				MultiValueMap<String, String> map = new LinkedMultiValueMap<>();
+				map.putAll(DefaultRestClient.this.defaultCookies);
+				map.putAll(this.cookies);
+				return map;
 			}
-
-			return mergedCookies;
 		}
 
-		private String cookiesToHeaderValue(MultiValueMap<String, String> cookies) {
-			List<String> flatCookies = new ArrayList<>();
-			cookies.forEach((name, cookieValues) -> cookieValues.forEach(value ->
-				flatCookies.add(new HttpCookie(name, value).toString())
-			));
-			return String.join(COOKIE_DELIMITER, flatCookies);
+		private String serializeCookies(MultiValueMap<String, String> cookies) {
+			boolean first = true;
+			StringBuilder sb = new StringBuilder();
+			for (Map.Entry<String, List<String>> entry : cookies.entrySet()) {
+				for (String value : entry.getValue()) {
+					if (!first) {
+						sb.append("; ");
+					}
+					else {
+						first = false;
+					}
+					sb.append(entry.getKey()).append("=").append(value);
+				}
+			}
+			return sb.toString();
 		}
 
 		private ClientHttpRequest createRequest(URI uri) throws IOException {

spring-web/src/main/java/org/springframework/web/client/DefaultRestClientBuilder.java

@@ -468,21 +468,21 @@ final class DefaultRestClientBuilder implements RestClient.Builder {
 	public RestClient build() {
 		ClientHttpRequestFactory requestFactory = initRequestFactory();
 		UriBuilderFactory uriBuilderFactory = initUriBuilderFactory();
+
 		HttpHeaders defaultHeaders = copyDefaultHeaders();
 		MultiValueMap<String, String> defaultCookies = copyDefaultCookies();
-		List<HttpMessageConverter<?>> messageConverters = (this.messageConverters != null ?
-				this.messageConverters : initMessageConverters());
-		return new DefaultRestClient(requestFactory,
-				this.interceptors, this.initializers, uriBuilderFactory,
-				defaultHeaders,
-				defaultCookies,
+
+		List<HttpMessageConverter<?>> converters =
+				(this.messageConverters != null ? this.messageConverters : initMessageConverters());
+
+		return new DefaultRestClient(
+				requestFactory, this.interceptors, this.initializers,
+				uriBuilderFactory, defaultHeaders, defaultCookies,
 				this.defaultRequest,
 				this.statusHandlers,
-				messageConverters,
-				this.observationRegistry,
-				this.observationConvention,
-				new DefaultRestClientBuilder(this)
-				);
+				converters,
+				this.observationRegistry, this.observationConvention,
+				new DefaultRestClientBuilder(this));
 	}
 
 	private ClientHttpRequestFactory initRequestFactory() {
@@ -519,26 +519,22 @@ final class DefaultRestClientBuilder implements RestClient.Builder {
 
 	@Nullable
 	private HttpHeaders copyDefaultHeaders() {
-		if (this.defaultHeaders != null) {
+		if (this.defaultHeaders == null) {
+			return null;
+		}
 		HttpHeaders copy = new HttpHeaders();
 		this.defaultHeaders.forEach((key, values) -> copy.put(key, new ArrayList<>(values)));
 		return HttpHeaders.readOnlyHttpHeaders(copy);
 	}
-		else {
-			return null;
-		}
-	}
 
 	@Nullable
 	private MultiValueMap<String, String> copyDefaultCookies() {
-		if (this.defaultCookies != null) {
+		if (this.defaultCookies == null) {
+			return null;
+		}
 		MultiValueMap<String, String> copy = new LinkedMultiValueMap<>(this.defaultCookies.size());
 		this.defaultCookies.forEach((key, values) -> copy.put(key, new ArrayList<>(values)));
 		return CollectionUtils.unmodifiableMultiValueMap(copy);
 	}
-		else {
-			return null;
-		}
-	}
 
 }

spring-web/src/test/java/org/springframework/web/client/RestClientBuilderTests.java

@@ -151,7 +151,7 @@ public class RestClientBuilderTests {
 	}
 
 	@Test
-	void defaultCookieWithMultipleValuesAddsCookieToDefaultCookiesMapWithAllValues() {
+	void defaultCookieWithMultipleValuesAddsCookieToDefaultCookiesMap() {
 		RestClient.Builder builder = RestClient.builder();
 
 		builder.defaultCookie("myCookie", "testValue1", "testValue2");

spring-web/src/test/java/org/springframework/web/client/RestClientIntegrationTests.java

@@ -23,7 +23,6 @@ import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 import java.net.URI;
 import java.net.URISyntaxException;
-import java.nio.charset.StandardCharsets;
 import java.util.List;
 import java.util.Map;
 import java.util.function.Consumer;
@@ -56,6 +55,7 @@ import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.util.MultiValueMap;
 import org.springframework.web.testfixture.xml.Pojo;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.junit.jupiter.api.Assumptions.assumeFalse;
@@ -660,8 +660,8 @@ class RestClientIntegrationTests {
 		startServer(requestFactory);
 
 		String content = "Internal Server error";
-		prepareResponse(response -> response.setResponseCode(500)
-				.setHeader("Content-Type", "text/plain").setBody(content));
+		prepareResponse(response ->
+				response.setResponseCode(500).setHeader("Content-Type", "text/plain").setBody(content));
 
 		ResponseEntity<String> result = this.restClient.get()
 				.uri("/").accept(MediaType.APPLICATION_JSON)
@@ -689,7 +689,7 @@ class RestClientIntegrationTests {
 		String result = this.restClient.get()
 				.uri("/greeting")
 				.header("X-Test-Header", "testvalue")
-				.exchange((request, response) -> new String(RestClientUtils.getBody(response), StandardCharsets.UTF_8));
+				.exchange((request, response) -> new String(RestClientUtils.getBody(response), UTF_8));
 
 		assertThat(result).isEqualTo("Hello Spring!");
 
@@ -753,12 +753,12 @@ class RestClientIntegrationTests {
 	void exchangeFor404(ClientHttpRequestFactory requestFactory) {
 		startServer(requestFactory);
 
-		prepareResponse(response -> response.setResponseCode(404)
-				.setHeader("Content-Type", "text/plain").setBody("Not Found"));
+		prepareResponse(response ->
+				response.setResponseCode(404).setHeader("Content-Type", "text/plain").setBody("Not Found"));
 
 		String result = this.restClient.get()
 				.uri("/greeting")
-				.exchange((request, response) -> new String(RestClientUtils.getBody(response), StandardCharsets.UTF_8));
+				.exchange((request, response) -> new String(RestClientUtils.getBody(response), UTF_8));
 
 		assertThat(result).isEqualTo("Not Found");
 
@@ -770,8 +770,8 @@ class RestClientIntegrationTests {
 	void requestInitializer(ClientHttpRequestFactory requestFactory) {
 		startServer(requestFactory);
 
-		prepareResponse(response -> response.setHeader("Content-Type", "text/plain")
-				.setBody("Hello Spring!"));
+		prepareResponse(response ->
+				response.setHeader("Content-Type", "text/plain").setBody("Hello Spring!"));
 
 		RestClient initializedClient = this.restClient.mutate()
 				.requestInitializer(request -> request.getHeaders().add("foo", "bar"))
@@ -792,9 +792,8 @@ class RestClientIntegrationTests {
 	void requestInterceptor(ClientHttpRequestFactory requestFactory) {
 		startServer(requestFactory);
 
-		prepareResponse(response -> response.setHeader("Content-Type", "text/plain")
-				.setBody("Hello Spring!"));
-
+		prepareResponse(response ->
+				response.setHeader("Content-Type", "text/plain").setBody("Hello Spring!"));
 
 		RestClient interceptedClient = this.restClient.mutate()
 				.requestInterceptor((request, body, execution) -> {
@@ -819,6 +818,7 @@ class RestClientIntegrationTests {
 		startServer(requestFactory);
 		prepareResponse(response ->
 				response.setHeader("Content-Type", "text/plain").setBody("Hello Spring!"));
+
 		RestClient restClientWithCookies = this.restClient.mutate()
 				.defaultCookie("testCookie", "firstValue", "secondValue")
 				.build();
@@ -852,8 +852,8 @@ class RestClientIntegrationTests {
 		RestClient interceptedClient = this.restClient.mutate().requestInterceptor(interceptor).build();
 
 		// header not present
-		prepareResponse(response -> response
-				.setHeader("Content-Type", "text/plain").setBody("Hello Spring!"));
+		prepareResponse(response ->
+				response.setHeader("Content-Type", "text/plain").setBody("Hello Spring!"));
 
 		assertThatExceptionOfType(MyException.class).isThrownBy(() ->
 				interceptedClient.get()
@@ -881,8 +881,8 @@ class RestClientIntegrationTests {
 	void defaultHeaders(ClientHttpRequestFactory requestFactory) {
 		startServer(requestFactory);
 
-		prepareResponse(response -> response.setHeader("Content-Type", "text/plain")
-				.setBody("Hello Spring!"));
+		prepareResponse(response ->
+				response.setHeader("Content-Type", "text/plain").setBody("Hello Spring!"));
 
 		RestClient headersClient = this.restClient.mutate()
 				.defaultHeaders(headers -> headers.add("foo", "bar"))
@@ -903,8 +903,8 @@ class RestClientIntegrationTests {
 	void defaultRequest(ClientHttpRequestFactory requestFactory) {
 		startServer(requestFactory);
 
-		prepareResponse(response -> response.setHeader("Content-Type", "text/plain")
-				.setBody("Hello Spring!"));
+		prepareResponse(response ->
+				response.setHeader("Content-Type", "text/plain").setBody("Hello Spring!"));
 
 		RestClient headersClient = this.restClient.mutate()
 				.defaultRequest(request -> request.header("foo", "bar"))
@@ -925,8 +925,8 @@ class RestClientIntegrationTests {
 	void defaultRequestOverride(ClientHttpRequestFactory requestFactory) {
 		startServer(requestFactory);
 
-		prepareResponse(response -> response.setHeader("Content-Type", "text/plain")
-				.setBody("Hello Spring!"));
+		prepareResponse(response ->
+				response.setHeader("Content-Type", "text/plain").setBody("Hello Spring!"));
 
 		RestClient headersClient = this.restClient.mutate()
 				.defaultRequest(request -> request.accept(MediaType.APPLICATION_JSON))
@@ -948,8 +948,8 @@ class RestClientIntegrationTests {
 	void relativeUri(ClientHttpRequestFactory requestFactory) throws URISyntaxException {
 		startServer(requestFactory);
 
-		prepareResponse(response -> response.setHeader("Content-Type", "text/plain")
-				.setBody("Hello Spring!"));
+		prepareResponse(response ->
+				response.setHeader("Content-Type", "text/plain").setBody("Hello Spring!"));
 
 		URI uri = new URI(null, null, "/foo bar", null);
 
@@ -969,23 +969,28 @@ class RestClientIntegrationTests {
 	@ParameterizedRestClientTest
 	void cookieAddsCookie(ClientHttpRequestFactory requestFactory) {
 		startServer(requestFactory);
-		prepareResponse(response -> response.setHeader("Content-Type", "text/plain")
-				.setBody("Hello Spring!"));
+
+		prepareResponse(response ->
+				response.setHeader("Content-Type", "text/plain").setBody("Hello Spring!"));
 
 		this.restClient.get()
 				.uri("/greeting")
-				.cookie("foo", "bar")
+				.cookie("c1", "v1a")
+				.cookie("c1", "v1b")
+				.cookie("c2", "v2a")
 				.retrieve()
 				.body(String.class);
 
-		expectRequest(request -> assertThat(request.getHeader("Cookie")).isEqualTo("foo=bar"));
+		expectRequest(request -> assertThat(request.getHeader("Cookie")).isEqualTo("c1=v1a; c1=v1b; c2=v2a"));
 	}
 
 	@ParameterizedRestClientTest
 	void cookieOverridesDefaultCookie(ClientHttpRequestFactory requestFactory) {
 		startServer(requestFactory);
-		prepareResponse(response -> response.setHeader("Content-Type", "text/plain")
-				.setBody("Hello Spring!"));
+
+		prepareResponse(response ->
+				response.setHeader("Content-Type", "text/plain").setBody("Hello Spring!"));
+
 		RestClient restClientWithCookies = this.restClient.mutate()
 				.defaultCookie("testCookie", "firstValue", "secondValue")
 				.build();
@@ -1002,8 +1007,9 @@ class RestClientIntegrationTests {
 	@ParameterizedRestClientTest
 	void cookiesCanRemoveCookie(ClientHttpRequestFactory requestFactory) {
 		startServer(requestFactory);
-		prepareResponse(response -> response.setHeader("Content-Type", "text/plain")
-				.setBody("Hello Spring!"));
+
+		prepareResponse(response ->
+				response.setHeader("Content-Type", "text/plain").setBody("Hello Spring!"));
 
 		this.restClient.get()
 				.uri("/greeting")

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

@@ -319,18 +319,14 @@ final class DefaultWebClientBuilder implements WebClient.Builder {
 				.orElse(null) : null);
 
 		HttpHeaders defaultHeaders = copyDefaultHeaders();
-
 		MultiValueMap<String, String> defaultCookies = copyDefaultCookies();
 
-		return new DefaultWebClient(exchange,
-				filterFunctions,
-				initUriBuilderFactory(),
-				defaultHeaders,
-				defaultCookies,
+		return new DefaultWebClient(
+				exchange, filterFunctions,
+				initUriBuilderFactory(), defaultHeaders, defaultCookies,
 				this.defaultRequest,
 				this.statusHandlers,
-				this.observationRegistry,
-				this.observationConvention,
+				this.observationRegistry, this.observationConvention,
 				new DefaultWebClientBuilder(this));
 	}
 
@@ -374,26 +370,22 @@ final class DefaultWebClientBuilder implements WebClient.Builder {
 
 	@Nullable
 	private HttpHeaders copyDefaultHeaders() {
-		if (this.defaultHeaders != null) {
-			HttpHeaders copy = new HttpHeaders();
-			this.defaultHeaders.forEach((key, values) -> copy.put(key, new ArrayList<>(values)));
-			return HttpHeaders.readOnlyHttpHeaders(copy);
-		}
-		else {
+		if (this.defaultHeaders == null) {
 			return null;
 		}
+		HttpHeaders headers = new HttpHeaders();
+		this.defaultHeaders.forEach((key, values) -> headers.put(key, new ArrayList<>(values)));
+		return HttpHeaders.readOnlyHttpHeaders(headers);
 	}
 
 	@Nullable
 	private MultiValueMap<String, String> copyDefaultCookies() {
-		if (this.defaultCookies != null) {
-			MultiValueMap<String, String> copy = new LinkedMultiValueMap<>(this.defaultCookies.size());
-			this.defaultCookies.forEach((key, values) -> copy.put(key, new ArrayList<>(values)));
-			return CollectionUtils.unmodifiableMultiValueMap(copy);
-		}
-		else {
+		if (this.defaultCookies == null) {
 			return null;
 		}
+		MultiValueMap<String, String> map = new LinkedMultiValueMap<>(this.defaultCookies.size());
+		this.defaultCookies.forEach((key, values) -> map.put(key, new ArrayList<>(values)));
+		return CollectionUtils.unmodifiableMultiValueMap(map);
 	}
 
 }