Unified createRequestConfig(Object) method, avoiding getInternalRequestConfig()

Issue: SPR-13125

spring-web/src/main/java/org/springframework/http/client/HttpComponentsAsyncClientHttpRequestFactory.java

@@ -65,7 +65,7 @@ public class HttpComponentsAsyncClientHttpRequestFactory extends HttpComponentsC
 	 */
 	public HttpComponentsAsyncClientHttpRequestFactory(CloseableHttpAsyncClient httpAsyncClient) {
 		super();
-		Assert.notNull(httpAsyncClient, "'httpAsyncClient' must not be null");
+		Assert.notNull(httpAsyncClient, "HttpAsyncClient must not be null");
 		this.httpAsyncClient = httpAsyncClient;
 	}
 
@@ -79,7 +79,7 @@ public class HttpComponentsAsyncClientHttpRequestFactory extends HttpComponentsC
 			CloseableHttpClient httpClient, CloseableHttpAsyncClient httpAsyncClient) {
 
 		super(httpClient);
-		Assert.notNull(httpAsyncClient, "'httpAsyncClient' must not be null");
+		Assert.notNull(httpAsyncClient, "HttpAsyncClient must not be null");
 		this.httpAsyncClient = httpAsyncClient;
 	}
 
@@ -140,24 +140,6 @@ public class HttpComponentsAsyncClientHttpRequestFactory extends HttpComponentsC
 		return new HttpComponentsAsyncClientHttpRequest(asyncClient, httpRequest, context);
 	}
 
