diff --git a/spring-ws-core/src/main/java/org/springframework/ws/transport/http/HttpComponents5ClientFactory.java b/spring-ws-core/src/main/java/org/springframework/ws/transport/http/HttpComponents5ClientFactory.java index 82fc6075d..584909498 100644 --- a/spring-ws-core/src/main/java/org/springframework/ws/transport/http/HttpComponents5ClientFactory.java +++ b/spring-ws-core/src/main/java/org/springframework/ws/transport/http/HttpComponents5ClientFactory.java @@ -18,8 +18,8 @@ import java.net.URI; import java.net.URISyntaxException; +import java.time.Duration; import java.util.Map; -import java.util.concurrent.TimeUnit; import org.apache.hc.client5.http.HttpRoute; import org.apache.hc.client5.http.auth.AuthScope; @@ -31,25 +31,28 @@ import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager; import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder; import org.apache.hc.core5.http.HttpHost; - +import org.apache.hc.core5.util.Timeout; import org.springframework.beans.factory.FactoryBean; /** - * {@code FactoryBean} to set up a Apache - * CloseableHttpClient + * {@link FactoryBean} to set up a {@link CloseableHttpClient} using HttpComponents HttpClient 5. * + * @see http://hc.apache.org/httpcomponents-client * @author Lars Uffmann * @since 4.0.5 */ public class HttpComponents5ClientFactory implements FactoryBean { - private static final int DEFAULT_CONNECTION_TIMEOUT_MILLISECONDS = (60 * 1000); - private int connectionTimeout = DEFAULT_CONNECTION_TIMEOUT_MILLISECONDS; - private static final int DEFAULT_READ_TIMEOUT_MILLISECONDS = (60 * 1000); + private static final Duration DEFAULT_CONNECTION_TIMEOUT = Duration.ofSeconds(60); + + private static final Duration DEFAULT_READ_TIMEOUT = Duration.ofSeconds(60); + + private Duration connectionTimeout = DEFAULT_CONNECTION_TIMEOUT; - private int readTimeout = DEFAULT_READ_TIMEOUT_MILLISECONDS; + private Duration readTimeout = DEFAULT_READ_TIMEOUT; private int maxTotalConnections = -1; + private AuthScope authScope = null; private Credentials credentials = null; @@ -86,24 +89,28 @@ public void setAuthScope(AuthScope authScope) { /** * Sets the timeout until a connection is established. A value of 0 means never timeout. * - * @param timeout the timeout value in milliseconds + * @param timeout the timeout value */ - public void setConnectionTimeout(int timeout) { - if (timeout < 0) { + public void setConnectionTimeout(Duration timeout) { + + if (timeout.isNegative()) { throw new IllegalArgumentException("timeout must be a non-negative value"); } + this.connectionTimeout = timeout; } /** * Set the socket read timeout for the underlying HttpClient. A value of 0 means never timeout. * - * @param timeout the timeout value in milliseconds + * @param timeout the timeout value */ - public void setReadTimeout(int timeout) { - if (timeout < 0) { + public void setReadTimeout(Duration timeout) { + + if (timeout.isNegative()) { throw new IllegalArgumentException("timeout must be a non-negative value"); } + this.readTimeout = timeout; } @@ -114,9 +121,11 @@ public void setReadTimeout(int timeout) { * @see PoolingHttpClientConnectionManager... */ public void setMaxTotalConnections(int maxTotalConnections) { + if (maxTotalConnections <= 0) { throw new IllegalArgumentException("maxTotalConnections must be a positive value"); } + this.maxTotalConnections = maxTotalConnections; } @@ -142,14 +151,14 @@ public void setMaxConnectionsPerHost(Map maxConnectionsPerHost) void applyMaxConnectionsPerHost(PoolingHttpClientConnectionManager connectionManager) throws URISyntaxException { for (Map.Entry entry : maxConnectionsPerHost.entrySet()) { + URI uri = new URI(entry.getKey()); HttpHost host = new HttpHost(uri.getScheme(), uri.getHost(), getPort(uri)); final HttpRoute route; if (uri.getScheme().equals("https")) { route = new HttpRoute(host, null, true); - } - else { + } else { route = new HttpRoute(host); } int max = Integer.parseInt(entry.getValue()); @@ -158,7 +167,9 @@ void applyMaxConnectionsPerHost(PoolingHttpClientConnectionManager connectionMan } static int getPort(URI uri) { + if (uri.getPort() == -1) { + if ("https".equalsIgnoreCase(uri.getScheme())) { return 443; } @@ -166,6 +177,7 @@ static int getPort(URI uri) { return 80; } } + return uri.getPort(); } @@ -176,12 +188,15 @@ public boolean isSingleton() { @Override public CloseableHttpClient getObject() throws Exception { - PoolingHttpClientConnectionManagerBuilder connectionManagerBuilder = PoolingHttpClientConnectionManagerBuilder.create(); + + PoolingHttpClientConnectionManagerBuilder connectionManagerBuilder = PoolingHttpClientConnectionManagerBuilder + .create(); + if (this.maxTotalConnections != -1) { connectionManagerBuilder.setMaxConnTotal(this.maxTotalConnections); } - if (null != this.connectionManagerBuilderCustomizer) { + if (this.connectionManagerBuilderCustomizer != null) { this.connectionManagerBuilderCustomizer.customize(connectionManagerBuilder); } @@ -189,21 +204,22 @@ public CloseableHttpClient getObject() throws Exception { applyMaxConnectionsPerHost(connectionManager); - RequestConfig.Builder requestConfigBuilder = RequestConfig.custom() - .setConnectTimeout(connectionTimeout, TimeUnit.MILLISECONDS) - .setResponseTimeout(readTimeout, TimeUnit.MILLISECONDS); + RequestConfig.Builder requestConfigBuilder = RequestConfig.custom() // + .setConnectionRequestTimeout(Timeout.of(connectionTimeout)) // + .setResponseTimeout(Timeout.of(readTimeout)); - HttpClientBuilder httpClientBuilder = HttpClientBuilder.create() - .setDefaultRequestConfig(requestConfigBuilder.build()) + HttpClientBuilder httpClientBuilder = HttpClientBuilder.create() // + .setDefaultRequestConfig(requestConfigBuilder.build()) // .setConnectionManager(connectionManager); - if (null != credentials && null != authScope) { + if (credentials != null && authScope != null) { + BasicCredentialsProvider basicCredentialsProvider = new BasicCredentialsProvider(); basicCredentialsProvider.setCredentials(authScope, credentials); httpClientBuilder.setDefaultCredentialsProvider(basicCredentialsProvider); } - if (null != this.clientBuilderCustomizer) { + if (this.clientBuilderCustomizer != null) { clientBuilderCustomizer.customize(httpClientBuilder); } @@ -223,7 +239,8 @@ public void setClientBuilderCustomizer(HttpClientBuilderCustomizer clientBuilder this.clientBuilderCustomizer = clientBuilderCustomizer; } - public void setConnectionManagerBuilderCustomizer(PoolingHttpClientConnectionManagerBuilderCustomizer connectionManagerBuilderCustomizer) { + public void setConnectionManagerBuilderCustomizer( + PoolingHttpClientConnectionManagerBuilderCustomizer connectionManagerBuilderCustomizer) { this.connectionManagerBuilderCustomizer = connectionManagerBuilderCustomizer; } diff --git a/spring-ws-core/src/main/java/org/springframework/ws/transport/http/HttpComponents5Connection.java b/spring-ws-core/src/main/java/org/springframework/ws/transport/http/HttpComponents5Connection.java index d0e0bdb9c..11c1baf40 100644 --- a/spring-ws-core/src/main/java/org/springframework/ws/transport/http/HttpComponents5Connection.java +++ b/spring-ws-core/src/main/java/org/springframework/ws/transport/http/HttpComponents5Connection.java @@ -28,20 +28,19 @@ import org.apache.hc.client5.http.classic.HttpClient; import org.apache.hc.client5.http.classic.methods.HttpPost; import org.apache.hc.core5.http.ClassicHttpResponse; -import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.HttpEntity; import org.apache.hc.core5.http.HttpResponse; +import org.apache.hc.core5.http.NameValuePair; import org.apache.hc.core5.http.io.entity.ByteArrayEntity; import org.apache.hc.core5.http.io.entity.EntityUtils; import org.apache.hc.core5.http.protocol.HttpContext; - import org.springframework.util.Assert; import org.springframework.ws.WebServiceMessage; import org.springframework.ws.transport.WebServiceConnection; /** - * Implementation of {@link WebServiceConnection} that is based on Apache HttpClient. Exposes a {@link org.apache.hc.client5.http.classic.methods.HttpPost} and - * {@link org.apache.hc.core5.http.HttpResponse + * Implementation of {@link WebServiceConnection} that is based on Apache HttpClient 5. Exposes a {@link HttpPost} and + * {@link HttpResponse}. * * @author Alan Stewart * @author Barry Pitman @@ -63,8 +62,10 @@ public class HttpComponents5Connection extends AbstractHttpSenderConnection { private ByteArrayOutputStream requestBuffer; protected HttpComponents5Connection(HttpClient httpClient, HttpPost httpPost, HttpContext httpContext) { + Assert.notNull(httpClient, "httpClient must not be null"); Assert.notNull(httpPost, "httpPost must not be null"); + this.httpClient = httpClient; this.httpPost = httpPost; this.httpContext = httpContext; @@ -80,8 +81,9 @@ public HttpResponse getHttpResponse() { @Override public void onClose() throws IOException { - //XXX: + if (httpResponse instanceof ClassicHttpResponse response) { + if (response.getEntity() != null) { EntityUtils.consume(response.getEntity()); } @@ -117,9 +119,10 @@ protected OutputStream getRequestOutputStream() throws IOException { @Override protected void onSendAfterWrite(WebServiceMessage message) throws IOException { - //XXX + httpPost.setEntity(new ByteArrayEntity(requestBuffer.toByteArray(), null)); requestBuffer = null; + if (httpContext != null) { httpResponse = httpClient.execute(httpPost, httpContext); } else { @@ -143,8 +146,9 @@ protected String getResponseMessage() throws IOException { @Override protected long getResponseContentLength() throws IOException { - //XXX: + if (httpResponse instanceof ClassicHttpResponse response) { + HttpEntity entity = response.getEntity(); if (entity != null) { return entity.getContentLength(); @@ -155,32 +159,31 @@ protected long getResponseContentLength() throws IOException { @Override protected InputStream getRawResponseInputStream() throws IOException { + if (httpResponse instanceof ClassicHttpResponse response) { + HttpEntity entity = response.getEntity(); if (entity != null) { return entity.getContent(); } } + throw new IllegalStateException("Response has no enclosing response entity, cannot create input stream"); } @Override public Iterator getResponseHeaderNames() throws IOException { - Header[] headers = httpResponse.getHeaders(); - String[] names = new String[headers.length]; - for (int i = 0; i < headers.length; i++) { - names[i] = headers[i].getName(); - } - return Arrays.asList(names).iterator(); + + return Arrays.stream(httpResponse.getHeaders()) // + .map(NameValuePair::getName) // + .iterator(); } @Override public Iterator getResponseHeaders(String name) throws IOException { - Header[] headers = httpResponse.getHeaders(name); - String[] values = new String[headers.length]; - for (int i = 0; i < headers.length; i++) { - values[i] = headers[i].getValue(); - } - return Arrays.asList(values).iterator(); + + return Arrays.stream(httpResponse.getHeaders(name)) // + .map(NameValuePair::getValue) // + .iterator(); } } 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 22caf2f13..27bc7c8e5 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 @@ -18,6 +18,7 @@ import java.io.IOException; import java.net.URI; +import java.time.Duration; import java.util.Map; import org.apache.hc.client5.http.auth.AuthScope; @@ -33,7 +34,6 @@ import org.apache.hc.core5.http.HttpRequest; import org.apache.hc.core5.http.HttpRequestInterceptor; import org.apache.hc.core5.http.protocol.HttpContext; - import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.InitializingBean; import org.springframework.util.Assert; @@ -57,7 +57,9 @@ */ public class HttpComponents5MessageSender extends AbstractHttpWebServiceMessageSender implements InitializingBean, DisposableBean { + private static final String HTTP_CLIENT_ALREADY_SET = "httpClient already set"; + private HttpClient httpClient; private HttpComponents5ClientFactory clientFactory; @@ -67,22 +69,23 @@ public class HttpComponents5MessageSender extends AbstractHttpWebServiceMessageS * {@link PoolingHttpClientConnectionManager}. */ public HttpComponents5MessageSender() { + this.clientFactory = new HttpComponents5ClientFactory(); - this.clientFactory.setClientBuilderCustomizer(httpClientBuilder -> - httpClientBuilder.addRequestInterceptorFirst(new RemoveSoapHeadersInterceptor())); + this.clientFactory.setClientBuilderCustomizer( + httpClientBuilder -> httpClientBuilder.addRequestInterceptorFirst(new RemoveSoapHeadersInterceptor())); } /** - * Create a new instance of the {@code HttpClientMessageSender} with the given {@link HttpClient} instance. + * Create a new instance of the {@link HttpComponents5MessageSender} with the given {@link HttpClient} instance. *

* This constructor 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}. + * does it {@linkplain HttpClientBuilder#addRequestInterceptorFirst(HttpRequestInterceptor) add} the + * {@link RemoveSoapHeadersInterceptor}. * * @param httpClient the HttpClient instance to use for this sender */ public HttpComponents5MessageSender(HttpClient httpClient) { + Assert.notNull(httpClient, "httpClient must not be null"); this.httpClient = httpClient; } @@ -91,9 +94,11 @@ public HttpComponents5MessageSender(HttpClient httpClient) { * @see HttpComponents5ClientFactory#setAuthScope(AuthScope) */ public void setAuthScope(AuthScope authScope) { - if (null != getHttpClient()) { + + if (getHttpClient() != null) { throw new IllegalStateException(HTTP_CLIENT_ALREADY_SET); } + this.clientFactory.setAuthScope(authScope); } @@ -101,9 +106,11 @@ public void setAuthScope(AuthScope authScope) { * @see HttpComponents5ClientFactory#setCredentials(Credentials) */ public void setCredentials(Credentials credentials) { - if (null != getHttpClient()) { + + if (getHttpClient() != null) { throw new IllegalStateException(HTTP_CLIENT_ALREADY_SET); } + this.clientFactory.setCredentials(credentials); } @@ -122,22 +129,26 @@ public void setHttpClient(HttpClient httpClient) { } /** - * @see HttpComponents5ClientFactory#setConnectionTimeout(int) + * @see HttpComponents5ClientFactory#setConnectionTimeout(Duration) */ - public void setConnectionTimeout(int timeout) { - if (null != getHttpClient()) { + public void setConnectionTimeout(Duration timeout) { + + if (getHttpClient() != null) { throw new IllegalStateException(HTTP_CLIENT_ALREADY_SET); } + this.clientFactory.setConnectionTimeout(timeout); } /** - * @see HttpComponents5ClientFactory#setReadTimeout(int) + * @see HttpComponents5ClientFactory#setReadTimeout(Duration) */ - public void setReadTimeout(int timeout) { - if (null != getHttpClient()) { + public void setReadTimeout(Duration timeout) { + + if (getHttpClient() != null) { throw new IllegalStateException(HTTP_CLIENT_ALREADY_SET); } + this.clientFactory.setReadTimeout(timeout); } @@ -145,9 +156,11 @@ public void setReadTimeout(int timeout) { * @see HttpComponents5ClientFactory#setMaxTotalConnections(int) */ public void setMaxTotalConnections(int maxTotalConnections) { - if (null != getHttpClient()) { + + if (getHttpClient() != null) { throw new IllegalStateException(HTTP_CLIENT_ALREADY_SET); } + this.clientFactory.setMaxTotalConnections(maxTotalConnections); } @@ -155,9 +168,11 @@ public void setMaxTotalConnections(int maxTotalConnections) { * @see HttpComponents5ClientFactory#setMaxConnectionsPerHost(Map) */ public void setMaxConnectionsPerHost(Map maxConnectionsPerHost) { - if (null != getHttpClient()) { + + if (getHttpClient() != null) { throw new IllegalStateException(HTTP_CLIENT_ALREADY_SET); } + this.clientFactory.setMaxConnectionsPerHost(maxConnectionsPerHost); } @@ -168,16 +183,20 @@ public void afterPropertiesSet() throws Exception { @Override public WebServiceConnection createConnection(URI uri) throws IOException { + HttpPost httpPost = new HttpPost(uri); + if (isAcceptGzipEncoding()) { httpPost.addHeader(HttpTransportConstants.HEADER_ACCEPT_ENCODING, HttpTransportConstants.CONTENT_ENCODING_GZIP); } + HttpContext httpContext = createContext(uri); + return new HttpComponents5Connection(getHttpClient(), httpPost, httpContext); } /** - * Template method that allows for creation of a {@link HttpContext} for the given uri. Default implementation returns + * Template method that allows for creation of an {@link HttpContext} for the given uri. Default implementation returns * {@code null}. * * @param uri the URI to create the context for @@ -189,7 +208,8 @@ protected HttpContext createContext(URI uri) { @Override public void destroy() throws Exception { - if (getHttpClient() instanceof CloseableHttpClient client) { + + if (getHttpClient()instanceof CloseableHttpClient client) { client.close(); } } @@ -200,11 +220,15 @@ public void destroy() throws Exception { * these headers themselves, and HttpClient throws an exception if they have been set. */ public static class RemoveSoapHeadersInterceptor implements HttpRequestInterceptor { + @Override - public void process(HttpRequest request, EntityDetails entityDetails, HttpContext httpContext) throws HttpException, IOException { + public void process(HttpRequest request, EntityDetails entityDetails, HttpContext httpContext) + throws HttpException, IOException { + if (request.containsHeader(HttpHeaders.TRANSFER_ENCODING)) { request.removeHeaders(HttpHeaders.TRANSFER_ENCODING); } + if (request.containsHeader(HttpHeaders.CONTENT_LENGTH)) { request.removeHeaders(HttpHeaders.CONTENT_LENGTH); } diff --git a/spring-ws-core/src/test/java/org/springframework/ws/transport/http/HttpComponents5MessageSenderIntegrationTest.java b/spring-ws-core/src/test/java/org/springframework/ws/transport/http/HttpComponents5MessageSenderIntegrationTest.java index a83203983..e34d3b852 100644 --- a/spring-ws-core/src/test/java/org/springframework/ws/transport/http/HttpComponents5MessageSenderIntegrationTest.java +++ b/spring-ws-core/src/test/java/org/springframework/ws/transport/http/HttpComponents5MessageSenderIntegrationTest.java @@ -16,15 +16,19 @@ package org.springframework.ws.transport.http; -import java.io.IOException; -import java.net.URI; -import java.util.HashMap; -import java.util.Map; +import static org.assertj.core.api.AssertionsForClassTypes.*; +import static org.springframework.ws.transport.http.HttpComponents5ClientFactory.*; import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import jakarta.xml.soap.MessageFactory; + +import java.io.IOException; +import java.net.URI; +import java.util.HashMap; +import java.util.Map; + import org.apache.hc.client5.http.HttpRoute; import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager; @@ -34,15 +38,12 @@ import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.servlet.ServletContextHandler; import org.junit.jupiter.api.Test; - import org.springframework.context.support.StaticApplicationContext; import org.springframework.util.FileCopyUtils; import org.springframework.ws.soap.saaj.SaajSoapMessage; import org.springframework.ws.soap.saaj.SaajSoapMessageFactory; import org.springframework.ws.transport.WebServiceConnection; import org.springframework.ws.transport.support.FreePortScanner; -import static org.assertj.core.api.AssertionsForClassTypes.*; -import static org.springframework.ws.transport.http.HttpComponents5ClientFactory.*; class HttpComponents5MessageSenderIntegrationTest extends AbstractHttpWebServiceMessageSenderIntegrationTestCase { @@ -52,7 +53,7 @@ protected HttpComponents5MessageSender createMessageSender() { return new HttpComponents5MessageSender(); } - @Test + @Test // GH-1164 void testMaxConnections() throws Exception { final String url1 = "https://www.example.com"; @@ -103,7 +104,7 @@ void testMaxConnections() throws Exception { assertThat(poolingHttpClientConnectionManager.getMaxPerRoute(route3)).isEqualTo(10); } - @Test + @Test // GH-1164 void testContextClose() throws Exception { MessageFactory messageFactory = MessageFactory.newInstance(); @@ -138,6 +139,7 @@ void testContextClose() throws Exception { appContext.close(); } finally { + if (connection != null) { try { connection.close();