Polish upgrade to HttpComponents 4.3

Polish fbe6051 to make explicit the fact that a null RequestConfig
can be used to force the defaults of the current HttpClient.

Issue: SPR-11113

Author: snicoll

spring-web/src/main/java/org/springframework/remoting/httpinvoker/HttpComponentsHttpInvokerRequestExecutor.java

@@ -56,6 +56,7 @@
  * <p>As of Spring 4.1, this request executor requires Apache HttpComponents 4.3 or higher.
  *
  * @author Juergen Hoeller
+ * @author Stephane Nicoll
  * @since 3.1
  * @see org.springframework.remoting.httpinvoker.SimpleHttpInvokerRequestExecutor
  */
@@ -213,7 +214,10 @@ protected RemoteInvocationResult doExecuteRequest(
 	 */
 	protected HttpPost createHttpPost(HttpInvokerClientConfiguration config) throws IOException {
 		HttpPost httpPost = new HttpPost(config.getServiceUrl());
-		httpPost.setConfig(createRequestConfig(config, httpPost));
+		RequestConfig requestConfig = createRequestConfig(config);
+		if (requestConfig != null) {
+			httpPost.setConfig(requestConfig);
+		}
 		LocaleContext localeContext = LocaleContextHolder.getLocaleContext();
 		if (localeContext != null) {
 			Locale locale = localeContext.getLocale();
@@ -228,13 +232,14 @@ protected HttpPost createHttpPost(HttpInvokerClientConfiguration config) throws
 	}
 
 	/**
-	 * Create a {@link RequestConfig} for the given configuration and {@link HttpPost}.
+	 * Create a {@link RequestConfig} for the given configuration. Can return {@code null}
+	 * to indicate that no custom request config should be set and the defaults of the
+	 * {@link HttpClient} should be used.
 	 * @param config the HTTP invoker configuration that specifies the
 	 * target service
-	 * @param httpPost the HttpPost instance
-	 * @return the RequestConfig to use for that HttpPost
+	 * @return the RequestConfig to use
 	 */
-	protected RequestConfig createRequestConfig(HttpInvokerClientConfiguration config, HttpPost httpPost) {
+	protected RequestConfig createRequestConfig(HttpInvokerClientConfiguration config) {
 		if (this.connectionTimeout > 0 || this.readTimeout > 0) {
 			return RequestConfig.custom()
 					.setConnectTimeout(this.connectionTimeout)

spring-web/src/test/java/org/springframework/remoting/httpinvoker/HttpComponentsHttpInvokerRequestExecutorTests.java

@@ -2,7 +2,11 @@
 
 import java.io.IOException;
 
+import org.apache.http.client.HttpClient;
+import org.apache.http.client.config.RequestConfig;
 import org.apache.http.client.methods.HttpPost;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
 import org.junit.Test;
 
 import static org.junit.Assert.*;
@@ -14,6 +18,21 @@
  */
 public class HttpComponentsHttpInvokerRequestExecutorTests {
 
+	@SuppressWarnings("deprecation")
+	@Test
+	public void assertLegacyCustomConfig() {
+		HttpClient httpClient = new org.apache.http.impl.client.DefaultHttpClient(); // Does not support RequestConfig
+		HttpComponentsHttpInvokerRequestExecutor executor = new HttpComponentsHttpInvokerRequestExecutor(httpClient);
+
+		executor.setConnectTimeout(1234);
+		assertEquals(1234, httpClient.getParams().getIntParameter(
+				org.apache.http.params.CoreConnectionPNames.CONNECTION_TIMEOUT, 0));
+
+		executor.setReadTimeout(4567);
+		assertEquals(4567, httpClient.getParams().getIntParameter(
+				org.apache.http.params.CoreConnectionPNames.SO_TIMEOUT, 0));
+	}
+
 	@Test
 	public void customizeConnectionTimeout() throws IOException {
 		HttpComponentsHttpInvokerRequestExecutor executor = new HttpComponentsHttpInvokerRequestExecutor();
@@ -34,6 +53,21 @@ public void customizeReadTimeout() throws IOException {
 		assertEquals(10000, httpPost.getConfig().getSocketTimeout());
 	}
 
+	@Test
+	public void ignoreFactorySettings() throws IOException {
+		CloseableHttpClient httpClient = HttpClientBuilder.create().build();
+		HttpComponentsHttpInvokerRequestExecutor executor = new HttpComponentsHttpInvokerRequestExecutor(httpClient) {
+			@Override
+			protected RequestConfig createRequestConfig(HttpInvokerClientConfiguration config) {
+				return null;
+			}
+		};
+
+		HttpInvokerClientConfiguration config = mockHttpInvokerClientConfiguration("http://fake-service");
+		HttpPost httpPost = executor.createHttpPost(config);
+		assertNull("custom request config should not be set", httpPost.getConfig());
+	}
+
 	private HttpInvokerClientConfiguration mockHttpInvokerClientConfiguration(String serviceUrl) {
 		HttpInvokerClientConfiguration config = mock(HttpInvokerClientConfiguration.class);
 		when(config.getServiceUrl()).thenReturn(serviceUrl);