Merge pull request #350 from ok2c/httpclient_4_3

* httpclient_4_3:
  Post SPR-8804 optimizations: better use of HC 4.3 APIs

spring-web/src/main/java/org/springframework/http/client/HttpComponentsAsyncClientHttpRequest.java

@@ -27,8 +27,8 @@ import org.apache.http.HttpEntity;
 import org.apache.http.HttpEntityEnclosingRequest;
 import org.apache.http.HttpResponse;
 import org.apache.http.client.methods.HttpUriRequest;
-import org.apache.http.entity.ByteArrayEntity;
 import org.apache.http.nio.client.HttpAsyncClient;
+import org.apache.http.nio.entity.NByteArrayEntity;
 import org.apache.http.protocol.HttpContext;
 
 import org.springframework.http.HttpHeaders;
@@ -79,7 +79,7 @@ final class HttpComponentsAsyncClientHttpRequest extends AbstractBufferingAsyncC
 		if (this.httpRequest instanceof HttpEntityEnclosingRequest) {
 			HttpEntityEnclosingRequest entityEnclosingRequest =
 					(HttpEntityEnclosingRequest) this.httpRequest;
-			HttpEntity requestEntity = new ByteArrayEntity(bufferedOutput);
+			HttpEntity requestEntity = new NByteArrayEntity(bufferedOutput);
 			entityEnclosingRequest.setEntity(requestEntity);
 		}
 
@@ -117,14 +117,14 @@ final class HttpComponentsAsyncClientHttpRequest extends AbstractBufferingAsyncC
 		public ClientHttpResponse get()
 				throws InterruptedException, ExecutionException {
 			HttpResponse response = futureResponse.get();
-			return new HttpComponentsClientHttpResponse(response);
+			return new HttpComponentsAsyncClientHttpResponse(response);
 		}
 
 		@Override
 		public ClientHttpResponse get(long timeout, TimeUnit unit)
 				throws InterruptedException, ExecutionException, TimeoutException {
 			HttpResponse response = futureResponse.get(timeout, unit);
-			return new HttpComponentsClientHttpResponse(response);
+			return new HttpComponentsAsyncClientHttpResponse(response);
 		}
 
 	}

spring-web/src/main/java/org/springframework/http/client/HttpComponentsAsyncClientHttpRequestFactory.java

@@ -20,10 +20,16 @@ import java.io.IOException;
 import java.net.URI;
 
 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;
+import org.apache.http.impl.nio.client.CloseableHttpAsyncClient;
 import org.apache.http.impl.nio.client.HttpAsyncClients;
 import org.apache.http.nio.client.HttpAsyncClient;
 import org.apache.http.nio.reactor.IOReactorStatus;
+import org.apache.http.protocol.HttpContext;
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.http.HttpMethod;
 import org.springframework.util.Assert;
