Allow default settings of a custom HttpClient to apply
Previously the default settings of a custom HttpClient were always ignored since a RequestConfig instance was always set no matter if some customizations were applied or not. This commit keeps an internal RequestConfig object instance that is only initialized if the user applies a customization. If he does not, the default settings of the HttpClient are used as it should. Note that if the HttpComponents API exposed the default RequestConfig of a given HttpClient, we would be able to merge our customizations with the one specified by the client. Unfortunately, such API does not exist and the "defaultSettingsOfHttpClientLostOnExecutorCustomization" test illustrates that limitation. Issue: SPR-12540
This commit is contained in:
parent
961574bd17
commit
71783c5d86
|
|
@ -59,11 +59,7 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
|
|||
|
||||
private CloseableHttpClient httpClient;
|
||||
|
||||
private int connectTimeout;
|
||||
|
||||
private int connectionRequestTimeout;
|
||||
|
||||
private int socketTimeout;
|
||||
private RequestConfig requestConfig;
|
||||
|
||||
private boolean bufferRequestBody = true;
|
||||
|
||||
|
|
@ -111,11 +107,15 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
|
|||
/**
|
||||
* Set the connection timeout for the underlying HttpClient.
|
||||
* A timeout value of 0 specifies an infinite timeout.
|
||||
* <p>Additional properties can be configured by specifying a
|
||||
* {@link RequestConfig} instance on a custom {@link HttpClient}.
|
||||
* @param timeout the timeout value in milliseconds
|
||||
* @see RequestConfig#getConnectTimeout()
|
||||
*/
|
||||
public void setConnectTimeout(int timeout) {
|
||||
Assert.isTrue(timeout >= 0, "Timeout must be a non-negative value");
|
||||
this.connectTimeout = timeout;
|
||||
this.requestConfig = cloneRequestConfig()
|
||||
.setConnectTimeout(timeout).build();
|
||||
setLegacyConnectionTimeout(getHttpClient(), timeout);
|
||||
}
|
||||
|
||||
|
|
@ -145,20 +145,28 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
|
|||
* Set the timeout in milliseconds used when requesting a connection from the connection
|
||||
* manager using the underlying HttpClient.
|
||||
* A timeout value of 0 specifies an infinite timeout.
|
||||
* <p>Additional properties can be configured by specifying a
|
||||
* {@link RequestConfig} instance on a custom {@link HttpClient}.
|
||||
* @param connectionRequestTimeout the timeout value to request a connection in milliseconds
|
||||
* @see RequestConfig#getConnectionRequestTimeout()
|
||||
*/
|
||||
public void setConnectionRequestTimeout(int connectionRequestTimeout) {
|
||||
this.connectionRequestTimeout = connectionRequestTimeout;
|
||||
this.requestConfig = cloneRequestConfig()
|
||||
.setConnectionRequestTimeout(connectionRequestTimeout).build();
|
||||
}
|
||||
|
||||
/**
|
||||
* Set the socket read timeout for the underlying HttpClient.
|
||||
* A timeout value of 0 specifies an infinite timeout.
|
||||
* <p>Additional properties can be configured by specifying a
|
||||
* {@link RequestConfig} instance on a custom {@link HttpClient}.
|
||||
* @param timeout the timeout value in milliseconds
|
||||
* @see RequestConfig#getSocketTimeout()
|
||||
*/
|
||||
public void setReadTimeout(int timeout) {
|
||||
Assert.isTrue(timeout >= 0, "Timeout must be a non-negative value");
|
||||
this.socketTimeout = timeout;
|
||||
this.requestConfig = cloneRequestConfig()
|
||||
.setSocketTimeout(timeout).build();
|
||||
setLegacySocketTimeout(getHttpClient(), timeout);
|
||||
}
|
||||
|
||||
|
|
@ -177,6 +185,10 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
|
|||
}
|
||||
}
|
||||
|
||||
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
|
||||
|
|
@ -205,18 +217,11 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
|
|||
config = ((Configurable) httpRequest).getConfig();
|
||||
}
|
||||
if (config == null) {
|
||||
if (this.connectTimeout > 0 || this.connectionRequestTimeout > 0 || this.socketTimeout > 0) {
|
||||
config = RequestConfig.custom()
|
||||
.setConnectTimeout(this.connectTimeout)
|
||||
.setConnectionRequestTimeout(this.connectionRequestTimeout)
|
||||
.setSocketTimeout(this.socketTimeout)
|
||||
.build();
|
||||
}
|
||||
else {
|
||||
config = RequestConfig.DEFAULT;
|
||||
}
|
||||
config = this.requestConfig;
|
||||
}
|
||||
if (config != null) {
|
||||
context.setAttribute(HttpClientContext.REQUEST_CONFIG, config);
|
||||
}
|
||||
context.setAttribute(HttpClientContext.REQUEST_CONFIG, config);
|
||||
}
|
||||
if (this.bufferRequestBody) {
|
||||
return new HttpComponentsClientHttpRequest(client, httpRequest, context);
|
||||
|
|
|
|||
|
|
@ -69,27 +69,30 @@ public class HttpComponentsHttpInvokerRequestExecutor extends AbstractHttpInvoke
|
|||
private static final int DEFAULT_READ_TIMEOUT_MILLISECONDS = (60 * 1000);
|
||||
|
||||
private HttpClient httpClient;
|
||||
private int connectionTimeout = 0;
|
||||
private int connectionRequestTimeout = 0;
|
||||
private int readTimeout = DEFAULT_READ_TIMEOUT_MILLISECONDS;
|
||||
|
||||
private RequestConfig requestConfig;
|
||||
|
||||
/**
|
||||
* Create a new instance of the HttpComponentsHttpInvokerRequestExecutor with a default
|
||||
* {@link HttpClient} that uses a default {@code org.apache.http.impl.conn.PoolingClientConnectionManager}.
|
||||
*/
|
||||
public HttpComponentsHttpInvokerRequestExecutor() {
|
||||
this(createDefaultHttpClient(), RequestConfig.custom()
|
||||
.setSocketTimeout(DEFAULT_READ_TIMEOUT_MILLISECONDS).build());
|
||||
}
|
||||
|
||||
private static HttpClient createDefaultHttpClient() {
|
||||
Registry<ConnectionSocketFactory> schemeRegistry = RegistryBuilder.<ConnectionSocketFactory>create()
|
||||
.register("http", PlainConnectionSocketFactory.getSocketFactory())
|
||||
.register("https", SSLConnectionSocketFactory.getSocketFactory())
|
||||
.build();
|
||||
.register("http", PlainConnectionSocketFactory.getSocketFactory())
|
||||
.register("https", SSLConnectionSocketFactory.getSocketFactory())
|
||||
.build();
|
||||
|
||||
PoolingHttpClientConnectionManager connectionManager
|
||||
= new PoolingHttpClientConnectionManager(schemeRegistry);
|
||||
connectionManager.setMaxTotal(DEFAULT_MAX_TOTAL_CONNECTIONS);
|
||||
connectionManager.setDefaultMaxPerRoute(DEFAULT_MAX_CONNECTIONS_PER_ROUTE);
|
||||
|
||||
this.httpClient = HttpClientBuilder.create().setConnectionManager(connectionManager).build();
|
||||
return HttpClientBuilder.create().setConnectionManager(connectionManager).build();
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -98,9 +101,13 @@ public class HttpComponentsHttpInvokerRequestExecutor extends AbstractHttpInvoke
|
|||
* @param httpClient the HttpClient instance to use for this request executor
|
||||
*/
|
||||
public HttpComponentsHttpInvokerRequestExecutor(HttpClient httpClient) {
|
||||
this.httpClient = httpClient;
|
||||
this(httpClient, null);
|
||||
}
|
||||
|
||||
private HttpComponentsHttpInvokerRequestExecutor(HttpClient httpClient, RequestConfig requestConfig) {
|
||||
this.httpClient = httpClient;
|
||||
this.requestConfig = requestConfig;
|
||||
}
|
||||
|
||||
/**
|
||||
* Set the {@link HttpClient} instance to use for this request executor.
|
||||
|
|
@ -119,11 +126,15 @@ public class HttpComponentsHttpInvokerRequestExecutor extends AbstractHttpInvoke
|
|||
/**
|
||||
* Set the connection timeout for the underlying HttpClient.
|
||||
* A timeout value of 0 specifies an infinite timeout.
|
||||
* <p>Additional properties can be configured by specifying a
|
||||
* {@link RequestConfig} instance on a custom {@link HttpClient}.
|
||||
* @param timeout the timeout value in milliseconds
|
||||
* @see RequestConfig#getConnectTimeout()
|
||||
*/
|
||||
public void setConnectTimeout(int timeout) {
|
||||
Assert.isTrue(timeout >= 0, "Timeout must be a non-negative value");
|
||||
this.connectionTimeout = timeout;
|
||||
this.requestConfig = cloneRequestConfig()
|
||||
.setConnectTimeout(timeout).build();
|
||||
setLegacyConnectionTimeout(getHttpClient(), timeout);
|
||||
}
|
||||
|
||||
|
|
@ -153,21 +164,29 @@ public class HttpComponentsHttpInvokerRequestExecutor extends AbstractHttpInvoke
|
|||
* Set the timeout in milliseconds used when requesting a connection from the connection
|
||||
* manager using the underlying HttpClient.
|
||||
* A timeout value of 0 specifies an infinite timeout.
|
||||
* <p>Additional properties can be configured by specifying a
|
||||
* {@link RequestConfig} instance on a custom {@link HttpClient}.
|
||||
* @param connectionRequestTimeout the timeout value to request a connection in milliseconds
|
||||
* @see RequestConfig#getConnectionRequestTimeout()
|
||||
*/
|
||||
public void setConnectionRequestTimeout(int connectionRequestTimeout) {
|
||||
this.connectionRequestTimeout = connectionRequestTimeout;
|
||||
this.requestConfig = cloneRequestConfig()
|
||||
.setConnectionRequestTimeout(connectionRequestTimeout).build();
|
||||
}
|
||||
|
||||
/**
|
||||
* Set the socket read timeout for the underlying HttpClient.
|
||||
* A timeout value of 0 specifies an infinite timeout.
|
||||
* <p>Additional properties can be configured by specifying a
|
||||
* {@link RequestConfig} instance on a custom {@link HttpClient}.
|
||||
* @param timeout the timeout value in milliseconds
|
||||
* @see #DEFAULT_READ_TIMEOUT_MILLISECONDS
|
||||
* @see RequestConfig#getSocketTimeout()
|
||||
*/
|
||||
public void setReadTimeout(int timeout) {
|
||||
Assert.isTrue(timeout >= 0, "Timeout must be a non-negative value");
|
||||
this.readTimeout = timeout;
|
||||
this.requestConfig = cloneRequestConfig()
|
||||
.setSocketTimeout(timeout).build();
|
||||
setLegacySocketTimeout(getHttpClient(), timeout);
|
||||
}
|
||||
|
||||
|
|
@ -186,6 +205,10 @@ public class HttpComponentsHttpInvokerRequestExecutor extends AbstractHttpInvoke
|
|||
}
|
||||
}
|
||||
|
||||
private RequestConfig.Builder cloneRequestConfig() {
|
||||
return this.requestConfig != null ? RequestConfig.copy(this.requestConfig) : RequestConfig.custom();
|
||||
}
|
||||
|
||||
/**
|
||||
* Execute the given request through the HttpClient.
|
||||
* <p>This method implements the basic processing workflow:
|
||||
|
|
@ -251,16 +274,7 @@ public class HttpComponentsHttpInvokerRequestExecutor extends AbstractHttpInvoke
|
|||
* @return the RequestConfig to use
|
||||
*/
|
||||
protected RequestConfig createRequestConfig(HttpInvokerClientConfiguration config) {
|
||||
if (this.connectionTimeout > 0 || this.connectionRequestTimeout > 0 || this.readTimeout > 0) {
|
||||
return RequestConfig.custom()
|
||||
.setConnectTimeout(this.connectionTimeout)
|
||||
.setConnectionRequestTimeout(this.connectionRequestTimeout)
|
||||
.setSocketTimeout(this.readTimeout)
|
||||
.build();
|
||||
}
|
||||
else {
|
||||
return RequestConfig.DEFAULT;
|
||||
}
|
||||
return (this.requestConfig != null ? this.requestConfig : null);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -16,10 +16,6 @@
|
|||
|
||||
package org.springframework.http.client;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertNotNull;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
|
||||
import java.net.URI;
|
||||
|
||||
import org.apache.http.HttpEntityEnclosingRequest;
|
||||
|
|
@ -27,12 +23,13 @@ import org.apache.http.client.HttpClient;
|
|||
import org.apache.http.client.config.RequestConfig;
|
||||
import org.apache.http.client.methods.HttpUriRequest;
|
||||
import org.apache.http.client.protocol.HttpClientContext;
|
||||
import org.apache.http.impl.client.DefaultHttpClient;
|
||||
import org.apache.http.impl.client.CloseableHttpClient;
|
||||
import org.apache.http.impl.client.HttpClientBuilder;
|
||||
import org.apache.http.params.CoreConnectionPNames;
|
||||
import org.junit.Test;
|
||||
import org.springframework.http.HttpMethod;
|
||||
|
||||
import static org.junit.Assert.*;
|
||||
|
||||
public class HttpComponentsClientHttpRequestFactoryTests extends AbstractHttpRequestFactoryTestCase {
|
||||
|
||||
@Override
|
||||
|
|
@ -49,13 +46,15 @@ public class HttpComponentsClientHttpRequestFactoryTests extends AbstractHttpReq
|
|||
@SuppressWarnings("deprecation")
|
||||
@Test
|
||||
public void assertLegacyCustomConfig() {
|
||||
HttpClient httpClient = new DefaultHttpClient(); // Does not support RequestConfig
|
||||
HttpClient httpClient = new org.apache.http.impl.client.DefaultHttpClient(); // Does not support RequestConfig
|
||||
HttpComponentsClientHttpRequestFactory hrf = new HttpComponentsClientHttpRequestFactory(httpClient);
|
||||
hrf.setConnectTimeout(1234);
|
||||
assertEquals(1234, httpClient.getParams().getIntParameter(CoreConnectionPNames.CONNECTION_TIMEOUT, 0));
|
||||
assertEquals(1234, httpClient.getParams().getIntParameter(
|
||||
org.apache.http.params.CoreConnectionPNames.CONNECTION_TIMEOUT, 0));
|
||||
|
||||
hrf.setReadTimeout(4567);
|
||||
assertEquals(4567, httpClient.getParams().getIntParameter(CoreConnectionPNames.SO_TIMEOUT, 0));
|
||||
assertEquals(4567, httpClient.getParams().getIntParameter(
|
||||
org.apache.http.params.CoreConnectionPNames.SO_TIMEOUT, 0));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -80,6 +79,45 @@ public class HttpComponentsClientHttpRequestFactoryTests extends AbstractHttpReq
|
|||
assertEquals("Wrong custom socket timeout", 4567, requestConfig.getSocketTimeout());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void customHttpClientUsesItsDefault() throws Exception {
|
||||
HttpComponentsClientHttpRequestFactory hrf =
|
||||
new HttpComponentsClientHttpRequestFactory(HttpClientBuilder.create().build());
|
||||
|
||||
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));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void defaultSettingsOfHttpClientLostOnExecutorCustomization() throws Exception {
|
||||
CloseableHttpClient client = HttpClientBuilder.create()
|
||||
.setDefaultRequestConfig(RequestConfig.custom().setConnectTimeout(1234).build())
|
||||
.build();
|
||||
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));
|
||||
|
||||
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;
|
||||
|
||||
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());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void createHttpUriRequest() throws Exception {
|
||||
URI uri = new URI("http://example.com");
|
||||
|
|
|
|||
|
|
@ -1,3 +1,19 @@
|
|||
/*
|
||||
* Copyright 2002-2014 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.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
package org.springframework.remoting.httpinvoker;
|
||||
|
||||
import java.io.IOException;
|
||||
|
|
@ -63,6 +79,36 @@ public class HttpComponentsHttpInvokerRequestExecutorTests {
|
|||
assertEquals(10000, httpPost.getConfig().getSocketTimeout());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void customHttpClientUsesItsDefault() throws IOException {
|
||||
HttpComponentsHttpInvokerRequestExecutor executor =
|
||||
new HttpComponentsHttpInvokerRequestExecutor(HttpClientBuilder.create().build());
|
||||
|
||||
HttpInvokerClientConfiguration config = mockHttpInvokerClientConfiguration("http://fake-service");
|
||||
HttpPost httpPost = executor.createHttpPost(config);
|
||||
assertNull("No custom config should be set with a custom HttpClient", httpPost.getConfig());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void defaultSettingsOfHttpClientLostOnExecutorCustomization() throws IOException {
|
||||
CloseableHttpClient client = HttpClientBuilder.create()
|
||||
.setDefaultRequestConfig(RequestConfig.custom().setConnectTimeout(1234).build())
|
||||
.build();
|
||||
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());
|
||||
|
||||
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());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void ignoreFactorySettings() throws IOException {
|
||||
CloseableHttpClient httpClient = HttpClientBuilder.create().build();
|
||||
|
|
|
|||
Loading…
Reference in New Issue