Upgrade to Apache HttpClient 5.x (#439)

* Apache HttpClient 5.x * don't stream - buffer * Update .github/workflows/ci.yml * Update README.md * imports cleanup * code cleanup
zalando-nakadi · Nov 2, 2023 · 284e534 · 284e534
1 parent 1607ea1
commit 284e534
Show file tree

Hide file tree

Showing 11 changed files with 64 additions and 77 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -29,6 +29,6 @@ jobs:
           files: ./build/reports/jacoco/jacocoAggregateReport/jacocoAggregateReport.xml
           flags: unittests
       - name: Test oldest supported library versions
-        run: gradle build -Pjackson.version=2.8.0 -Pspring.version=6.0.0 -Pspringboot.version=3.0.0 -Pokhttp.version=3.3.0 -Papachehttp.version=4.4
+        run: gradle build -Pjackson.version=2.8.0 -Pspring.version=6.0.0 -Pspringboot.version=3.0.0 -Pokhttp.version=3.3.0 -Papachehttp.version=5.0
       - name: Test with latest library (minor) updates
-        run: gradle build -Pjackson.version=2.+ -Pspring.version=6.+ -Pspringboot.version=3.+ -Pokhttp.version=4.+ -Papachehttp.version=4.+ -Pjdk.version=21
+        run: gradle build -Pjackson.version=2.+ -Pspring.version=6.+ -Pspringboot.version=3.+ -Pokhttp.version=4.+ -Papachehttp.version=5.+ -Pjdk.version=21
diff --git a/README.md b/README.md
@@ -422,7 +422,7 @@ Although Fahrschein is using fixed dependency versions, it is integration-tested
 | Spring Core       | 6.0.0    | 6.+    |
 | Spring Boot       | 3.0.0    | 3.+    |
 | okHttp            | 3.3.0    | 4.+    |
-| Apache HttpClient | 4.4      | 4.+    |
+| Apache HttpClient | 5.0      | 5.+    |
 
 ## Content-Compression
 

diff --git a/fahrschein-e2e-test/src/test/java/org/zalando/fahrschein/e2e/NakadiClientEnd2EndTest.java b/fahrschein-e2e-test/src/test/java/org/zalando/fahrschein/e2e/NakadiClientEnd2EndTest.java
@@ -12,8 +12,8 @@
 import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
 import okhttp3.OkHttpClient;
 import okhttp3.logging.HttpLoggingInterceptor;
-import org.apache.http.impl.client.CloseableHttpClient;
-import org.apache.http.impl.client.HttpClients;
+import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
+import org.apache.hc.client5.http.impl.classic.HttpClients;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.MethodSource;
 import org.mockito.Mockito;

diff --git a/...schein-e2e-test/src/test/java/org/zalando/fahrschein/e2e/NakadiTestWithDockerCompose.java b/...schein-e2e-test/src/test/java/org/zalando/fahrschein/e2e/NakadiTestWithDockerCompose.java
@@ -1,12 +1,12 @@
 package org.zalando.fahrschein.e2e;
 
-import org.apache.http.HttpEntity;
-import org.apache.http.client.HttpResponseException;
-import org.apache.http.client.methods.HttpPost;
-import org.apache.http.entity.StringEntity;
-import org.apache.http.impl.client.BasicResponseHandler;
-import org.apache.http.impl.client.CloseableHttpClient;
-import org.apache.http.impl.client.HttpClients;
+import org.apache.hc.client5.http.HttpResponseException;
+import org.apache.hc.client5.http.classic.methods.HttpPost;
+import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
+import org.apache.hc.client5.http.impl.classic.HttpClients;
+import org.apache.hc.core5.http.HttpEntity;
+import org.apache.hc.core5.http.io.entity.EntityUtils;
+import org.apache.hc.core5.http.io.entity.StringEntity;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.testcontainers.containers.DockerComposeContainer;
@@ -92,7 +92,14 @@ private String postJson(HttpEntity e, URI uri) throws IOException {
         HttpPost post = new HttpPost(uri);
         post.setEntity(e);
         post.setHeader("Content-Type", "application/json");
-        return httpClient.execute(post, new BasicResponseHandler());
+        return httpClient.execute(post, response -> {
+            if (response.getCode() >= 300) {
+                EntityUtils.consume(response.getEntity());
+                throw new HttpResponseException(response.getCode(), response.getReasonPhrase());
+            } else {
+                return response.getEntity() == null ? null : EntityUtils.toString(response.getEntity());
+            }
+        });
     }
 
 }
