Allow ExchangeStrategies customizations in WebClient

Prior to this commit, developers could configure their WebClient to use
their custom `ExchangeStrategies`, by providing it in the
`WebClient.Builder` chain.
Once created, an `ExchangeStrategies` instance is not mutable, which
makes it hard for further customizations by other components. In the
case of the reported issue, other components would override the default
configuration for the codecs maxInMemorySize.

This commit makes the `ExchangeStrategies` mutable and uses that fact to
further customize them with a new `WebClient.Builder#exchangeStrategies`
`Consumer` variant. This commit is also deprecating those mutating
variants in favor of a new `WebClient.Builder#exchangeStrategies` that
takes a `ExchangeStrategies#Builder` directly and avoids mutation issues
altogether.

Closes gh-24106

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -42,13 +42,18 @@ final class DefaultExchangeStrategiesBuilder implements ExchangeStrategies.Build
 	}
 
 
-	private final ClientCodecConfigurer codecConfigurer = ClientCodecConfigurer.create();
+	private final ClientCodecConfigurer codecConfigurer;
 
 
 	public DefaultExchangeStrategiesBuilder() {
+		this.codecConfigurer = ClientCodecConfigurer.create();
 		this.codecConfigurer.registerDefaults(false);
 	}
 
+	private DefaultExchangeStrategiesBuilder(DefaultExchangeStrategies other) {
+		this.codecConfigurer = other.codecConfigurer.clone();
+	}
+
 
 	public void defaultConfiguration() {
 		this.codecConfigurer.registerDefaults(true);
@@ -62,21 +67,23 @@ final class DefaultExchangeStrategiesBuilder implements ExchangeStrategies.Build
 
 	@Override
 	public ExchangeStrategies build() {
-		return new DefaultExchangeStrategies(
-				this.codecConfigurer.getReaders(), this.codecConfigurer.getWriters());
+		return new DefaultExchangeStrategies(this.codecConfigurer);
 	}
 
 
 	private static class DefaultExchangeStrategies implements ExchangeStrategies {
 
+		private final ClientCodecConfigurer codecConfigurer;
+
 		private final List<HttpMessageReader<?>> readers;
 
 		private final List<HttpMessageWriter<?>> writers;
 
 
-		public DefaultExchangeStrategies(List<HttpMessageReader<?>> readers, List<HttpMessageWriter<?>> writers) {
-			this.readers = unmodifiableCopy(readers);
-			this.writers = unmodifiableCopy(writers);
+		public DefaultExchangeStrategies(ClientCodecConfigurer codecConfigurer) {
+			this.codecConfigurer = codecConfigurer;
+			this.readers = unmodifiableCopy(this.codecConfigurer.getReaders());
+			this.writers = unmodifiableCopy(this.codecConfigurer.getWriters());
 		}
 
 		private static <T> List<T> unmodifiableCopy(List<? extends T> list) {
@@ -84,6 +91,12 @@ final class DefaultExchangeStrategiesBuilder implements ExchangeStrategies.Build
 		}
 
 
+		@Override
+		@Deprecated
+		public Builder mutate() {
+			return new DefaultExchangeStrategiesBuilder(this);
+		}
+
 		@Override
 		public List<HttpMessageReader<?>> messageReaders() {
 			return this.readers;

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

@@ -38,6 +38,7 @@ import org.springframework.web.util.UriBuilderFactory;
  * Default implementation of {@link WebClient.Builder}.
  *
  * @author Rossen Stoyanchev
+ * @author Brian Clozel
  * @since 5.0
  */
 final class DefaultWebClientBuilder implements WebClient.Builder {
@@ -66,14 +67,16 @@ final class DefaultWebClientBuilder implements WebClient.Builder {
 	@Nullable
 	private ClientHttpConnector connector;
 
-	private ExchangeStrategies exchangeStrategies;
+	@Nullable
+	private ExchangeStrategies.Builder exchangeStrategies;
+
+	private List<Consumer<ExchangeStrategies.Builder>> strategiesConfigurers;
 
 	@Nullable
 	private ExchangeFunction exchangeFunction;
 
 
 	public DefaultWebClientBuilder() {
-		this.exchangeStrategies = ExchangeStrategies.withDefaults();
 	}
 
 	public DefaultWebClientBuilder(DefaultWebClientBuilder other) {
@@ -190,12 +193,26 @@ final class DefaultWebClientBuilder implements WebClient.Builder {
 	}
 
 	@Override
+	@Deprecated
 	public WebClient.Builder exchangeStrategies(ExchangeStrategies strategies) {
+		Assert.notNull(strategies, "ExchangeStrategies must not be null");
+		this.exchangeStrategies = strategies.mutate();
+		return this;
+	}
+
+	@Override
+	public WebClient.Builder exchangeStrategies(ExchangeStrategies.Builder strategies) {
 		Assert.notNull(strategies, "ExchangeStrategies must not be null");
 		this.exchangeStrategies = strategies;
 		return this;
 	}
 
+	@Override
+	public WebClient.Builder exchangeStrategies(Consumer<ExchangeStrategies.Builder> configurer) {
+		this.strategiesConfigurers.add(configurer);
+		return this;
+	}
+
 	@Override
 	public WebClient.Builder exchangeFunction(ExchangeFunction exchangeFunction) {
 		this.exchangeFunction = exchangeFunction;
@@ -231,13 +248,26 @@ final class DefaultWebClientBuilder implements WebClient.Builder {
 			return this.exchangeFunction;
 		}
 		else if (this.connector != null) {
-			return ExchangeFunctions.create(this.connector, this.exchangeStrategies);
+			return ExchangeFunctions.create(this.connector, initExchangeStrategies());
 		}
 		else {
-			return ExchangeFunctions.create(new ReactorClientHttpConnector(), this.exchangeStrategies);
+			return ExchangeFunctions.create(new ReactorClientHttpConnector(), initExchangeStrategies());
 		}
 	}
 
+	@SuppressWarnings("deprecation")
+	private ExchangeStrategies initExchangeStrategies() {
+		if (CollectionUtils.isEmpty(this.strategiesConfigurers)) {
+			return this.exchangeStrategies != null ? this.exchangeStrategies.build() : ExchangeStrategies.withDefaults();
+		}
+
+		ExchangeStrategies.Builder builder =
+				this.exchangeStrategies != null ? this.exchangeStrategies : ExchangeStrategies.builder();
+
+		this.strategiesConfigurers.forEach(configurer -> configurer.accept(builder));
+		return builder.build();
+	}
+
 	private UriBuilderFactory initUriBuilderFactory() {
 		if (this.uriBuilderFactory != null) {
 			return this.uriBuilderFactory;

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

@@ -47,6 +47,18 @@ public interface ExchangeStrategies {
 	 */
 	List<HttpMessageWriter<?>> messageWriters();
 
+	/**
+	 * Return a builder to create a new {@link ExchangeStrategies} instance
+	 * replicated from the current instance.
+	 * @since 5.1.12
+	 * @deprecated APIs should consume {@link ExchangeStrategies} as final or accept an
+	 * {@link ExchangeStrategies.Builder builder}.
+	 */
+	@Deprecated
+	default Builder mutate() {
+		throw new UnsupportedOperationException("This ExchangeStrategies implementation does not support mutation.");
+	}
+
 
 	// Static builder methods
 

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

@@ -64,6 +64,7 @@ import org.springframework.web.util.UriBuilderFactory;
  *
  * @author Rossen Stoyanchev
  * @author Arjen Poutsma
+ * @author Brian Clozel
  * @since 5.0
  */
 public interface WebClient {
@@ -288,12 +289,35 @@ public interface WebClient {
 		Builder clientConnector(ClientHttpConnector connector);
 
 		/**
-		 * Configure the {@link ExchangeStrategies} to use.
-		 * <p>By default this is obtained from {@link ExchangeStrategies#withDefaults()}.
+		 * Provide the {@link ExchangeStrategies} to use.
+		 * <p>This is useful for changing the default settings, yet still allowing
+		 * further customizations via {@link #exchangeStrategies(Consumer)}.
+		 * If not set, defaults are obtained from {@link ExchangeStrategies#withDefaults()}.
 		 * @param strategies the strategies to use
+		 * @deprecated as of 5.1, in favor of {@link #exchangeStrategies(ExchangeStrategies.Builder)}
 		 */
+		@Deprecated
 		Builder exchangeStrategies(ExchangeStrategies strategies);
 
+		/**
+		 * Provide the {@link ExchangeStrategies.Builder} to use.
+		 * <p>This is useful for changing the default settings, yet still allowing
+		 * further customizations via {@link #exchangeStrategies(Consumer)}.
+		 * If not set, defaults are obtained from {@link ExchangeStrategies#builder()}.
+		 * @param strategies the strategies to use
+		 * @since 5.1.12
+		 */
+		Builder exchangeStrategies(ExchangeStrategies.Builder strategies);
+
+		/**
+		 * Customize the {@link ExchangeStrategies}.
+		 * <p>Allows further customization on {@link ExchangeStrategies},
+		 * mutating them if they were {@link #exchangeStrategies(ExchangeStrategies) set},
+		 * or starting from {@link ExchangeStrategies#withDefaults() defaults}.
+		 * @since 5.1.12
+		 */
+		Builder exchangeStrategies(Consumer<ExchangeStrategies.Builder> configurer);
+
 		/**
 		 * Provide an {@link ExchangeFunction} pre-configured with
 		 * {@link ClientHttpConnector} and {@link ExchangeStrategies}.

spring-webflux/src/test/java/org/springframework/web/reactive/function/client/ExchangeStrategiesTests.java

@@ -39,4 +39,15 @@ public class ExchangeStrategiesTests {
 		assertFalse(strategies.messageWriters().isEmpty());
 	}
 
+	@Test
+	@SuppressWarnings("deprecation")
+	public void mutate() {
+		ExchangeStrategies strategies = ExchangeStrategies.empty().build();
+		assertTrue(strategies.messageReaders().isEmpty());
+		assertTrue(strategies.messageWriters().isEmpty());
+		ExchangeStrategies mutated = strategies.mutate().codecs(codecs -> codecs.registerDefaults(true)).build();
+		assertFalse(mutated.messageReaders().isEmpty());
+		assertFalse(mutated.messageWriters().isEmpty());
+	}
+
 }