From ba78fcbb6498463fb2f0fd217ce589b123cff4e4 Mon Sep 17 00:00:00 2001 From: Julen Garcia Leunda Date: Fri, 17 Jan 2025 22:07:02 +0100 Subject: [PATCH 1/4] Enable Http2Client tests --- .../src/test/java/feign/http2client/test/Http2ClientTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/java11/src/test/java/feign/http2client/test/Http2ClientTest.java b/java11/src/test/java/feign/http2client/test/Http2ClientTest.java index 4887c1791..2b84ed88c 100644 --- a/java11/src/test/java/feign/http2client/test/Http2ClientTest.java +++ b/java11/src/test/java/feign/http2client/test/Http2ClientTest.java @@ -32,11 +32,9 @@ import java.net.http.HttpTimeoutException; import java.util.concurrent.TimeUnit; import okhttp3.mockwebserver.MockResponse; -import org.junit.jupiter.api.Disabled; 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 { From 72a0912ce7c4cd35d2517d40f132984cbb531232 Mon Sep 17 00:00:00 2001 From: Julen Garcia Leunda Date: Fri, 17 Jan 2025 23:29:52 +0100 Subject: [PATCH 2/4] Fix Http2Client timeoutTest --- .../test/java/feign/http2client/test/Http2ClientTest.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/java11/src/test/java/feign/http2client/test/Http2ClientTest.java b/java11/src/test/java/feign/http2client/test/Http2ClientTest.java index 2b84ed88c..af33e15a2 100644 --- a/java11/src/test/java/feign/http2client/test/Http2ClientTest.java +++ b/java11/src/test/java/feign/http2client/test/Http2ClientTest.java @@ -115,12 +115,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()); From c02f368463708ccf1061f7fef0cda6ea0c4c4bde Mon Sep 17 00:00:00 2001 From: Julen Garcia Leunda Date: Mon, 20 Jan 2025 14:56:44 +0100 Subject: [PATCH 3/4] Override some AbstractClientTests as Http2Client does not expose reason phrase --- .../java/feign/client/AbstractClientTest.java | 12 +- .../http2client/test/Http2ClientTest.java | 177 +++++++++++++++++- 2 files changed, 175 insertions(+), 14 deletions(-) diff --git a/core/src/test/java/feign/client/AbstractClientTest.java b/core/src/test/java/feign/client/AbstractClientTest.java index 7270bb16c..bb01525c9 100644 --- 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/test/java/feign/http2client/test/Http2ClientTest.java b/java11/src/test/java/feign/http2client/test/Http2ClientTest.java index af33e15a2..806b9321a 100644 --- a/java11/src/test/java/feign/http2client/test/Http2ClientTest.java +++ b/java11/src/test/java/feign/http2client/test/Http2ClientTest.java @@ -15,23 +15,24 @@ */ 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 okhttp3.mockwebserver.RecordedRequest; import org.junit.jupiter.api.Test; /** Tests client-specific behavior, such as ensuring Content-Length is sent when specified. */ @@ -57,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 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 multiFoo); } @Override @@ -144,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()); From efe846849b0893fa08b2c622050bc0a33c886697 Mon Sep 17 00:00:00 2001 From: Julen Garcia Leunda Date: Tue, 21 Jan 2025 09:41:01 +0100 Subject: [PATCH 4/4] Add gzip and deflate compression support in Http2Client --- .../java/feign/http2client/Http2Client.java | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/java11/src/main/java/feign/http2client/Http2Client.java b/java11/src/main/java/feign/http2client/Http2Client.java index d20d03bb6..40d4548a2 100644 --- 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 { @@ -129,9 +131,20 @@ public CompletableFuture execute( protected Response toFeignResponse(Request request, HttpResponse 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())