diff --git a/fahrschein-http-apache/build.gradle b/fahrschein-http-apache/build.gradle
@@ -5,7 +5,7 @@ plugins {
 
 dependencies {
     api project(':fahrschein-http-api')
-    api "org.apache.httpcomponents:httpclient:${property('apachehttp.version')}"
+    api "org.apache.httpcomponents.client5:httpclient5:${property('apachehttp.version')}"
     testImplementation(testFixtures(project(':fahrschein-http-test-support')))
 }
 

diff --git a/...n-http-apache/src/main/java/org/zalando/fahrschein/http/apache/HttpComponentsRequest.java b/...n-http-apache/src/main/java/org/zalando/fahrschein/http/apache/HttpComponentsRequest.java
@@ -1,13 +1,12 @@
 package org.zalando.fahrschein.http.apache;
 
-import org.apache.http.HttpEntity;
-import org.apache.http.HttpEntityEnclosingRequest;
-import org.apache.http.HttpHeaders;
-import org.apache.http.HttpResponse;
-import org.apache.http.client.HttpClient;
-import org.apache.http.client.methods.HttpUriRequest;
-import org.apache.http.entity.ByteArrayEntity;
-import org.apache.http.protocol.HTTP;
+import org.apache.hc.client5.http.classic.HttpClient;
+
+import org.apache.hc.core5.http.ClassicHttpRequest;
+import org.apache.hc.core5.http.ClassicHttpResponse;
+import org.apache.hc.core5.http.ContentType;
+import org.apache.hc.core5.http.HttpEntity;
+import org.apache.hc.core5.http.io.entity.ByteArrayEntity;
 import org.zalando.fahrschein.http.api.ContentEncoding;
 import org.zalando.fahrschein.http.api.Headers;
 import org.zalando.fahrschein.http.api.HeadersImpl;
@@ -34,14 +33,14 @@
 final class HttpComponentsRequest implements Request {
 
     private final HttpClient httpClient;
-    private final HttpUriRequest httpRequest;
+    private final ClassicHttpRequest httpRequest;
     private final ContentEncoding contentEncoding;
 
     private final Headers headers;
     private ByteArrayOutputStream bufferedOutput;
     private boolean executed;
 
-    HttpComponentsRequest(HttpClient client, HttpUriRequest request, ContentEncoding contentEncoding) {
+    HttpComponentsRequest(HttpClient client, ClassicHttpRequest request, ContentEncoding contentEncoding) {
         this.httpClient = client;
         this.httpRequest = request;
         this.contentEncoding = contentEncoding;
@@ -55,7 +54,7 @@ public String getMethod() {
 
     @Override
     public URI getURI() {
-        return this.httpRequest.getURI();
+        return URI.create(this.httpRequest.getRequestUri());
     }
 
     private Response executeInternal(Headers headers) throws IOException {
@@ -67,20 +66,17 @@ private Response executeInternal(Headers headers) throws IOException {
 
         for (String headerName : headers.headerNames()) {
             final List<String> value = headers.get(headerName);
-            if (!HTTP.CONTENT_LEN.equalsIgnoreCase(headerName) && !HTTP.TRANSFER_ENCODING.equalsIgnoreCase(headerName)) {
+            if (! Headers.CONTENT_LENGTH.equalsIgnoreCase(headerName) && !Headers.TRANSFER_ENCODING.equalsIgnoreCase(headerName)) {
                 for (String headerValue : value) {
                     this.httpRequest.addHeader(headerName, headerValue);
                 }
             }
         }
 
-        if (this.httpRequest instanceof HttpEntityEnclosingRequest) {
-            HttpEntityEnclosingRequest entityEnclosingRequest = (HttpEntityEnclosingRequest) this.httpRequest;
-            HttpEntity requestEntity = new ByteArrayEntity(bytes);
-            entityEnclosingRequest.setEntity(requestEntity);
-        }
+        HttpEntity requestEntity = new ByteArrayEntity(bytes, ContentType.APPLICATION_JSON);
+        this.httpRequest.setEntity(requestEntity);
 
-        final HttpResponse httpResponse = this.httpClient.execute(this.httpRequest);
+        final ClassicHttpResponse httpResponse = (ClassicHttpResponse) this.httpClient.execute(this.httpRequest);
         final Response result = new HttpComponentsResponse(httpResponse);
         this.bufferedOutput = null;
 
@@ -101,7 +97,7 @@ public final OutputStream getBody() throws IOException {
                 // probably premature optimization, but we're omitting the unnecessary
                 // "Content-Encoding: identity" header
                 if (ContentEncoding.IDENTITY != this.contentEncoding) {
-                    this.httpRequest.setHeader(HttpHeaders.CONTENT_ENCODING, this.contentEncoding.value());
+                    this.httpRequest.setHeader(Headers.CONTENT_ENCODING, this.contentEncoding.value());
                 }
                 return this.contentEncoding.wrap(this.bufferedOutput);
             }

diff --git a/...apache/src/main/java/org/zalando/fahrschein/http/apache/HttpComponentsRequestFactory.java b/...apache/src/main/java/org/zalando/fahrschein/http/apache/HttpComponentsRequestFactory.java
@@ -1,15 +1,16 @@
 package org.zalando.fahrschein.http.apache;
 
-import org.apache.http.client.HttpClient;
-import org.apache.http.client.methods.HttpEntityEnclosingRequestBase;
-import org.apache.http.client.methods.HttpGet;
-import org.apache.http.client.methods.HttpHead;
-import org.apache.http.client.methods.HttpOptions;
-import org.apache.http.client.methods.HttpPatch;
-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.hc.client5.http.classic.HttpClient;
+import org.apache.hc.client5.http.classic.methods.HttpGet;
+import org.apache.hc.client5.http.classic.methods.HttpHead;
+import org.apache.hc.client5.http.classic.methods.HttpOptions;
+import org.apache.hc.client5.http.classic.methods.HttpPatch;
+import org.apache.hc.client5.http.classic.methods.HttpPost;
+import org.apache.hc.client5.http.classic.methods.HttpPut;
+import org.apache.hc.client5.http.classic.methods.HttpDelete;
+import org.apache.hc.client5.http.classic.methods.HttpTrace;
+import org.apache.hc.client5.http.classic.methods.HttpUriRequest;
 import org.zalando.fahrschein.http.api.ContentEncoding;
 import org.zalando.fahrschein.http.api.Request;
 import org.zalando.fahrschein.http.api.RequestFactory;
@@ -88,25 +89,4 @@ private static HttpUriRequest createHttpUriRequest(String method, URI uri) {
         }
     }
 
-    /**
-     * An alternative to {@link org.apache.http.client.methods.HttpDelete} that
-     * extends {@link org.apache.http.client.methods.HttpEntityEnclosingRequestBase}
-     * rather than {@link org.apache.http.client.methods.HttpRequestBase} and
-     * hence allows HTTP delete with a request body. For use with the RestTemplate
-     * exchange methods which allow the combination of HTTP DELETE with entity.
-     * @since 4.1.2
-     */
-    static class HttpDelete extends HttpEntityEnclosingRequestBase {
-
-        HttpDelete(URI uri) {
-            super();
-            setURI(uri);
-        }
-
-        @Override
-        public String getMethod() {
-            return "DELETE";
-        }
-    }
-
 }
diff --git a/...-http-apache/src/main/java/org/zalando/fahrschein/http/apache/HttpComponentsResponse.java b/...-http-apache/src/main/java/org/zalando/fahrschein/http/apache/HttpComponentsResponse.java
@@ -1,8 +1,8 @@
 package org.zalando.fahrschein.http.apache;
 
-import org.apache.http.Header;
-import org.apache.http.HttpEntity;
-import org.apache.http.HttpResponse;
+import org.apache.hc.core5.http.ClassicHttpResponse;
+import org.apache.hc.core5.http.Header;
+import org.apache.hc.core5.http.HttpEntity;
 import org.zalando.fahrschein.http.api.Headers;
 import org.zalando.fahrschein.http.api.HeadersImpl;
 import org.zalando.fahrschein.http.api.Response;
@@ -25,29 +25,29 @@
  */
 final class HttpComponentsResponse implements Response {
 
-    private final HttpResponse httpResponse;
+    private final ClassicHttpResponse httpResponse;
     private Headers headers;
     private InputStream responseStream;
 
-    HttpComponentsResponse(HttpResponse httpResponse) {
+    HttpComponentsResponse(ClassicHttpResponse httpResponse) {
         this.httpResponse = httpResponse;
     }
 
     @Override
     public int getStatusCode() throws IOException {
-        return this.httpResponse.getStatusLine().getStatusCode();
+        return this.httpResponse.getCode();
     }
 
     @Override
     public String getStatusText() throws IOException {
-        return this.httpResponse.getStatusLine().getReasonPhrase();
+        return this.httpResponse.getReasonPhrase();
     }
 
     @Override
     public Headers getHeaders() {
         if (this.headers == null) {
             this.headers = new HeadersImpl();
-            for (Header header : this.httpResponse.getAllHeaders()) {
+            for (Header header : this.httpResponse.getHeaders()) {
                 this.headers.add(header.getName(), header.getValue());
             }
         }

diff --git a/...he/src/test/java/org/zalando/fahrschein/http/apache/HttpComponentsRequestFactoryTest.java b/...he/src/test/java/org/zalando/fahrschein/http/apache/HttpComponentsRequestFactoryTest.java
@@ -1,8 +1,8 @@
 package org.zalando.fahrschein.http.apache;
 
-import org.apache.http.client.config.RequestConfig;
-import org.apache.http.impl.client.CloseableHttpClient;
-import org.apache.http.impl.client.HttpClients;
+import org.apache.hc.client5.http.config.RequestConfig;
+import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
+import org.apache.hc.client5.http.impl.classic.HttpClients;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.junit.jupiter.MockitoExtension;
@@ -16,6 +16,7 @@
 
 import java.io.IOException;
 import java.net.SocketTimeoutException;
+import java.util.concurrent.TimeUnit;
 
 @ExtendWith(MockitoExtension.class)
 public class HttpComponentsRequestFactoryTest extends AbstractRequestFactoryTest {
@@ -33,7 +34,7 @@ public void testTimeout() throws IOException {
         });
 
         // when
-        RequestConfig requestConfig = RequestConfig.custom().setSocketTimeout(1).build();
+        RequestConfig requestConfig = RequestConfig.custom().setResponseTimeout(1, TimeUnit.MILLISECONDS).build();
         final CloseableHttpClient httpClient = HttpClients.custom().setDefaultRequestConfig(requestConfig).build();
         RequestFactory f = new HttpComponentsRequestFactory(httpClient, ContentEncoding.GZIP);
         Request r = f.createRequest(serverAddress.resolve("/timeout"), "GET");

diff --git a/fahrschein-http-api/src/main/java/org/zalando/fahrschein/http/api/Headers.java b/fahrschein-http-api/src/main/java/org/zalando/fahrschein/http/api/Headers.java
@@ -8,6 +8,9 @@ public interface Headers {
 
     String AUTHORIZATION = "Authorization";
     String CONTENT_LENGTH = "Content-Length";
+
+    String TRANSFER_ENCODING = "Transfer-Encoding";
+
     String CONTENT_TYPE = "Content-Type";
     String CONTENT_ENCODING = "Content-Encoding";
     String ACCEPT_ENCODING = "Accept-Encoding";

diff --git a/gradle.properties b/gradle.properties
@@ -11,7 +11,7 @@ mockito.version=5.6.0
 junit.version=5.10.0
 testcontainers.version=1.19.1
 okhttp.version=4.10.0
-apachehttp.version=4.5.14
+apachehttp.version=5.2.1
 opentelemetry.version=1.31.0
 opentracing.version=0.33.0