From 611eb9b98d32bb3985c1b410034c259b0f4357fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Nicoll?= Date: Thu, 27 Mar 2025 18:48:49 +0100 Subject: [PATCH] Use provided HttpClient consistently This commit harmonizes the behavior of HttpComponents5MessageSender to reuse a provided HttpClient, be it provided via constructor or property. Closes gh-1512 --- .../http/HttpComponents5MessageSender.java | 48 +++++++++---------- .../HttpComponents5MessageSenderTest.java | 27 +++++++---- 2 files changed, 40 insertions(+), 35 deletions(-) diff --git a/spring-ws-core/src/main/java/org/springframework/ws/transport/http/HttpComponents5MessageSender.java b/spring-ws-core/src/main/java/org/springframework/ws/transport/http/HttpComponents5MessageSender.java index 1a1069b4..ca321901 100644 --- a/spring-ws-core/src/main/java/org/springframework/ws/transport/http/HttpComponents5MessageSender.java +++ b/spring-ws-core/src/main/java/org/springframework/ws/transport/http/HttpComponents5MessageSender.java @@ -63,16 +63,15 @@ public class HttpComponents5MessageSender extends AbstractHttpWebServiceMessageS private static final String HTTP_CLIENT_ALREADY_SET = "httpClient already set"; - private HttpClient httpClient; + private final HttpComponents5ClientFactory clientFactory; - private HttpComponents5ClientFactory clientFactory; + private HttpClient httpClient; /** * Create a new instance of the {@code HttpClientMessageSender} with a default * {@link HttpClient} that uses a default {@link PoolingHttpClientConnectionManager}. */ public HttpComponents5MessageSender() { - this.clientFactory = new HttpComponents5ClientFactory(); this.clientFactory.setClientBuilderCustomizer( httpClientBuilder -> httpClientBuilder.addRequestInterceptorFirst(new RemoveSoapHeadersInterceptor())); @@ -89,7 +88,7 @@ public class HttpComponents5MessageSender extends AbstractHttpWebServiceMessageS * @param httpClient the HttpClient instance to use for this sender */ public HttpComponents5MessageSender(HttpClient httpClient) { - + this(); Assert.notNull(httpClient, "httpClient must not be null"); this.httpClient = httpClient; } @@ -98,11 +97,9 @@ public class HttpComponents5MessageSender extends AbstractHttpWebServiceMessageS * * @see HttpComponents5ClientFactory#setAuthScope(AuthScope) */ public void setAuthScope(AuthScope authScope) { - if (getHttpClient() != null) { throw new IllegalStateException(HTTP_CLIENT_ALREADY_SET); } - this.clientFactory.setAuthScope(authScope); } @@ -110,11 +107,9 @@ public class HttpComponents5MessageSender extends AbstractHttpWebServiceMessageS * * @see HttpComponents5ClientFactory#setCredentials(Credentials) */ public void setCredentials(Credentials credentials) { - if (getHttpClient() != null) { throw new IllegalStateException(HTTP_CLIENT_ALREADY_SET); } - this.clientFactory.setCredentials(credentials); } @@ -127,63 +122,64 @@ public class HttpComponents5MessageSender extends AbstractHttpWebServiceMessageS /** * Set the {@code HttpClient} used by this message sender. + *

+ * This effectively disable any customization and does not change the given + * {@code HttpClient} in any way. As such, it does not set timeouts, nor does it + * {@linkplain HttpClientBuilder#addRequestInterceptorFirst(HttpRequestInterceptor) + * add} the {@link RemoveSoapHeadersInterceptor}. + * @param httpClient the HttpClient to use */ public void setHttpClient(HttpClient httpClient) { this.httpClient = httpClient; } - /* - * * @see HttpComponents5ClientFactory#setConnectionTimeout(Duration) + /** + * Set the timeout until a connection is established. + * @see HttpComponents5ClientFactory#setConnectionTimeout(Duration) */ public void setConnectionTimeout(Duration timeout) { - if (getHttpClient() != null) { throw new IllegalStateException(HTTP_CLIENT_ALREADY_SET); } - this.clientFactory.setConnectionTimeout(timeout); } - /* - * * @see HttpComponents5ClientFactory#setReadTimeout(Duration) + /** + * Set the socket read timeout for the underlying HttpClient. + * @see HttpComponents5ClientFactory#setReadTimeout(Duration) */ public void setReadTimeout(Duration timeout) { - if (getHttpClient() != null) { throw new IllegalStateException(HTTP_CLIENT_ALREADY_SET); } - this.clientFactory.setReadTimeout(timeout); } - /* - * * @see HttpComponents5ClientFactory#setMaxTotalConnections(int) + /** + * Sets the maximum number of connections allowed for the underlying HttpClient. + * @see HttpComponents5ClientFactory#setMaxTotalConnections(int) */ public void setMaxTotalConnections(int maxTotalConnections) { - if (getHttpClient() != null) { throw new IllegalStateException(HTTP_CLIENT_ALREADY_SET); } - this.clientFactory.setMaxTotalConnections(maxTotalConnections); } - /* - * * @see HttpComponents5ClientFactory#setMaxConnectionsPerHost(Map) + /** + * Sets the maximum number of connections per host for the underlying HttpClient. + * @see HttpComponents5ClientFactory#setMaxConnectionsPerHost(Map) */ public void setMaxConnectionsPerHost(Map maxConnectionsPerHost) { - if (getHttpClient() != null) { throw new IllegalStateException(HTTP_CLIENT_ALREADY_SET); } - this.clientFactory.setMaxConnectionsPerHost(maxConnectionsPerHost); } @Override public void afterPropertiesSet() throws Exception { - - if (this.clientFactory != null) { + if (getHttpClient() == null) { this.httpClient = this.clientFactory.getObject(); } } diff --git a/spring-ws-core/src/test/java/org/springframework/ws/transport/http/HttpComponents5MessageSenderTest.java b/spring-ws-core/src/test/java/org/springframework/ws/transport/http/HttpComponents5MessageSenderTest.java index 70a17fc2..97d7069f 100644 --- a/spring-ws-core/src/test/java/org/springframework/ws/transport/http/HttpComponents5MessageSenderTest.java +++ b/spring-ws-core/src/test/java/org/springframework/ws/transport/http/HttpComponents5MessageSenderTest.java @@ -25,30 +25,39 @@ import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; +/** + * Tests for {@link HttpComponents5MessageSender}. + * + * @author Lars Uffmann + * @author Stephane Nicoll + */ class HttpComponents5MessageSenderTest { @Test void afterPropertiesSetShouldProperlyInitializeHttpClient() throws Exception { - HttpComponents5MessageSender messageSender = new HttpComponents5MessageSender(); assertThat(messageSender.getHttpClient()).isNull(); - - Duration timeout = Duration.ofSeconds(1); - assertThatCode(() -> messageSender.setConnectionTimeout(timeout)).doesNotThrowAnyException(); + messageSender.setConnectionTimeout(Duration.ofSeconds(1)); messageSender.afterPropertiesSet(); assertThat(messageSender.getHttpClient()).isNotNull(); } @Test - void afterPropertiesSetShouldUseAlreadyProvidedHttpClientIfAvailable() throws Exception { - + void afterPropertiesSetShouldUseAlreadyProvidedHttpClientIfAvailableWithConstructor() throws Exception { CloseableHttpClient httpClient = HttpClientBuilder.create().build(); HttpComponents5MessageSender messageSender = new HttpComponents5MessageSender(httpClient); + assertThatCode(() -> messageSender.setConnectionTimeout(Duration.ofSeconds(1))) + .isInstanceOf(IllegalStateException.class); + messageSender.afterPropertiesSet(); + assertThat(messageSender.getHttpClient()).isSameAs(httpClient); + } - Duration timeout = Duration.ofSeconds(1); - assertThatCode(() -> messageSender.setConnectionTimeout(timeout)).isInstanceOf(IllegalStateException.class); - + @Test + void afterPropertiesSetShouldUseAlreadyProvidedHttpClientIfAvailableWithProperty() throws Exception { + CloseableHttpClient httpClient = HttpClientBuilder.create().build(); + HttpComponents5MessageSender messageSender = new HttpComponents5MessageSender(); + messageSender.setHttpClient(httpClient); messageSender.afterPropertiesSet(); assertThat(messageSender.getHttpClient()).isSameAs(httpClient); }