Merged HttpClient defaults with local customizations
Update HttpComponents wrapper to merge local customizations with the default of the current HttpClient instead of overriding everything. This is available as from HttpComponents 4.4. that exposes the default request config from the client via the Configurable interface. If the client does not implement such interface, the previous behaviour is applied Issue: SPR-12583
This commit is contained in:
parent
3662ad4f94
commit
bce145c06e
|
|
@ -217,7 +217,7 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
|
|||
config = ((Configurable) httpRequest).getConfig();
|
||||
}
|
||||
if (config == null) {
|
||||
config = this.requestConfig;
|
||||
config = createRequestConfig(client);
|
||||
}
|
||||
if (config != null) {
|
||||
context.setAttribute(HttpClientContext.REQUEST_CONFIG, config);
|
||||
|
|
@ -231,6 +231,43 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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.
|
||||
* @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
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
* <p>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();
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue