Deprecated execute methods that return an open response object in favor of execute methods with a response handler and automatic resource deallocation

httpclient5-fluent/src/main/java/org/apache/hc/client5/http/fluent/Request.java

@@ -198,7 +198,7 @@ public class Request {
         }
         final RequestConfig config = builder.build();
         localContext.setRequestConfig(config);
-        return client.execute(this.request, localContext);
+        return client.executeOpen(null, this.request, localContext);
     }
 
     public Response execute() throws IOException {

httpclient5-testing/src/test/java/org/apache/hc/client5/testing/external/CachingHttpClientCompatibilityTest.java

@@ -118,7 +118,7 @@ public class CachingHttpClientCompatibilityTest {
         {
             final HttpCacheContext context = HttpCacheContext.create();
             final HttpOptions options = new HttpOptions("*");
-            try (final ClassicHttpResponse response = client.execute(target, options, context)) {
+            try (final ClassicHttpResponse response = client.executeOpen(target, options, context)) {
                 final int code = response.getCode();
                 EntityUtils.consume(response.getEntity());
                 if (code == HttpStatus.SC_OK) {
@@ -137,7 +137,7 @@ public class CachingHttpClientCompatibilityTest {
             final Pattern linkPattern = Pattern.compile("^<(.*)>;rel=preload$");
             final List<String> links = new ArrayList<>();
             final HttpGet getRoot1 = new HttpGet("/");
-            try (ClassicHttpResponse response = client.execute(target, getRoot1, context)) {
+            try (ClassicHttpResponse response = client.executeOpen(target, getRoot1, context)) {
                 final int code = response.getCode();
                 final CacheResponseStatus cacheResponseStatus = context.getCacheResponseStatus();
                 EntityUtils.consume(response.getEntity());
@@ -158,7 +158,7 @@ public class CachingHttpClientCompatibilityTest {
 
             for (final String link: links) {
                 final HttpGet getLink = new HttpGet(link);
-                try (ClassicHttpResponse response = client.execute(target, getLink, context)) {
+                try (ClassicHttpResponse response = client.executeOpen(target, getLink, context)) {
                     final int code = response.getCode();
                     final CacheResponseStatus cacheResponseStatus = context.getCacheResponseStatus();
                     EntityUtils.consume(response.getEntity());
@@ -172,7 +172,7 @@ public class CachingHttpClientCompatibilityTest {
                 }
             }
             final HttpGet getRoot2 = new HttpGet("/");
-            try (ClassicHttpResponse response = client.execute(target, getRoot2, context)) {
+            try (ClassicHttpResponse response = client.executeOpen(target, getRoot2, context)) {
                 final int code = response.getCode();
                 final CacheResponseStatus cacheResponseStatus = context.getCacheResponseStatus();
                 EntityUtils.consume(response.getEntity());
@@ -186,7 +186,7 @@ public class CachingHttpClientCompatibilityTest {
             }
             for (final String link: links) {
                 final HttpGet getLink = new HttpGet(link);
-                try (ClassicHttpResponse response = client.execute(target, getLink, context)) {
+                try (ClassicHttpResponse response = client.executeOpen(target, getLink, context)) {
                     final int code = response.getCode();
                     final CacheResponseStatus cacheResponseStatus = context.getCacheResponseStatus();
                     EntityUtils.consume(response.getEntity());

httpclient5-testing/src/test/java/org/apache/hc/client5/testing/external/HttpClientCompatibilityTest.java

@@ -142,7 +142,7 @@ public class HttpClientCompatibilityTest {
             final HttpClientContext context = HttpClientContext.create();
             context.setCredentialsProvider(credentialsProvider);
             final HttpOptions options = new HttpOptions("*");
-            try (ClassicHttpResponse response = client.execute(target, options, context)) {
+            try (ClassicHttpResponse response = client.executeOpen(target, options, context)) {
                 final int code = response.getCode();
                 EntityUtils.consume(response.getEntity());
                 if (code == HttpStatus.SC_OK) {
@@ -162,7 +162,7 @@ public class HttpClientCompatibilityTest {
             final String[] requestUris = new String[] {"/", "/news.html", "/status.html"};
             for (final String requestUri: requestUris) {
                 final HttpGet httpGet = new HttpGet(requestUri);
-                try (ClassicHttpResponse response = client.execute(target, httpGet, context)) {
+                try (ClassicHttpResponse response = client.executeOpen(target, httpGet, context)) {
                     final int code = response.getCode();
                     EntityUtils.consume(response.getEntity());
                     if (code == HttpStatus.SC_OK) {
@@ -185,7 +185,7 @@ public class HttpClientCompatibilityTest {
             context.setCredentialsProvider(credentialsProvider);
 
             final HttpGet httpGetSecret = new HttpGet("/private/big-secret.txt");
-            try (ClassicHttpResponse response = client.execute(target, httpGetSecret, context)) {
+            try (ClassicHttpResponse response = client.executeOpen(target, httpGetSecret, context)) {
                 final int code = response.getCode();
                 EntityUtils.consume(response.getEntity());
                 if (code == HttpStatus.SC_UNAUTHORIZED) {
@@ -207,7 +207,7 @@ public class HttpClientCompatibilityTest {
             context.setCredentialsProvider(credentialsProvider);
 
             final HttpGet httpGetSecret = new HttpGet("/private/big-secret.txt");
-            try (ClassicHttpResponse response = client.execute(target, httpGetSecret, context)) {
+            try (ClassicHttpResponse response = client.executeOpen(target, httpGetSecret, context)) {
                 final int code = response.getCode();
                 EntityUtils.consume(response.getEntity());
                 if (code == HttpStatus.SC_UNAUTHORIZED) {
@@ -229,7 +229,7 @@ public class HttpClientCompatibilityTest {
             context.setCredentialsProvider(credentialsProvider);
 
             final HttpGet httpGetSecret = new HttpGet("/private/big-secret.txt");
-            try (ClassicHttpResponse response = client.execute(target, httpGetSecret, context)) {
+            try (ClassicHttpResponse response = client.executeOpen(target, httpGetSecret, context)) {
                 final int code = response.getCode();
                 EntityUtils.consume(response.getEntity());
                 if (code == HttpStatus.SC_OK) {
@@ -252,7 +252,7 @@ public class HttpClientCompatibilityTest {
 
             final HttpGet httpGetSecret = new HttpGet("/private/big-secret.txt");
             httpGetSecret.setHeader(HttpHeaders.CONNECTION, HeaderElements.CLOSE);
-            try (ClassicHttpResponse response = client.execute(target, httpGetSecret, context)) {
+            try (ClassicHttpResponse response = client.executeOpen(target, httpGetSecret, context)) {
                 final int code = response.getCode();
                 EntityUtils.consume(response.getEntity());
                 if (code == HttpStatus.SC_OK) {

httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestBasicConnectionManager.java

@@ -30,8 +30,8 @@ import org.apache.hc.client5.http.classic.methods.HttpGet;
 import org.apache.hc.client5.http.impl.io.BasicHttpClientConnectionManager;
 import org.apache.hc.core5.http.HttpHost;
 import org.apache.hc.core5.http.io.entity.EntityUtils;
-import org.junit.jupiter.api.Assertions;
 import org.junit.Test;
+import org.junit.jupiter.api.Assertions;
 
 public class TestBasicConnectionManager extends LocalServerTestBase {
 
@@ -54,10 +54,10 @@ public class TestBasicConnectionManager extends LocalServerTestBase {
 
         final HttpHost target = start();
         final HttpGet get1 = new HttpGet("/random/1024");
-        this.httpclient.execute(target, get1);
+        this.httpclient.executeOpen(target, get1, null);
         final HttpGet get2 = new HttpGet("/random/1024");
         Assertions.assertThrows(IllegalStateException.class, () ->
-                this.httpclient.execute(target, get2));
+                this.httpclient.executeOpen(target, get2, null));
     }
 
 }

httpclient5/src/main/java/org/apache/hc/client5/http/classic/HttpClient.java

@@ -27,6 +27,7 @@
 
 package org.apache.hc.client5.http.classic;
 
+import java.io.Closeable;
 import java.io.IOException;
 
 import org.apache.hc.core5.http.ClassicHttpRequest;
@@ -57,7 +58,17 @@ public interface HttpClient {
      *          or handled automatically depends on the implementation and
      *          configuration of this client.
      * @throws IOException in case of a problem or the connection was aborted
+     *
+     * @deprecated It is strongly recommended to use execute methods with {@link HttpClientResponseHandler}
+     * such as {@link #execute(ClassicHttpRequest, HttpClientResponseHandler)} in order
+     * to ensure automatic resource deallocation by the client.
+     * For special cases one can still use {@link #executeOpen(HttpHost, ClassicHttpRequest, HttpContext)}
+     * to keep the response object open after the request execution.
+     *
+     * @see #execute(ClassicHttpRequest, HttpClientResponseHandler)
+     * @see #executeOpen(HttpHost, ClassicHttpRequest, HttpContext)
      */
+    @Deprecated
     HttpResponse execute(ClassicHttpRequest request) throws IOException;
 
     /**
@@ -73,7 +84,17 @@ public interface HttpClient {
      *          or handled automatically depends on the implementation and
      *          configuration of this client.
      * @throws IOException in case of a problem or the connection was aborted
+     *
+     * @deprecated It is strongly recommended to use execute methods with {@link HttpClientResponseHandler}
+     * such as {@link #execute(ClassicHttpRequest, HttpContext, HttpClientResponseHandler)} in order
+     * to ensure automatic resource deallocation by the client.
+     * For special cases one can still use {@link #executeOpen(HttpHost, ClassicHttpRequest, HttpContext)}
+     * to keep the response object open after the request execution.
+     *
+     * @see #execute(ClassicHttpRequest, HttpContext, HttpClientResponseHandler)
+     * @see #executeOpen(HttpHost, ClassicHttpRequest, HttpContext)
      */
+    @Deprecated
     HttpResponse execute(ClassicHttpRequest request, HttpContext context) throws IOException;
 
     /**
@@ -91,7 +112,17 @@ public interface HttpClient {
      *          or handled automatically depends on the implementation and
      *          configuration of this client.
      * @throws IOException in case of a problem or the connection was aborted
+     *
+     * @deprecated It is strongly recommended to use execute methods with {@link HttpClientResponseHandler}
+     * such as {@link #execute(HttpHost, ClassicHttpRequest, HttpClientResponseHandler)} in order
+     * to ensure automatic resource deallocation by the client.
+     * For special cases one can still use {@link #executeOpen(HttpHost, ClassicHttpRequest, HttpContext)}
+     * to keep the response object open after the request execution.
+     *
+     * @see #execute(HttpHost, ClassicHttpRequest, HttpClientResponseHandler)
+     * @see #executeOpen(HttpHost, ClassicHttpRequest, HttpContext)
      */
+     @Deprecated
      ClassicHttpResponse execute(HttpHost target, ClassicHttpRequest request) throws IOException;
 
     /**
@@ -111,9 +142,47 @@ public interface HttpClient {
      *          or handled automatically depends on the implementation and
      *          configuration of this client.
      * @throws IOException in case of a problem or the connection was aborted
+     *
+     * @deprecated It is strongly recommended to use execute methods with {@link HttpClientResponseHandler}
+     * such as {@link #execute(HttpHost, ClassicHttpRequest, HttpContext, HttpClientResponseHandler)} in order
+     * to ensure automatic resource deallocation by the client.
+     * For special cases one can still use {@link #executeOpen(HttpHost, ClassicHttpRequest, HttpContext)}
+     * to keep the response object open after the request execution.
+     *
+     * @see #execute(HttpHost, ClassicHttpRequest, HttpContext, HttpClientResponseHandler)
+     * @see #executeOpen(HttpHost, ClassicHttpRequest, HttpContext)
      */
+    @Deprecated
     HttpResponse execute(HttpHost target, ClassicHttpRequest request, HttpContext context) throws IOException;
 
+    /**
+     * Executes the request and opens the response stream using the given context.
+     *
+     * @param target    the target host for the request.
+     *                  Implementations may accept {@code null}
+     *                  if they can still determine a route, for example
+     *                  to a default target or by inspecting the request.
+     * @param request   the request to execute
+     * @param context   the context to use for the execution, or
+     *                  {@code null} to use the default context
+     *
+     * @return  the response to the request. This is always a final response,
+     *          never an intermediate response with an 1xx status code.
+     *          Whether redirects or authentication challenges will be returned
+     *          or handled automatically depends on the implementation and
+     *          configuration of this client.
+     *          The response returned by this method must be closed with
+     *          {@link Closeable#close()} in order ensure deallocation
+     *          of system resources.
+     * @throws IOException in case of a problem or the connection was aborted
+     *
+     * @since 5.2
+     */
+    @SuppressWarnings("deprecation")
+    default ClassicHttpResponse executeOpen(HttpHost target, ClassicHttpRequest request, HttpContext context) throws IOException {
+        return (ClassicHttpResponse) execute(target, request, context);
+    }
+
     /**
      * Executes HTTP request using the default context and processes the
      * response using the given response handler.

httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/CloseableHttpClient.java

@@ -35,6 +35,7 @@ import org.apache.hc.client5.http.routing.RoutingSupport;
 import org.apache.hc.core5.annotation.Contract;
 import org.apache.hc.core5.annotation.ThreadingBehavior;
 import org.apache.hc.core5.http.ClassicHttpRequest;
+import org.apache.hc.core5.http.ClassicHttpResponse;
 import org.apache.hc.core5.http.HttpEntity;
 import org.apache.hc.core5.http.HttpException;
 import org.apache.hc.core5.http.HttpHost;
@@ -57,23 +58,7 @@ public abstract class CloseableHttpClient implements HttpClient, ModalCloseable
     private static final Logger LOG = LoggerFactory.getLogger(CloseableHttpClient.class);
 
     protected abstract CloseableHttpResponse doExecute(HttpHost target, ClassicHttpRequest request,
-                                                     HttpContext context) throws IOException;
+                                                       HttpContext context) throws IOException;
-
-    @Override
-    public CloseableHttpResponse execute(
-            final HttpHost target,
-            final ClassicHttpRequest request,
-            final HttpContext context) throws IOException {
-        return doExecute(target, request, context);
-    }
-
-    @Override
-    public CloseableHttpResponse execute(
-            final ClassicHttpRequest request,
-            final HttpContext context) throws IOException {
-        Args.notNull(request, "HTTP request");
-        return doExecute(determineTarget(request), request, context);
-    }
 
     private static HttpHost determineTarget(final ClassicHttpRequest request) throws ClientProtocolException {
         try {
@@ -83,12 +68,72 @@ public abstract class CloseableHttpClient implements HttpClient, ModalCloseable
         }
     }
 
+    /**
+     * @deprecated It is strongly recommended to use execute methods with {@link HttpClientResponseHandler}
+     * such as {@link #execute(HttpHost, ClassicHttpRequest, HttpContext, HttpClientResponseHandler)} in order
+     * to ensure automatic resource deallocation by the client.
+     * For special cases one can still use {@link #executeOpen(HttpHost, ClassicHttpRequest, HttpContext)}
+     * to keep the response object open after the request execution.
+     *
+     * @see #execute(HttpHost, ClassicHttpRequest, HttpContext, HttpClientResponseHandler)
+     * @see #executeOpen(HttpHost, ClassicHttpRequest, HttpContext)
+     */
+    @Deprecated
+    @Override
+    public CloseableHttpResponse execute(
+            final HttpHost target,
+            final ClassicHttpRequest request,
+            final HttpContext context) throws IOException {
+        return doExecute(target, request, context);
+    }
+
+    /**
+     * @deprecated It is strongly recommended to use execute methods with {@link HttpClientResponseHandler}
+     * such as {@link #execute(ClassicHttpRequest, HttpContext, HttpClientResponseHandler)} in order
+     * to ensure automatic resource deallocation by the client.
+     * For special cases one can still use {@link #executeOpen(HttpHost, ClassicHttpRequest, HttpContext)}
+     * to keep the response object open after the request execution.
+     *
+     * @see #execute(ClassicHttpRequest, HttpContext, HttpClientResponseHandler)
+     * @see #executeOpen(HttpHost, ClassicHttpRequest, HttpContext)
+     */
+    @Deprecated
+    @Override
+    public CloseableHttpResponse execute(
+            final ClassicHttpRequest request,
+            final HttpContext context) throws IOException {
+        Args.notNull(request, "HTTP request");
+        return doExecute(determineTarget(request), request, context);
+    }
+
+    /**
+     * @deprecated It is strongly recommended to use execute methods with {@link HttpClientResponseHandler}
+     * such as {@link #execute(ClassicHttpRequest, HttpClientResponseHandler)} in order
+     * to ensure automatic resource deallocation by the client.
+     * For special cases one can still use {@link #executeOpen(HttpHost, ClassicHttpRequest, HttpContext)}
+     * to keep the response object open after the request execution.
+     *
+     * @see #execute(ClassicHttpRequest, HttpClientResponseHandler)
+     * @see #executeOpen(HttpHost, ClassicHttpRequest, HttpContext)
+     */
+    @Deprecated
     @Override
     public CloseableHttpResponse execute(
             final ClassicHttpRequest request) throws IOException {
-        return execute(request, (HttpContext) null);
+        return doExecute(determineTarget(request), request, null);
     }
 
+    /**
+     * @deprecated It is strongly recommended to use execute methods with {@link HttpClientResponseHandler}
+     * such as {@link #execute(HttpHost, ClassicHttpRequest, HttpClientResponseHandler)} in order
+     * to ensure automatic resource deallocation by the client.
+     * For special cases one can still use {@link #executeOpen(HttpHost, ClassicHttpRequest, HttpContext)}
+     * to keep the response object open after the request execution.
+     *
+     * @see #execute(HttpHost, ClassicHttpRequest, HttpClientResponseHandler)
+     * @see #executeOpen(HttpHost, ClassicHttpRequest, HttpContext)
+     */
+    @Deprecated
     @Override
     public CloseableHttpResponse execute(
             final HttpHost target,
@@ -197,7 +242,7 @@ public abstract class CloseableHttpClient implements HttpClient, ModalCloseable
             final HttpClientResponseHandler<? extends T> responseHandler) throws IOException {
         Args.notNull(responseHandler, "Response handler");
 
-        try (final CloseableHttpResponse response = execute(target, request, context)) {
+        try (final ClassicHttpResponse response = doExecute(target, request, context)) {
             try {
                 final T result = responseHandler.handleResponse(response);
                 final HttpEntity entity = response.getEntity();

httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestCloseableHttpClient.java

@@ -81,7 +81,9 @@ public class TestCloseableHttpClient {
     @Test
     public void testExecuteRequestAbsoluteURI() throws Exception {
         final HttpGet httpget = new HttpGet("https://somehost:444/stuff");
-        client.execute(httpget);
+        Mockito.when(client.doExecute(
+                Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(response);
+        client.execute(httpget, response -> null);
 
         Mockito.verify(client).doExecute(
                 Mockito.eq(new HttpHost("https", "somehost", 444)),
@@ -92,7 +94,9 @@ public class TestCloseableHttpClient {
     @Test
     public void testExecuteRequestRelativeURI() throws Exception {
         final HttpGet httpget = new HttpGet("/stuff");
-        client.execute(httpget);
+        Mockito.when(client.doExecute(
+                Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(response);
+        client.execute(httpget, response -> null);
 
         Mockito.verify(client).doExecute(
                 (HttpHost) Mockito.isNull(),
@@ -100,17 +104,6 @@ public class TestCloseableHttpClient {
                 (HttpContext) Mockito.isNull());
     }
 
-    @Test
-    public void testExecuteRequest() throws Exception {
-        final HttpGet httpget = new HttpGet("https://somehost:444/stuff");
-
-        Mockito.when(client.doExecute(
-                new HttpHost("https", "somehost", 444), httpget, null)).thenReturn(response);
-
-        final CloseableHttpResponse result = client.execute(httpget);
-        Assertions.assertSame(response, result);
-    }
-
     @Test
     public void testExecuteRequestHandleResponse() throws Exception {
         final HttpGet httpget = new HttpGet("https://somehost:444/stuff");

httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestInternalHttpClient.java

@@ -46,6 +46,7 @@ import org.apache.hc.core5.http.HttpException;
 import org.apache.hc.core5.http.HttpHost;
 import org.apache.hc.core5.http.config.Lookup;
 import org.apache.hc.core5.http.impl.io.HttpRequestExecutor;
+import org.apache.hc.core5.http.message.BasicClassicHttpResponse;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -101,8 +102,11 @@ public class TestInternalHttpClient {
         Mockito.when(routePlanner.determineRoute(
                 Mockito.eq(new HttpHost("somehost")),
                 Mockito.<HttpClientContext>any())).thenReturn(route);
+        Mockito.when(execChain.execute(
+                Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(
+                CloseableHttpResponse.adapt(new BasicClassicHttpResponse(200)));
 
-        client.execute(httpget);
+        client.execute(httpget, response -> null);
 
         Mockito.verify(execChain).execute(
                 Mockito.any(),
@@ -118,13 +122,16 @@ public class TestInternalHttpClient {
         Mockito.when(routePlanner.determineRoute(
                 Mockito.eq(new HttpHost("somehost")),
                 Mockito.<HttpClientContext>any())).thenReturn(route);
+        Mockito.when(execChain.execute(
+                Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(
+                CloseableHttpResponse.adapt(new BasicClassicHttpResponse(200)));
         Mockito.when(execChain.execute(
                 Mockito.any(),
                 Mockito.any(),
                 Mockito.any())).thenThrow(new HttpException());
 
         Assertions.assertThrows(ClientProtocolException.class, () ->
-                client.execute(httpget));
+                client.execute(httpget, response -> null));
     }
 
     @Test
@@ -135,9 +142,12 @@ public class TestInternalHttpClient {
         Mockito.when(routePlanner.determineRoute(
                 Mockito.eq(new HttpHost("somehost")),
                 Mockito.<HttpClientContext>any())).thenReturn(route);
+        Mockito.when(execChain.execute(
+                Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(
+                CloseableHttpResponse.adapt(new BasicClassicHttpResponse(200)));
 
         final HttpClientContext context = HttpClientContext.create();
-        client.execute(httpget, context);
+        client.execute(httpget, context, response -> null);
 
         Assertions.assertSame(cookieSpecRegistry, context.getCookieSpecRegistry());
         Assertions.assertSame(authSchemeRegistry, context.getAuthSchemeRegistry());
@@ -154,11 +164,14 @@ public class TestInternalHttpClient {
         Mockito.when(routePlanner.determineRoute(
                 Mockito.eq(new HttpHost("somehost")),
                 Mockito.<HttpClientContext>any())).thenReturn(route);
+        Mockito.when(execChain.execute(
+                Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(
+                CloseableHttpResponse.adapt(new BasicClassicHttpResponse(200)));
 
         final RequestConfig config = RequestConfig.custom().build();
         httpget.setConfig(config);
         final HttpClientContext context = HttpClientContext.create();
-        client.execute(httpget, context);
+        client.execute(httpget, context, response -> null);
 
         Assertions.assertSame(config, context.getRequestConfig());
     }
@@ -171,6 +184,9 @@ public class TestInternalHttpClient {
         Mockito.when(routePlanner.determineRoute(
                 Mockito.eq(new HttpHost("somehost")),
                 Mockito.<HttpClientContext>any())).thenReturn(route);
+        Mockito.when(execChain.execute(
+                Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(
+                CloseableHttpResponse.adapt(new BasicClassicHttpResponse(200)));
 
         final HttpClientContext context = HttpClientContext.create();
 
@@ -186,7 +202,7 @@ public class TestInternalHttpClient {
         context.setCredentialsProvider(localCredentialsProvider);
         context.setRequestConfig(localConfig);
 
-        client.execute(httpget, context);
+        client.execute(httpget, context, response -> null);
 
         Assertions.assertSame(localCookieSpecRegistry, context.getCookieSpecRegistry());
         Assertions.assertSame(localAuthSchemeRegistry, context.getAuthSchemeRegistry());