Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add gzip and deflate compression support in Http2Client #2738

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions core/src/test/java/feign/client/AbstractClientTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public void reasonPhraseIsOptional() throws IOException, InterruptedException {
}

@Test
void parsesErrorResponse() {
public void parsesErrorResponse() {

server.enqueue(new MockResponse().setResponseCode(500).setBody("ARGHH"));

Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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 =
Expand All @@ -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 =
Expand All @@ -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 =
Expand All @@ -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 =
Expand Down
17 changes: 15 additions & 2 deletions java11/src/main/java/feign/http2client/Http2Client.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/
package feign.http2client;

import static feign.Util.enumForName;
import static feign.Util.*;

import feign.AsyncClient;
import feign.Client;
Expand Down Expand Up @@ -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> {

Expand Down Expand Up @@ -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())
Expand Down
184 changes: 172 additions & 12 deletions java11/src/test/java/feign/http2client/test/Http2ClientTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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());
Expand All @@ -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());
Expand Down