Allow default settings of a custom HttpAsyncClient to apply

This is a rework of 71783c5 for SPR-12540 for the async extension that
was not merging the internal RequestConfig as it should.

Issue: SPR-13125

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -76,6 +76,10 @@ final class HttpComponentsAsyncClientHttpRequest extends AbstractBufferingAsyncC
 		return this.httpRequest.getURI();
 	}
 
+	HttpContext getHttpContext() {
+		return this.httpContext;
+	}
+
 	@Override
 	protected ListenableFuture<ClientHttpResponse> executeInternal(HttpHeaders headers, byte[] bufferedOutput)
 			throws IOException {

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

@@ -40,6 +40,7 @@ import org.springframework.util.Assert;
  * HttpAsyncClient 4.0</a> to create requests.
  *
  * @author Arjen Poutsma
+ * @author Stephane Nicoll
  * @since 4.0
  * @see HttpAsyncClient
  */
@@ -122,21 +123,41 @@ public class HttpComponentsAsyncClientHttpRequestFactory extends HttpComponentsC
         if (context == null) {
             context = HttpClientContext.create();
         }
-        // Request configuration not set in the context
-        if (context.getAttribute(HttpClientContext.REQUEST_CONFIG) == null) {
-            // Use request configuration given by the user, when available
-            RequestConfig config = null;
-            if (httpRequest instanceof Configurable) {
-                config = ((Configurable) httpRequest).getConfig();
-            }
-            if (config == null) {
-                config = RequestConfig.DEFAULT;
-            }
-            context.setAttribute(HttpClientContext.REQUEST_CONFIG, config);
-        }
+		// Request configuration not set in the context
+		if (context.getAttribute(HttpClientContext.REQUEST_CONFIG) == null) {
+			// Use request configuration given by the user, when available
+			RequestConfig config = null;
+			if (httpRequest instanceof Configurable) {
+				config = ((Configurable) httpRequest).getConfig();
+			}
+			if (config == null) {
+				config = createRequestConfig(asyncClient);
+			}
+			if (config != null) {
+				context.setAttribute(HttpClientContext.REQUEST_CONFIG, config);
+			}
+		}
 		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

@@ -245,7 +245,7 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
 		return this.requestConfig;
 	}
 
-	private RequestConfig mergeRequestConfig(RequestConfig defaultRequestConfig) {
+	protected RequestConfig mergeRequestConfig(RequestConfig defaultRequestConfig) {
 		if (this.requestConfig == null) { // nothing to merge
 			return defaultRequestConfig;
 		}
@@ -265,6 +265,10 @@ 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

spring-web/src/test/java/org/springframework/http/client/HttpComponentsAsyncClientHttpRequestFactoryTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -16,12 +16,21 @@
 
 package org.springframework.http.client;
 
+import java.net.URI;
+
+import org.apache.http.client.config.RequestConfig;
+import org.apache.http.client.protocol.HttpClientContext;
+import org.apache.http.impl.nio.client.CloseableHttpAsyncClient;
+import org.apache.http.impl.nio.client.HttpAsyncClientBuilder;
 import org.junit.Test;
 
 import org.springframework.http.HttpMethod;
 
+import static org.junit.Assert.*;
+
 /**
  * @author Arjen Poutsma
+ * @author Stephane Nicoll
  */
 public class HttpComponentsAsyncClientHttpRequestFactoryTests extends AbstractAsyncHttpRequestFactoryTestCase {
 
@@ -38,4 +47,43 @@ public class HttpComponentsAsyncClientHttpRequestFactoryTests extends AbstractAs
 		assertHttpMethod("patch", HttpMethod.PATCH);
 	}
 
+	@Test
+	public void customHttpAsyncClientUsesItsDefault() throws Exception {
+		HttpComponentsAsyncClientHttpRequestFactory factory =
+				new HttpComponentsAsyncClientHttpRequestFactory();
+
+		URI uri = new URI(baseUrl + "/status/ok");
+		HttpComponentsAsyncClientHttpRequest request = (HttpComponentsAsyncClientHttpRequest)
+				factory.createAsyncRequest(uri, HttpMethod.GET);
+
+		assertNull("No custom config should be set with a custom HttpAsyncClient",
+				request.getHttpContext().getAttribute(HttpClientContext.REQUEST_CONFIG));
+	}
+
+	@Test
+	public void defaultSettingsOfHttpAsyncClientLostOnExecutorCustomization() throws Exception {
+		CloseableHttpAsyncClient client = HttpAsyncClientBuilder.create()
+				.setDefaultRequestConfig(RequestConfig.custom().setConnectTimeout(1234).build())
+				.build();
+		HttpComponentsAsyncClientHttpRequestFactory factory = new HttpComponentsAsyncClientHttpRequestFactory(client);
+
+		URI uri = new URI(baseUrl + "/status/ok");
+		HttpComponentsAsyncClientHttpRequest request = (HttpComponentsAsyncClientHttpRequest)
+				factory.createAsyncRequest(uri, HttpMethod.GET);
+
+		assertNull("No custom config should be set with a custom HttpClient",
+				request.getHttpContext().getAttribute(HttpClientContext.REQUEST_CONFIG));
+
+		factory.setConnectionRequestTimeout(4567);
+		HttpComponentsAsyncClientHttpRequest request2 = (HttpComponentsAsyncClientHttpRequest)
+				factory.createAsyncRequest(uri, HttpMethod.GET);
+		Object requestConfigAttribute = request2.getHttpContext().getAttribute(HttpClientContext.REQUEST_CONFIG);
+		assertNotNull(requestConfigAttribute);
+		RequestConfig requestConfig = (RequestConfig) requestConfigAttribute;
+
+		assertEquals(4567, requestConfig.getConnectionRequestTimeout());
+		// No way to access the request config of the HTTP client so no way to "merge" our customizations
+		assertEquals(-1, requestConfig.getConnectTimeout());
+	}
+
 }