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 + */ + protected RequestConfig createRequestConfig(CloseableHttpClient client) { + if (client instanceof Configurable) { + RequestConfig clientRequestConfig = ((Configurable) client).getConfig(); + return mergeRequestConfig(clientRequestConfig); + } + return this.requestConfig; + } + + private RequestConfig mergeRequestConfig(RequestConfig defaultRequestConfig) { + if (this.requestConfig == null) { // nothing to merge + return defaultRequestConfig; + } + RequestConfig.Builder builder = RequestConfig.copy(defaultRequestConfig); + int connectTimeout = this.requestConfig.getConnectTimeout(); + if (connectTimeout >= 0) { + builder.setConnectTimeout(connectTimeout); + } + int connectionRequestTimeout = this.requestConfig.getConnectionRequestTimeout(); + if (connectionRequestTimeout >= 0) { + builder.setConnectionRequestTimeout(connectionRequestTimeout); + } + int socketTimeout = this.requestConfig.getSocketTimeout(); + if (socketTimeout >= 0) { + builder.setSocketTimeout(socketTimeout); + } + return builder.build(); + } + /** * Create a Commons HttpMethodBase object for the given HTTP method and URI specification. * @param httpMethod the HTTP method diff --git a/spring-web/src/main/java/org/springframework/remoting/httpinvoker/HttpComponentsHttpInvokerRequestExecutor.java b/spring-web/src/main/java/org/springframework/remoting/httpinvoker/HttpComponentsHttpInvokerRequestExecutor.java index 68202e92d35..25430d7c7f0 100644 --- a/spring-web/src/main/java/org/springframework/remoting/httpinvoker/HttpComponentsHttpInvokerRequestExecutor.java +++ b/spring-web/src/main/java/org/springframework/remoting/httpinvoker/HttpComponentsHttpInvokerRequestExecutor.java @@ -28,6 +28,7 @@ import org.apache.http.NoHttpResponseException; import org.apache.http.StatusLine; import org.apache.http.client.HttpClient; import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.methods.Configurable; import org.apache.http.client.methods.HttpPost; import org.apache.http.config.Registry; import org.apache.http.config.RegistryBuilder; @@ -269,12 +270,39 @@ public class HttpComponentsHttpInvokerRequestExecutor extends AbstractHttpInvoke * 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. + *
The default implementation tries to merge the defaults of the client with the + * local customizations of the instance, if any. * @param config the HTTP invoker configuration that specifies the * target service * @return the RequestConfig to use */ protected RequestConfig createRequestConfig(HttpInvokerClientConfiguration config) { - return (this.requestConfig != null ? this.requestConfig : null); + HttpClient client = getHttpClient(); + if (client instanceof Configurable) { + RequestConfig clientRequestConfig = ((Configurable) client).getConfig(); + return mergeRequestConfig(clientRequestConfig); + } + return this.requestConfig; + } + + private RequestConfig mergeRequestConfig(RequestConfig defaultRequestConfig) { + if (this.requestConfig == null) { // nothing to merge + return defaultRequestConfig; + } + RequestConfig.Builder builder = RequestConfig.copy(defaultRequestConfig); + int connectTimeout = this.requestConfig.getConnectTimeout(); + if (connectTimeout >= 0) { + builder.setConnectTimeout(connectTimeout); + } + int connectionRequestTimeout = this.requestConfig.getConnectionRequestTimeout(); + if (connectionRequestTimeout >= 0) { + builder.setConnectionRequestTimeout(connectionRequestTimeout); + } + int socketTimeout = this.requestConfig.getSocketTimeout(); + if (socketTimeout >= 0) { + builder.setSocketTimeout(socketTimeout); + } + return builder.build(); } /** diff --git a/spring-web/src/test/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactoryTests.java b/spring-web/src/test/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactoryTests.java index 12efc4bc43f..6f5b800db3c 100644 --- a/spring-web/src/test/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactoryTests.java +++ b/spring-web/src/test/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactoryTests.java @@ -21,6 +21,7 @@ import java.net.URI; import org.apache.http.HttpEntityEnclosingRequest; import org.apache.http.client.HttpClient; import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.methods.Configurable; import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.client.protocol.HttpClientContext; import org.apache.http.impl.client.CloseableHttpClient; @@ -29,6 +30,7 @@ import org.junit.Test; import org.springframework.http.HttpMethod; import static org.junit.Assert.*; +import static org.mockito.Mockito.*; public class HttpComponentsClientHttpRequestFactoryTests extends AbstractHttpRequestFactoryTestCase { @@ -93,29 +95,80 @@ public class HttpComponentsClientHttpRequestFactoryTests extends AbstractHttpReq } @Test - public void defaultSettingsOfHttpClientLostOnExecutorCustomization() throws Exception { - CloseableHttpClient client = HttpClientBuilder.create() - .setDefaultRequestConfig(RequestConfig.custom().setConnectTimeout(1234).build()) - .build(); + public void defaultSettingsOfHttpClientMergedOnExecutorCustomization() throws Exception { + RequestConfig defaultConfig = RequestConfig.custom().setConnectTimeout(1234).build(); + CloseableHttpClient client = mock(CloseableHttpClient.class, + withSettings().extraInterfaces(Configurable.class)); + Configurable configurable = (Configurable) client; + when(configurable.getConfig()).thenReturn(defaultConfig); + HttpComponentsClientHttpRequestFactory hrf = new HttpComponentsClientHttpRequestFactory(client); - - URI uri = new URI(baseUrl + "/status/ok"); - HttpComponentsClientHttpRequest request = (HttpComponentsClientHttpRequest) - hrf.createRequest(uri, HttpMethod.GET); - - assertNull("No custom config should be set with a custom HttpClient", - request.getHttpContext().getAttribute(HttpClientContext.REQUEST_CONFIG)); + assertSame("Default client configuration is expected", defaultConfig, retrieveRequestConfig(hrf)); hrf.setConnectionRequestTimeout(4567); - HttpComponentsClientHttpRequest request2 = (HttpComponentsClientHttpRequest) - hrf.createRequest(uri, HttpMethod.GET); - Object requestConfigAttribute = request2.getHttpContext().getAttribute(HttpClientContext.REQUEST_CONFIG); - assertNotNull(requestConfigAttribute); - RequestConfig requestConfig = (RequestConfig) requestConfigAttribute; - + RequestConfig requestConfig = retrieveRequestConfig(hrf); + assertNotNull(requestConfig); assertEquals(4567, requestConfig.getConnectionRequestTimeout()); - // No way to access the request config of the HTTP client so no way to "merge" our customizations + // Default connection timeout merged + assertEquals(1234, requestConfig.getConnectTimeout()); + } + + @Test + public void localSettingsOverrideClientDefaultSettings() throws Exception { + RequestConfig defaultConfig = RequestConfig.custom() + .setConnectTimeout(1234).setConnectionRequestTimeout(6789).build(); + CloseableHttpClient client = mock(CloseableHttpClient.class, + withSettings().extraInterfaces(Configurable.class)); + Configurable configurable = (Configurable) client; + when(configurable.getConfig()).thenReturn(defaultConfig); + + HttpComponentsClientHttpRequestFactory hrf = new HttpComponentsClientHttpRequestFactory(client); + hrf.setConnectTimeout(5000); + + RequestConfig requestConfig = retrieveRequestConfig(hrf); + assertEquals(5000, requestConfig.getConnectTimeout()); + assertEquals(6789, requestConfig.getConnectionRequestTimeout()); + assertEquals(-1, requestConfig.getSocketTimeout()); + } + + @Test + public void mergeBasedOnCurrentHttpClient() throws Exception { + RequestConfig defaultConfig = RequestConfig.custom() + .setSocketTimeout(1234).build(); + final CloseableHttpClient client = mock(CloseableHttpClient.class, + withSettings().extraInterfaces(Configurable.class)); + Configurable configurable = (Configurable) client; + when(configurable.getConfig()).thenReturn(defaultConfig); + + HttpComponentsClientHttpRequestFactory hrf = new HttpComponentsClientHttpRequestFactory() { + @Override + public HttpClient getHttpClient() { + return client; + } + }; + hrf.setReadTimeout(5000); + + RequestConfig requestConfig = retrieveRequestConfig(hrf); assertEquals(-1, requestConfig.getConnectTimeout()); + assertEquals(-1, requestConfig.getConnectionRequestTimeout()); + assertEquals(5000, requestConfig.getSocketTimeout()); + + // Update the Http client so that it returns an updated config + RequestConfig updatedDefaultConfig = RequestConfig.custom() + .setConnectTimeout(1234).build(); + when(configurable.getConfig()).thenReturn(updatedDefaultConfig); + hrf.setReadTimeout(7000); + RequestConfig requestConfig2 = retrieveRequestConfig(hrf); + assertEquals(1234, requestConfig2.getConnectTimeout()); + assertEquals(-1, requestConfig2.getConnectionRequestTimeout()); + assertEquals(7000, requestConfig2.getSocketTimeout()); + } + + private RequestConfig retrieveRequestConfig(HttpComponentsClientHttpRequestFactory factory) throws Exception { + URI uri = new URI(baseUrl + "/status/ok"); + HttpComponentsClientHttpRequest request = (HttpComponentsClientHttpRequest) + factory.createRequest(uri, HttpMethod.GET); + return (RequestConfig) request.getHttpContext().getAttribute(HttpClientContext.REQUEST_CONFIG); } @Test diff --git a/spring-web/src/test/java/org/springframework/remoting/httpinvoker/HttpComponentsHttpInvokerRequestExecutorTests.java b/spring-web/src/test/java/org/springframework/remoting/httpinvoker/HttpComponentsHttpInvokerRequestExecutorTests.java index a0faf3c228a..1e95a0d7144 100644 --- a/spring-web/src/test/java/org/springframework/remoting/httpinvoker/HttpComponentsHttpInvokerRequestExecutorTests.java +++ b/spring-web/src/test/java/org/springframework/remoting/httpinvoker/HttpComponentsHttpInvokerRequestExecutorTests.java @@ -20,6 +20,7 @@ import java.io.IOException; import org.apache.http.client.HttpClient; import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.methods.Configurable; import org.apache.http.client.methods.HttpPost; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; @@ -90,23 +91,82 @@ public class HttpComponentsHttpInvokerRequestExecutorTests { } @Test - public void defaultSettingsOfHttpClientLostOnExecutorCustomization() throws IOException { - CloseableHttpClient client = HttpClientBuilder.create() - .setDefaultRequestConfig(RequestConfig.custom().setConnectTimeout(1234).build()) - .build(); + public void defaultSettingsOfHttpClientMergedOnExecutorCustomization() throws IOException { + RequestConfig defaultConfig = RequestConfig.custom().setConnectTimeout(1234).build(); + CloseableHttpClient client = mock(CloseableHttpClient.class, + withSettings().extraInterfaces(Configurable.class)); + Configurable configurable = (Configurable) client; + when(configurable.getConfig()).thenReturn(defaultConfig); + HttpComponentsHttpInvokerRequestExecutor executor = new HttpComponentsHttpInvokerRequestExecutor(client); - HttpInvokerClientConfiguration config = mockHttpInvokerClientConfiguration("http://fake-service"); HttpPost httpPost = executor.createHttpPost(config); - assertNull("No custom config should be set with a custom HttpClient", httpPost.getConfig()); + assertSame("Default client configuration is expected", defaultConfig, httpPost.getConfig()); executor.setConnectionRequestTimeout(4567); HttpPost httpPost2 = executor.createHttpPost(config); assertNotNull(httpPost2.getConfig()); assertEquals(4567, httpPost2.getConfig().getConnectionRequestTimeout()); - // No way to access the request config of the HTTP client so no way to "merge" our customizations - assertEquals(-1, httpPost2.getConfig().getConnectTimeout()); + // Default connection timeout merged + assertEquals(1234, httpPost2.getConfig().getConnectTimeout()); + } + + @Test + public void localSettingsOverrideClientDefaultSettings() throws Exception { + RequestConfig defaultConfig = RequestConfig.custom() + .setConnectTimeout(1234).setConnectionRequestTimeout(6789).build(); + CloseableHttpClient client = mock(CloseableHttpClient.class, + withSettings().extraInterfaces(Configurable.class)); + Configurable configurable = (Configurable) client; + when(configurable.getConfig()).thenReturn(defaultConfig); + + HttpComponentsHttpInvokerRequestExecutor executor = + new HttpComponentsHttpInvokerRequestExecutor(client); + executor.setConnectTimeout(5000); + + HttpInvokerClientConfiguration config = mockHttpInvokerClientConfiguration("http://fake-service"); + HttpPost httpPost = executor.createHttpPost(config); + RequestConfig requestConfig = httpPost.getConfig(); + assertEquals(5000, requestConfig.getConnectTimeout()); + assertEquals(6789, requestConfig.getConnectionRequestTimeout()); + assertEquals(-1, requestConfig.getSocketTimeout()); + } + + @Test + public void mergeBasedOnCurrentHttpClient() throws Exception { + RequestConfig defaultConfig = RequestConfig.custom() + .setSocketTimeout(1234).build(); + final CloseableHttpClient client = mock(CloseableHttpClient.class, + withSettings().extraInterfaces(Configurable.class)); + Configurable configurable = (Configurable) client; + when(configurable.getConfig()).thenReturn(defaultConfig); + + HttpComponentsHttpInvokerRequestExecutor executor = + new HttpComponentsHttpInvokerRequestExecutor() { + @Override + public HttpClient getHttpClient() { + return client; + } + }; + executor.setReadTimeout(5000); + HttpInvokerClientConfiguration config = mockHttpInvokerClientConfiguration("http://fake-service"); + HttpPost httpPost = executor.createHttpPost(config); + RequestConfig requestConfig = httpPost.getConfig(); + assertEquals(-1, requestConfig.getConnectTimeout()); + assertEquals(-1, requestConfig.getConnectionRequestTimeout()); + assertEquals(5000, requestConfig.getSocketTimeout()); + + // Update the Http client so that it returns an updated config + RequestConfig updatedDefaultConfig = RequestConfig.custom() + .setConnectTimeout(1234).build(); + when(configurable.getConfig()).thenReturn(updatedDefaultConfig); + executor.setReadTimeout(7000); + HttpPost httpPost2 = executor.createHttpPost(config); + RequestConfig requestConfig2 = httpPost2.getConfig(); + assertEquals(1234, requestConfig2.getConnectTimeout()); + assertEquals(-1, requestConfig2.getConnectionRequestTimeout()); + assertEquals(7000, requestConfig2.getSocketTimeout()); } @Test