-	/**
-	 * Create a default {@link RequestConfig} to use with the given client.
-	 * Can return {@code null} to indicate that no custom request config should
-	 * be set and the defaults of the {@link HttpAsyncClient} should be used.
-	 * <p>The default implementation tries to merge the defaults of the client
-	 * with the local customizations of this instance, if any.
-	 * @param client the client
-	 * @return the RequestConfig to use
-	 * @since 4.2
-	 */
-	protected RequestConfig createRequestConfig(HttpAsyncClient client) {
-		if (client instanceof Configurable) {
-			RequestConfig clientRequestConfig = ((Configurable) client).getConfig();
-			return mergeRequestConfig(clientRequestConfig);
-		}
-		return getInternalRequestConfig();
-	}
-
 	@Override
 	public void destroy() throws Exception {
 		try {

spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactory.java

@@ -53,6 +53,7 @@ import org.springframework.util.Assert;
  * @author Oleg Kalnichevski
  * @author Arjen Poutsma
  * @author Stephane Nicoll
+ * @author Juergen Hoeller
  * @since 3.1
  */
 public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequestFactory, DisposableBean {
@@ -109,8 +110,7 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
 	 */
 	public void setConnectTimeout(int timeout) {
 		Assert.isTrue(timeout >= 0, "Timeout must be a non-negative value");
-		this.requestConfig = cloneRequestConfig()
+		this.requestConfig = requestConfigBuilder().setConnectTimeout(timeout).build();
-				.setConnectTimeout(timeout).build();
 		setLegacyConnectionTimeout(getHttpClient(), timeout);
 	}
 
@@ -131,8 +131,7 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
 	@SuppressWarnings("deprecation")
 	private void setLegacyConnectionTimeout(HttpClient client, int timeout) {
 		if (org.apache.http.impl.client.AbstractHttpClient.class.isInstance(client)) {
-			client.getParams().setIntParameter(
+			client.getParams().setIntParameter(org.apache.http.params.CoreConnectionPNames.CONNECTION_TIMEOUT, timeout);
-					org.apache.http.params.CoreConnectionPNames.CONNECTION_TIMEOUT, timeout);
 		}
 	}
 
@@ -146,8 +145,7 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
 	 * @see RequestConfig#getConnectionRequestTimeout()
 	 */
 	public void setConnectionRequestTimeout(int connectionRequestTimeout) {
-		this.requestConfig = cloneRequestConfig()
+		this.requestConfig = requestConfigBuilder().setConnectionRequestTimeout(connectionRequestTimeout).build();
-				.setConnectionRequestTimeout(connectionRequestTimeout).build();
 	}
 
 	/**
@@ -160,8 +158,7 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
 	 */
 	public void setReadTimeout(int timeout) {
 		Assert.isTrue(timeout >= 0, "Timeout must be a non-negative value");
-		this.requestConfig = cloneRequestConfig()
+		this.requestConfig = requestConfigBuilder().setSocketTimeout(timeout).build();
-				.setSocketTimeout(timeout).build();
 		setLegacySocketTimeout(getHttpClient(), timeout);
 	}
 
@@ -175,15 +172,10 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
 	@SuppressWarnings("deprecation")
 	private void setLegacySocketTimeout(HttpClient client, int timeout) {
 		if (org.apache.http.impl.client.AbstractHttpClient.class.isInstance(client)) {
-			client.getParams().setIntParameter(
+			client.getParams().setIntParameter(org.apache.http.params.CoreConnectionPNames.SO_TIMEOUT, timeout);
-					org.apache.http.params.CoreConnectionPNames.SO_TIMEOUT, timeout);
 		}
 	}
 
-	private RequestConfig.Builder cloneRequestConfig() {
-		return this.requestConfig != null ? RequestConfig.copy(this.requestConfig) : RequestConfig.custom();
-	}
-
 	/**
 	 * Indicates whether this request factory should buffer the request body internally.
 	 * <p>Default is {@code true}. When sending large amounts of data via POST or PUT, it is
@@ -226,17 +218,27 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
 		}
 	}
 
+
+	/**
+	 * Return a builder for modifying the factory-level {@link RequestConfig}.
+	 * @since 4.2
+	 */
+	private RequestConfig.Builder requestConfigBuilder() {
+		return (this.requestConfig != null ? RequestConfig.copy(this.requestConfig) : RequestConfig.custom());
+	}
+
 	/**
 	 * Create a default {@link RequestConfig} to use with the given client.
 	 * Can return {@code null} to indicate that no custom request config should
 	 * be set and the defaults of the {@link HttpClient} should be used.
 	 * <p>The default implementation tries to merge the defaults of the client
-	 * with the local customizations of this instance, if any.
+	 * with the local customizations of this factory instance, if any.
-	 * @param client the client
+	 * @param client the {@link HttpClient} (or {@code HttpAsyncClient}) to check
-	 * @return the RequestConfig to use
+	 * @return the actual RequestConfig to use (may be {@code null})
 	 * @since 4.2
+	 * @see #mergeRequestConfig(RequestConfig)
 	 */
-	protected RequestConfig createRequestConfig(HttpClient client) {
+	protected RequestConfig createRequestConfig(Object client) {
 		if (client instanceof Configurable) {
 			RequestConfig clientRequestConfig = ((Configurable) client).getConfig();
 			return mergeRequestConfig(clientRequestConfig);
@@ -244,11 +246,20 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
 		return this.requestConfig;
 	}
 
-	protected RequestConfig mergeRequestConfig(RequestConfig defaultRequestConfig) {
+	/**
-		if (this.requestConfig == null) { // nothing to merge
+	 * Merge the given {@link HttpClient}-level {@link RequestConfig} with
-			return defaultRequestConfig;
+	 * the factory-level {@link RequestConfig}, if necessary.
+	 * @param clientConfig the config held by the current
+	 * @return the merged request config
+	 * (may be {@code null} if the given client config is {@code null})
+	 * @since 4.2
+	 */
+	protected RequestConfig mergeRequestConfig(RequestConfig clientConfig) {
+		if (this.requestConfig == null) {  // nothing to merge
+			return clientConfig;
 		}
-		RequestConfig.Builder builder = RequestConfig.copy(defaultRequestConfig);
+
+		RequestConfig.Builder builder = RequestConfig.copy(clientConfig);
 		int connectTimeout = this.requestConfig.getConnectTimeout();
 		if (connectTimeout >= 0) {
 			builder.setConnectTimeout(connectTimeout);
@@ -264,10 +275,6 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
 		return builder.build();
 	}
 
-	protected final RequestConfig getInternalRequestConfig() {
-		return this.requestConfig;
-	}
-
 	/**
 	 * Create a Commons HttpMethodBase object for the given HTTP method and URI specification.
 	 * @param httpMethod the HTTP method