Add gzip and deflate compression support in Http2Client (#2738)

hicom150 · web-flow · commit 71604b5e4dcc · 2025-01-27T13:31:09.000-03:00
* Enable Http2Client tests

* Fix Http2Client timeoutTest

* Override some AbstractClientTests as Http2Client does not expose reason phrase

* Add gzip and deflate compression support in Http2Client
diff --git a/core/src/test/java/feign/client/AbstractClientTest.java b/core/src/test/java/feign/client/AbstractClientTest.java
@@ -127,7 +127,7 @@ public void reasonPhraseIsOptional() throws IOException, InterruptedException {
   }
 
   @Test
-  void parsesErrorResponse() {
+  public void parsesErrorResponse() {
 
     server.enqueue(new MockResponse().setResponseCode(500).setBody("ARGHH"));
 
@@ -244,7 +244,7 @@ public void noResponseBodyForPatch() {
   }
 
   @Test
-  void parsesResponseMissingLength() throws IOException {
+  public void parsesResponseMissingLength() throws IOException {
     server.enqueue(new MockResponse().setChunkedBody("foo", 1));
 
     TestInterface api =
@@ -332,7 +332,7 @@ public void contentTypeDefaultsToRequestCharset() throws Exception {
   }
 
   @Test
-  void defaultCollectionFormat() throws Exception {
+  public void defaultCollectionFormat() throws Exception {
     server.enqueue(new MockResponse().setBody("body"));
 
     TestInterface api =
@@ -349,7 +349,7 @@ void defaultCollectionFormat() throws Exception {
   }
 
   @Test
-  void headersWithNullParams() throws InterruptedException {
+  public void headersWithNullParams() throws InterruptedException {
     server.enqueue(new MockResponse().setBody("body"));
 
     TestInterface api =
@@ -367,7 +367,7 @@ void headersWithNullParams() throws InterruptedException {
   }
 
   @Test
-  void headersWithNotEmptyParams() throws InterruptedException {
+  public void headersWithNotEmptyParams() throws InterruptedException {
     server.enqueue(new MockResponse().setBody("body"));
 
     TestInterface api =
@@ -385,7 +385,7 @@ void headersWithNotEmptyParams() throws InterruptedException {
   }
 
   @Test
-  void alternativeCollectionFormat() throws Exception {
+  public void alternativeCollectionFormat() throws Exception {
     server.enqueue(new MockResponse().setBody("body"));
 
     TestInterface api =
diff --git a/java11/src/main/java/feign/http2client/Http2Client.java b/java11/src/main/java/feign/http2client/Http2Client.java
@@ -15,7 +15,7 @@
  */
 package feign.http2client;
 
-import static feign.Util.enumForName;
+import static feign.Util.*;
 
 import feign.AsyncClient;
 import feign.Client;
@@ -54,6 +54,8 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Function;
 import java.util.stream.Collectors;
+import java.util.zip.GZIPInputStream;
+import java.util.zip.InflaterInputStream;
 
 public class Http2Client implements Client, AsyncClient<Object> {
 
@@ -129,9 +131,20 @@ public CompletableFuture<Response> execute(
   protected Response toFeignResponse(Request request, HttpResponse<InputStream> httpResponse) {
     final OptionalLong length = httpResponse.headers().firstValueAsLong("Content-Length");
 
+    InputStream body = httpResponse.body();
+
+    if (httpResponse.headers().allValues(CONTENT_ENCODING).contains(ENCODING_GZIP)) {
+      try {
+        body = new GZIPInputStream(body);
+      } catch (IOException ignored) {
+      }
+    } else if (httpResponse.headers().allValues(CONTENT_ENCODING).contains(ENCODING_DEFLATE)) {
+      body = new InflaterInputStream(body);
+    }
+
     return Response.builder()
         .protocolVersion(enumForName(ProtocolVersion.class, httpResponse.version()))
-        .body(httpResponse.body(), length.isPresent() ? (int) length.getAsLong() : null)
+        .body(body, length.isPresent() ? (int) length.getAsLong() : null)
         .reason(httpResponse.headers().firstValue("Reason-Phrase").orElse(null))
         .request(request)
         .status(httpResponse.statusCode())
diff --git a/java11/src/test/java/feign/http2client/test/Http2ClientTest.java b/java11/src/test/java/feign/http2client/test/Http2ClientTest.java
@@ -15,28 +15,27 @@
  */
 package feign.http2client.test;
 
+import static feign.Util.UTF_8;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.entry;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
-import feign.Body;
-import feign.Feign;
-import feign.FeignException;
-import feign.Headers;
-import feign.Request;
-import feign.RequestLine;
-import feign.Response;
-import feign.Retryer;
+import feign.*;
+import feign.assertj.MockWebServerAssertions;
 import feign.client.AbstractClientTest;
 import feign.http2client.Http2Client;
+import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.net.http.HttpTimeoutException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
 import java.util.concurrent.TimeUnit;
 import okhttp3.mockwebserver.MockResponse;
-import org.junit.jupiter.api.Disabled;
+import okhttp3.mockwebserver.RecordedRequest;
 import org.junit.jupiter.api.Test;
 
 /** Tests client-specific behavior, such as ensuring Content-Length is sent when specified. */
-@Disabled
 public class Http2ClientTest extends AbstractClientTest {
 
   public interface TestInterface {
@@ -59,6 +58,24 @@ public interface TestInterface {
     @RequestLine("DELETE /anything")
     @Body("some request body")
     String deleteWithBody();
+
+    @RequestLine("POST /?foo=bar&foo=baz&qux=")
+    @Headers({"Foo: Bar", "Foo: Baz", "Qux: ", "Content-Type: text/plain"})
+    Response post(String body);
+
+    @RequestLine("GET /")
+    @Headers("Accept: text/plain")
+    String get();
+
+    @RequestLine("GET /?foo={multiFoo}")
+    Response get(@Param("multiFoo") List<String> multiFoo);
+
+    @Headers({"Authorization: {authorization}"})
+    @RequestLine("GET /")
+    Response getWithHeaders(@Param("authorization") String authorization);
+
+    @RequestLine(value = "GET /?foo={multiFoo}", collectionFormat = CollectionFormat.CSV)
+    Response getCSV(@Param("multiFoo") List<String> multiFoo);
   }
 
   @Override
@@ -117,12 +134,13 @@ public void veryLongResponseNullLength() {
 
   @Test
   void timeoutTest() {
-    server.enqueue(new MockResponse().setBody("foo").setBodyDelay(30, TimeUnit.SECONDS));
+    server.enqueue(new MockResponse().setBody("foo").setHeadersDelay(1, TimeUnit.SECONDS));
 
     final TestInterface api =
         newBuilder()
             .retryer(Retryer.NEVER_RETRY)
-            .options(new Request.Options(1, TimeUnit.SECONDS, 1, TimeUnit.SECONDS, true))
+            .options(
+                new Request.Options(500, TimeUnit.MILLISECONDS, 500, TimeUnit.MILLISECONDS, true))
             .target(TestInterface.class, server.url("/").toString());
 
     FeignException exception = assertThrows(FeignException.class, () -> api.timeout());
@@ -145,6 +163,148 @@ void deleteWithRequestBody() {
     assertThat(result).contains("\"data\": \"some request body\"");
   }
 
+  @Override
+  @Test
+  public void parsesResponseMissingLength() throws IOException {
+    server.enqueue(new MockResponse().setChunkedBody("foo", 1));
+
+    TestInterface api =
+        newBuilder().target(TestInterface.class, "http://localhost:" + server.getPort());
+
+    Response response = api.post("testing");
+    assertThat(response.status()).isEqualTo(200);
+    // assertThat(response.reason()).isEqualTo("OK");
+    assertThat(response.body().length()).isNull();
+    assertThat(response.body().asInputStream())
+        .hasSameContentAs(new ByteArrayInputStream("foo".getBytes(UTF_8)));
+  }
+
+  @Override
+  @Test
+  public void parsesErrorResponse() {
+
+    server.enqueue(new MockResponse().setResponseCode(500).setBody("ARGHH"));
+
+    TestInterface api =
+        newBuilder().target(TestInterface.class, "http://localhost:" + server.getPort());
+
+    Throwable exception = assertThrows(FeignException.class, () -> api.get());
+    assertThat(exception.getMessage())
+        .contains(
+            "[500] during [GET] to [http://localhost:"
+                + server.getPort()
+                + "/] [TestInterface#get()]: [ARGHH]");
+  }
+
+  @Override
+  @Test
+  public void defaultCollectionFormat() throws Exception {
+    server.enqueue(new MockResponse().setBody("body"));
+
+    TestInterface api =
+        newBuilder().target(TestInterface.class, "http://localhost:" + server.getPort());
+
+    Response response = api.get(Arrays.asList("bar", "baz"));
+
+    assertThat(response.status()).isEqualTo(200);
+    // assertThat(response.reason()).isEqualTo("OK");
+
+    MockWebServerAssertions.assertThat(server.takeRequest())
+        .hasMethod("GET")
+        .hasPath("/?foo=bar&foo=baz");
+  }
+
+  @Override
+  @Test
+  public void headersWithNotEmptyParams() throws InterruptedException {
+    server.enqueue(new MockResponse().setBody("body"));
+
+    TestInterface api =
+        newBuilder().target(TestInterface.class, "http://localhost:" + server.getPort());
+
+    Response response = api.getWithHeaders("token");
+
+    assertThat(response.status()).isEqualTo(200);
+    // assertThat(response.reason()).isEqualTo("OK");
+
+    MockWebServerAssertions.assertThat(server.takeRequest())
+        .hasMethod("GET")
+        .hasPath("/")
+        .hasHeaders(entry("authorization", Collections.singletonList("token")));
+  }
+
+  @Override
+  @Test
+  public void headersWithNullParams() throws InterruptedException {
+    server.enqueue(new MockResponse().setBody("body"));
+
+    TestInterface api =
+        newBuilder().target(TestInterface.class, "http://localhost:" + server.getPort());
+
+    Response response = api.getWithHeaders(null);
+
+    assertThat(response.status()).isEqualTo(200);
+    // assertThat(response.reason()).isEqualTo("OK");
+
+    MockWebServerAssertions.assertThat(server.takeRequest())
+        .hasMethod("GET")
+        .hasPath("/")
+        .hasNoHeaderNamed("Authorization");
+  }
+
+  @Test
+  public void alternativeCollectionFormat() throws Exception {
+    server.enqueue(new MockResponse().setBody("body"));
+
+    TestInterface api =
+        newBuilder().target(TestInterface.class, "http://localhost:" + server.getPort());
+
+    Response response = api.getCSV(Arrays.asList("bar", "baz"));
+
+    assertThat(response.status()).isEqualTo(200);
+    // assertThat(response.reason()).isEqualTo("OK");
+
+    // Some HTTP libraries percent-encode commas in query parameters and others
+    // don't.
+    MockWebServerAssertions.assertThat(server.takeRequest())
+        .hasMethod("GET")
+        .hasOneOfPath("/?foo=bar,baz", "/?foo=bar%2Cbaz");
+  }
+
+  @Override
+  @Test
+  public void parsesRequestAndResponse() throws IOException, InterruptedException {
+    server.enqueue(new MockResponse().setBody("foo").addHeader("Foo: Bar"));
+
+    TestInterface api =
+        newBuilder().target(TestInterface.class, "http://localhost:" + server.getPort());
+
+    Response response = api.post("foo");
+
+    assertThat(response.status()).isEqualTo(200);
+    // assertThat(response.reason()).isEqualTo("OK");
+    assertThat(response.headers())
+        .hasEntrySatisfying(
+            "Content-Length",
+            value -> {
+              assertThat(value).contains("3");
+            })
+        .hasEntrySatisfying(
+            "Foo",
+            value -> {
+              assertThat(value).contains("Bar");
+            });
+    assertThat(response.body().asInputStream())
+        .hasSameContentAs(new ByteArrayInputStream("foo".getBytes(UTF_8)));
+
+    RecordedRequest recordedRequest = server.takeRequest();
+    assertThat(recordedRequest.getMethod()).isEqualToIgnoringCase("POST");
+    assertThat(recordedRequest.getHeader("Foo")).isEqualToIgnoringCase("Bar, Baz");
+    assertThat(recordedRequest.getHeader("Accept")).isEqualToIgnoringCase("*/*");
+    assertThat(recordedRequest.getHeader("Content-Length")).isEqualToIgnoringCase("3");
+    assertThat(recordedRequest.getBody().readUtf8()).isEqualToIgnoringCase("foo");
+  }
+
   @Override
   public Feign.Builder newBuilder() {
     return Feign.builder().client(new Http2Client());