@@ -41,7 +47,7 @@ public class HttpComponentsAsyncClientHttpRequestFactory
 		extends HttpComponentsClientHttpRequestFactory
 		implements AsyncClientHttpRequestFactory, InitializingBean {
 
-	private HttpAsyncClient httpAsyncClient;
+	private CloseableHttpAsyncClient httpAsyncClient;
 
 
 	/**
@@ -49,7 +55,7 @@ public class HttpComponentsAsyncClientHttpRequestFactory
 	 * with a default {@link HttpAsyncClient} and {@link HttpClient}.
 	 */
 	public HttpComponentsAsyncClientHttpRequestFactory() {
-		this(HttpAsyncClients.createDefault());
+		this(HttpAsyncClients.createSystem());
 	}
 
 	/**
@@ -57,7 +63,7 @@ public class HttpComponentsAsyncClientHttpRequestFactory
 	 * with the given {@link HttpAsyncClient} instance and a default {@link HttpClient}.
 	 * @param httpAsyncClient the HttpAsyncClient instance to use for this request factory
 	 */
-	public HttpComponentsAsyncClientHttpRequestFactory(HttpAsyncClient httpAsyncClient) {
+	public HttpComponentsAsyncClientHttpRequestFactory(CloseableHttpAsyncClient httpAsyncClient) {
 		super();
 		Assert.notNull(httpAsyncClient, "'httpAsyncClient' must not be null");
 		this.httpAsyncClient = httpAsyncClient;
@@ -69,8 +75,8 @@ public class HttpComponentsAsyncClientHttpRequestFactory
 	 * @param httpClient the HttpClient instance to use for this request factory
 	 * @param httpAsyncClient the HttpAsyncClient instance to use for this request factory
 	 */
-	public HttpComponentsAsyncClientHttpRequestFactory(HttpClient httpClient,
-			HttpAsyncClient httpAsyncClient) {
+	public HttpComponentsAsyncClientHttpRequestFactory(CloseableHttpClient httpClient,
+                                                       CloseableHttpAsyncClient httpAsyncClient) {
 		super(httpClient);
 		Assert.notNull(httpAsyncClient, "'httpAsyncClient' must not be null");
 		this.httpAsyncClient = httpAsyncClient;
@@ -80,7 +86,7 @@ public class HttpComponentsAsyncClientHttpRequestFactory
 	 * Set the {@code HttpClient} used for
 	 * {@linkplain #createAsyncRequest(java.net.URI, org.springframework.http.HttpMethod) asynchronous execution}.
 	 */
-	public void setHttpAsyncClient(HttpAsyncClient httpAsyncClient) {
+	public void setHttpAsyncClient(CloseableHttpAsyncClient httpAsyncClient) {
 		this.httpAsyncClient = httpAsyncClient;
 	}
 
@@ -88,7 +94,7 @@ public class HttpComponentsAsyncClientHttpRequestFactory
 	 * Return the {@code HttpClient} used for
 	 * {@linkplain #createAsyncRequest(URI, HttpMethod) asynchronous execution}.
 	 */
-	public HttpAsyncClient getHttpAsyncClient() {
+	public CloseableHttpAsyncClient getHttpAsyncClient() {
 		return httpAsyncClient;
 	}
 
@@ -98,7 +104,7 @@ public class HttpComponentsAsyncClientHttpRequestFactory
 	}
 
 	private void startAsyncClient() {
-		HttpAsyncClient asyncClient = getHttpAsyncClient();
+        CloseableHttpAsyncClient asyncClient = getHttpAsyncClient();
 		if (asyncClient.getStatus() != IOReactorStatus.ACTIVE) {
 			asyncClient.start();
 		}
@@ -111,8 +117,23 @@ public class HttpComponentsAsyncClientHttpRequestFactory
 		startAsyncClient();
 		HttpUriRequest httpRequest = createHttpUriRequest(httpMethod, uri);
 		postProcessHttpRequest(httpRequest);
-		return new HttpComponentsAsyncClientHttpRequest(asyncClient, httpRequest,
-				createHttpContext(httpMethod, uri));
+        HttpContext context = createHttpContext(httpMethod, uri);
+        if (context == null) {
+            context = HttpClientContext.create();
+        }
+        // Request configuration not set in the context
+        if (context.getAttribute(HttpClientContext.REQUEST_CONFIG) == null) {
+            // Use request configuration given by the user, when available
+            RequestConfig config = null;
+            if (httpRequest instanceof Configurable) {
+                config = ((Configurable) httpRequest).getConfig();
+            }
+            if (config == null) {
+                config = RequestConfig.DEFAULT;
+            }
+            context.setAttribute(HttpClientContext.REQUEST_CONFIG, config);
+        }
+		return new HttpComponentsAsyncClientHttpRequest(asyncClient, httpRequest, context);
 	}
 
 	@Override
@@ -121,7 +142,7 @@ public class HttpComponentsAsyncClientHttpRequestFactory
 			super.destroy();
 		}
 		finally {
-			getHttpAsyncClient().shutdown();
+			getHttpAsyncClient().close();
 		}
 	}
 }

spring-web/src/main/java/org/springframework/http/client/HttpComponentsAsyncClientHttpResponse.java

+/*
+ * Copyright 2002-2012 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.http.client;
+
+import org.apache.http.Header;
+import org.apache.http.HttpEntity;
+import org.apache.http.HttpResponse;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.util.EntityUtils;
+import org.springframework.http.HttpHeaders;
+
+import java.io.IOException;
+import java.io.InputStream;
+
+/**
+ * {@link ClientHttpResponse} implementation that uses
+ * Apache HttpComponents HttpClient to execute requests.
+ *
+ * <p>Created via the {@link HttpComponentsAsyncClientHttpRequest}.
+ *
+ * @author Oleg Kalnichevski
+ * @author Arjen Poutsma
+ * @since 3.1
+ * @see HttpComponentsAsyncClientHttpRequest#executeAsync()
+ */
+final class HttpComponentsAsyncClientHttpResponse extends AbstractClientHttpResponse {
+
+	private final HttpResponse httpResponse;
+
+	private HttpHeaders headers;
+
+
+	HttpComponentsAsyncClientHttpResponse(HttpResponse httpResponse) {
+		this.httpResponse = httpResponse;
+	}
+
+
+	@Override
+	public int getRawStatusCode() throws IOException {
+		return this.httpResponse.getStatusLine().getStatusCode();
+	}
+
+	@Override
+	public String getStatusText() throws IOException {
+		return this.httpResponse.getStatusLine().getReasonPhrase();
+	}
+
+	@Override
+	public HttpHeaders getHeaders() {
+		if (this.headers == null) {
+			this.headers = new HttpHeaders();
+			for (Header header : this.httpResponse.getAllHeaders()) {
+				this.headers.add(header.getName(), header.getValue());
+			}
+		}
+		return this.headers;
+	}
+
+	@Override
+	public InputStream getBody() throws IOException {
+		HttpEntity entity = this.httpResponse.getEntity();
+		return entity != null ? entity.getContent() : null;
+	}
+
+	@Override
+	public void close() {
+        // HTTP responses returned by async HTTP client
+        // are not bound to an active connection and
+        // do not have to deallocate any resources
+	}
+
+}

spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequest.java

@@ -21,6 +21,8 @@ import java.net.URI;
 import java.util.List;
 import java.util.Map;
 
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.impl.client.CloseableHttpClient;
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpMethod;
 
@@ -46,14 +48,14 @@ import org.apache.http.protocol.HttpContext;
  */
 final class HttpComponentsClientHttpRequest extends AbstractBufferingClientHttpRequest {
 
-	private final HttpClient httpClient;
+	private final CloseableHttpClient httpClient;
 
 	private final HttpUriRequest httpRequest;
 
 	private final HttpContext httpContext;
 
 
-	public HttpComponentsClientHttpRequest(HttpClient httpClient, HttpUriRequest httpRequest, HttpContext httpContext) {
+	public HttpComponentsClientHttpRequest(CloseableHttpClient httpClient, HttpUriRequest httpRequest, HttpContext httpContext) {
 		this.httpClient = httpClient;
 		this.httpRequest = httpRequest;
 		this.httpContext = httpContext;
@@ -81,7 +83,7 @@ final class HttpComponentsClientHttpRequest extends AbstractBufferingClientHttpR
 			HttpEntity requestEntity = new ByteArrayEntity(bufferedOutput);
 			entityEnclosingRequest.setEntity(requestEntity);
 		}
-		HttpResponse httpResponse =
+		CloseableHttpResponse httpResponse =
 				this.httpClient.execute(this.httpRequest, this.httpContext);
 		return new HttpComponentsClientHttpResponse(httpResponse);
 	}

spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactory.java

@@ -20,6 +20,8 @@ import java.io.IOException;
 import java.net.URI;
 
 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.HttpDelete;
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.methods.HttpHead;
@@ -29,8 +31,9 @@ import org.apache.http.client.methods.HttpPost;
 import org.apache.http.client.methods.HttpPut;
 import org.apache.http.client.methods.HttpTrace;
 import org.apache.http.client.methods.HttpUriRequest;
+import org.apache.http.client.protocol.HttpClientContext;
+import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.impl.client.HttpClients;
-import org.apache.http.params.CoreConnectionPNames;
 import org.apache.http.protocol.HttpContext;
 
 import org.springframework.beans.factory.DisposableBean;
@@ -52,7 +55,9 @@ import org.springframework.util.Assert;
 public class HttpComponentsClientHttpRequestFactory
 		implements ClientHttpRequestFactory, DisposableBean {
 
-	private HttpClient httpClient;
+	private int connectTimeout;
+    private int socketTimeout;
+    private CloseableHttpClient httpClient;
 
 	private boolean bufferRequestBody = true;
 
@@ -62,7 +67,7 @@ public class HttpComponentsClientHttpRequestFactory
 	 * a default {@link HttpClient}.
 	 */
 	public HttpComponentsClientHttpRequestFactory() {
-		this(HttpClients.createDefault());
+		this(HttpClients.createSystem());
 	}
 
 	/**
@@ -70,24 +75,24 @@ public class HttpComponentsClientHttpRequestFactory
 	 * with the given {@link HttpClient} instance.
 	 * @param httpClient the HttpClient instance to use for this request factory
 	 */
-	public HttpComponentsClientHttpRequestFactory(HttpClient httpClient) {
+	public HttpComponentsClientHttpRequestFactory(CloseableHttpClient httpClient) {
 		Assert.notNull(httpClient, "'httpClient' must not be null");
-		this.httpClient = httpClient;
+        this.httpClient = httpClient;
 	}
 
-	/**
-	 * Set the {@code HttpClient} used for
-	 * {@linkplain #createRequest(URI, HttpMethod) synchronous execution}.
-	 */
-	public void setHttpClient(HttpClient httpClient) {
-		this.httpClient = httpClient;
-	}
+    /**
+     * Set the {@code HttpClient} used for
+     * {@linkplain #createRequest(URI, HttpMethod) synchronous execution}.
+     */
+    public void setHttpClient(CloseableHttpClient httpClient) {
+        this.httpClient = httpClient;
+    }
 
-	/**
+    /**
 	 * Return the {@code HttpClient} used for
 	 * {@linkplain #createRequest(URI, HttpMethod) synchronous execution}.
 	 */
-	public HttpClient getHttpClient() {
+	public CloseableHttpClient getHttpClient() {
 		return this.httpClient;
 	}
 
@@ -95,24 +100,20 @@ public class HttpComponentsClientHttpRequestFactory
 	 * Set the connection timeout for the underlying HttpClient.
 	 * A timeout value of 0 specifies an infinite timeout.
 	 * @param timeout the timeout value in milliseconds
-	 * @deprecated With no direct replacement
 	 */
-	@Deprecated
 	public void setConnectTimeout(int timeout) {
 		Assert.isTrue(timeout >= 0, "Timeout must be a non-negative value");
-		getHttpClient().getParams().setIntParameter(CoreConnectionPNames.CONNECTION_TIMEOUT, timeout);
+        this.connectTimeout = timeout;
 	}
 
 	/**
 	 * Set the socket read timeout for the underlying HttpClient.
 	 * A timeout value of 0 specifies an infinite timeout.
 	 * @param timeout the timeout value in milliseconds
-	 * @deprecated With no direct replacement
 	 */
-	@Deprecated
 	public void setReadTimeout(int timeout) {
 		Assert.isTrue(timeout >= 0, "Timeout must be a non-negative value");
-		getHttpClient().getParams().setIntParameter(CoreConnectionPNames.SO_TIMEOUT, timeout);
+        this.socketTimeout= timeout;
 	}
 
 	/**
@@ -136,18 +137,39 @@ public class HttpComponentsClientHttpRequestFactory
 
 	@Override
 	public ClientHttpRequest createRequest(URI uri, HttpMethod httpMethod) throws IOException {
-		HttpClient client = getHttpClient();
+		CloseableHttpClient client = getHttpClient();
 		Assert.state(client != null,
 				"Synchronous execution requires an HttpClient to be set");
 		HttpUriRequest httpRequest = createHttpUriRequest(httpMethod, uri);
 		postProcessHttpRequest(httpRequest);
+        HttpContext context = createHttpContext(httpMethod, uri);
+        if (context == null) {
+            context = HttpClientContext.create();
+        }
+        // Request configuration not set in the context
+        if (context.getAttribute(HttpClientContext.REQUEST_CONFIG) == null) {
+            // Use request configuration given by the user, when available
+            RequestConfig config = null;
+            if (httpRequest instanceof Configurable) {
+                config = ((Configurable) httpRequest).getConfig();
+            }
+            if (config == null) {
+                if (this.socketTimeout > 0 || this.connectTimeout > 0) {
+                    config = RequestConfig.custom()
+                            .setConnectTimeout(this.connectTimeout)
+                            .setSocketTimeout(this.socketTimeout)
+                            .build();
+                } else {
+                    config = RequestConfig.DEFAULT;
+                }
+            }
+            context.setAttribute(HttpClientContext.REQUEST_CONFIG, config);
+        }
 		if (bufferRequestBody) {
-			return new HttpComponentsClientHttpRequest(client, httpRequest,
-					createHttpContext(httpMethod, uri));
+			return new HttpComponentsClientHttpRequest(client, httpRequest, context);
 		}
 		else {
-			return new HttpComponentsStreamingClientHttpRequest(client,
-					httpRequest, createHttpContext(httpMethod, uri));
+			return new HttpComponentsStreamingClientHttpRequest(client, httpRequest, context);
 		}
 	}
 
@@ -202,12 +224,12 @@ public class HttpComponentsClientHttpRequestFactory
 
 	/**
 	 * Shutdown hook that closes the underlying
-	 * {@link org.apache.http.conn.ClientConnectionManager ClientConnectionManager}'s
+	 * {@link org.apache.http.conn.HttpClientConnectionManager ClientConnectionManager}'s
 	 * connection pool, if any.
 	 */
 	@Override
 	public void destroy() throws Exception {
-		getHttpClient().getConnectionManager().shutdown();
-	}
+        httpClient.close();
+    }
 
 }

spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpResponse.java

@@ -21,7 +21,7 @@ import java.io.InputStream;
 
 import org.apache.http.Header;
 import org.apache.http.HttpEntity;
-import org.apache.http.HttpResponse;
+import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.util.EntityUtils;
 
 import org.springframework.http.HttpHeaders;
@@ -39,12 +39,12 @@ import org.springframework.http.HttpHeaders;
  */
 final class HttpComponentsClientHttpResponse extends AbstractClientHttpResponse {
 
-	private final HttpResponse httpResponse;
+	private final CloseableHttpResponse httpResponse;
 
 	private HttpHeaders headers;
 
 
-	HttpComponentsClientHttpResponse(HttpResponse httpResponse) {
+	HttpComponentsClientHttpResponse(CloseableHttpResponse httpResponse) {
 		this.httpResponse = httpResponse;
 	}
 
@@ -78,16 +78,18 @@ final class HttpComponentsClientHttpResponse extends AbstractClientHttpResponse
 
 	@Override
 	public void close() {
-		HttpEntity entity = this.httpResponse.getEntity();
-		if (entity != null) {
-			try {
-				// Release underlying connection back to the connection manager
-				EntityUtils.consume(entity);
-			}
-			catch (IOException e) {
-				// ignore
-			}
-		}
+        // Release underlying connection back to the connection manager
+        try {
+            try {
+                // Attempt to keep connection alive by consuming its remaining content
+                EntityUtils.consume(this.httpResponse.getEntity());
+            } finally {
+                // Paranoia
+                this.httpResponse.close();
+            }
+        }
+        catch (IOException ignore) {
+        }
 	}
 
 }

spring-web/src/main/java/org/springframework/http/client/HttpComponentsStreamingClientHttpRequest.java

@@ -24,9 +24,9 @@ import java.net.URI;
 import org.apache.http.Header;
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpEntityEnclosingRequest;
-import org.apache.http.HttpResponse;
-import org.apache.http.client.HttpClient;
+import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpUriRequest;
+import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.message.BasicHeader;
 import org.apache.http.protocol.HttpContext;
 
@@ -49,7 +49,7 @@ import org.springframework.http.StreamingHttpOutputMessage;
 final class HttpComponentsStreamingClientHttpRequest extends AbstractClientHttpRequest
 		implements StreamingHttpOutputMessage {
 
-	private final HttpClient httpClient;
+	private final CloseableHttpClient httpClient;
 
 	private final HttpUriRequest httpRequest;
 
@@ -57,7 +57,7 @@ final class HttpComponentsStreamingClientHttpRequest extends AbstractClientHttpR
 
 	private Body body;
 
-	public HttpComponentsStreamingClientHttpRequest(HttpClient httpClient,
+	public HttpComponentsStreamingClientHttpRequest(CloseableHttpClient httpClient,
 			HttpUriRequest httpRequest, HttpContext httpContext) {
 		this.httpClient = httpClient;
 		this.httpRequest = httpRequest;
@@ -97,7 +97,7 @@ final class HttpComponentsStreamingClientHttpRequest extends AbstractClientHttpR
 			HttpEntity requestEntity = new StreamingHttpEntity(getHeaders(), body);
 			entityEnclosingRequest.setEntity(requestEntity);
 		}
-		HttpResponse httpResponse =
+		CloseableHttpResponse httpResponse =
 				this.httpClient.execute(this.httpRequest, this.httpContext);
 		return new HttpComponentsClientHttpResponse(httpResponse);
 	}