Skip to content

Commit cca0cad

Browse files
velovelo
authored
Expanded test for HTTP/2 client (OpenFeign#812)
1 parent 6acbe94 commit cca0cad

2 files changed

Lines changed: 95 additions & 54 deletions

File tree

‎java11/src/main/java/feign/httpclient/Http2Client.java‎

Lines changed: 60 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -19,27 +19,59 @@
1919
import java.net.URISyntaxException;
2020
import java.net.http.HttpClient;
2121
import java.net.http.HttpClient.Redirect;
22+
import java.net.http.HttpClient.Version;
2223
import java.net.http.HttpRequest;
2324
import java.net.http.HttpRequest.BodyPublisher;
2425
import java.net.http.HttpRequest.BodyPublishers;
26+
import java.net.http.HttpRequest.Builder;
2527
import java.net.http.HttpResponse;
2628
import java.net.http.HttpResponse.BodyHandlers;
2729
import java.util.*;
2830
import java.util.function.Function;
2931
import java.util.stream.Collectors;
30-
import feign.Client;
31-
import feign.Request;
32+
import feign.*;
3233
import feign.Request.Options;
33-
import feign.Response;
3434

3535
public class Http2Client implements Client {
3636

37+
private final HttpClient client;
38+
39+
public Http2Client() {
40+
this(HttpClient.newBuilder()
41+
.followRedirects(Redirect.ALWAYS)
42+
.version(Version.HTTP_2)
43+
.build());
44+
}
45+
46+
public Http2Client(HttpClient client) {
47+
this.client = Util.checkNotNull(client, "http cliet must be not unll");
48+
}
49+
3750
@Override
3851
public Response execute(Request request, Options options) throws IOException {
39-
final HttpClient client = HttpClient.newBuilder()
40-
.followRedirects(Redirect.ALWAYS)
52+
final HttpRequest httpRequest = newRequestBuilder(request).build();
53+
54+
HttpResponse<byte[]> httpResponse;
55+
try {
56+
httpResponse = client.send(httpRequest, BodyHandlers.ofByteArray());
57+
} catch (final InterruptedException e) {
58+
throw new IOException("Invalid uri " + request.url(), e);
59+
}
60+
61+
final OptionalLong length = httpResponse.headers().firstValueAsLong("Content-Length");
62+
63+
final Response response = Response.builder()
64+
.body(new ByteArrayInputStream(httpResponse.body()),
65+
length.isPresent() ? (int) length.getAsLong() : null)
66+
.reason(httpResponse.headers().firstValue("Reason-Phrase").orElse("OK"))
67+
.request(request)
68+
.status(httpResponse.statusCode())
69+
.headers(castMapCollectType(httpResponse.headers().map()))
4170
.build();
71+
return response;
72+
}
4273

74+
private Builder newRequestBuilder(Request request) throws IOException {
4375
URI uri;
4476
try {
4577
uri = new URI(request.url());
@@ -54,32 +86,29 @@ public Response execute(Request request, Options options) throws IOException {
5486
body = BodyPublishers.ofByteArray(request.body());
5587
}
5688

57-
final HttpRequest httpRequest = HttpRequest.newBuilder()
89+
final Builder requestBuilder = HttpRequest.newBuilder()
5890
.uri(uri)
59-
.method(request.method(), body)
60-
.headers(asString(filterRestrictedHeaders(request.headers())))
61-
.build();
91+
.version(Version.HTTP_2);
6292

63-
HttpResponse<byte[]> httpResponse;
64-
try {
65-
httpResponse = client.send(httpRequest, BodyHandlers.ofByteArray());
66-
} catch (final InterruptedException e) {
67-
throw new IOException("Invalid uri " + request.url(), e);
93+
final Map<String, Collection<String>> headers = filterRestrictedHeaders(request.headers());
94+
if (!headers.isEmpty()) {
95+
requestBuilder.headers(asString(headers));
6896
}
6997

70-
System.out.println(httpResponse.headers().map());
71-
72-
final OptionalLong length = httpResponse.headers().firstValueAsLong("Content-Length");
98+
switch (request.httpMethod()) {
99+
case GET:
100+
return requestBuilder.GET();
101+
case POST:
102+
return requestBuilder.POST(body);
103+
case PUT:
104+
return requestBuilder.PUT(body);
105+
case DELETE:
106+
return requestBuilder.DELETE();
107+
default:
108+
// fall back scenario, http implementations may restrict some methods
109+
return requestBuilder.method(request.httpMethod().toString(), body);
110+
}
73111

74-
final Response response = Response.builder()
75-
.body(new ByteArrayInputStream(httpResponse.body()),
76-
length.isPresent() ? (int) length.getAsLong() : null)
77-
.reason(httpResponse.headers().firstValue("Reason-Phrase").orElse(null))
78-
.request(request)
79-
.status(httpResponse.statusCode())
80-
.headers(castMapCollectType(httpResponse.headers().map()))
81-
.build();
82-
return response;
83112
}
84113

85114
/**
@@ -98,12 +127,16 @@ public Response execute(Request request, Options options) throws IOException {
98127
}
99128

100129
private Map<String, Collection<String>> filterRestrictedHeaders(Map<String, Collection<String>> headers) {
101-
return headers.keySet()
130+
final Map<String, Collection<String>> filteredHeaders = headers.keySet()
102131
.stream()
103132
.filter(headerName -> !DISALLOWED_HEADERS_SET.contains(headerName))
104133
.collect(Collectors.toMap(
105134
Function.identity(),
106135
headers::get));
136+
137+
filteredHeaders.computeIfAbsent("Accept", key -> List.of("*/*"));
138+
139+
return filteredHeaders;
107140
}
108141

109142
private Map<String, Collection<String>> castMapCollectType(Map<String, List<String>> map) {

‎java11/src/test/java/feign/httpclient/test/Http2ClientTest.java‎

Lines changed: 35 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -13,52 +13,60 @@
1313
*/
1414
package feign.httpclient.test;
1515

16-
import feign.*;
17-
import feign.httpclient.Http2Client;
16+
import static org.assertj.core.api.Assertions.assertThat;
1817
import org.assertj.core.api.Assertions;
19-
import org.junit.Assert;
2018
import org.junit.Test;
19+
import java.io.IOException;
20+
import feign.*;
21+
import feign.client.AbstractClientTest;
22+
import feign.httpclient.Http2Client;
23+
import okhttp3.mockwebserver.MockResponse;
2124

2225
/**
2326
* Tests client-specific behavior, such as ensuring Content-Length is sent when specified.
2427
*/
25-
public class Http2ClientTest {
28+
public class Http2ClientTest extends AbstractClientTest {
2629

2730
public interface TestInterface {
28-
@RequestLine("POST /?foo=bar&foo=baz&qux=")
29-
@Headers({"Foo: Bar", "Foo: Baz", "Qux: ", "Content-Type: text/plain"})
30-
Response post(String var1);
31-
32-
@RequestLine("POST /path/{to}/resource")
33-
@Headers({"Accept: text/plain"})
34-
Response post(@Param("to") String var1, String var2);
35-
36-
@RequestLine("GET /")
37-
@Headers({"Accept: text/plain"})
38-
String get();
39-
4031
@RequestLine("PATCH /patch")
4132
@Headers({"Accept: text/plain"})
4233
String patch(String var1);
43-
44-
@RequestLine("POST")
45-
String noPostBody();
46-
47-
@RequestLine("PUT")
48-
String noPutBody();
49-
50-
@RequestLine("POST /?foo=bar&foo=baz&qux=")
51-
@Headers({"Foo: Bar", "Foo: Baz", "Qux: ", "Content-Type: {contentType}"})
52-
Response postWithContentType(String var1, @Param("contentType") String var2);
5334
}
5435

36+
@Override
5537
@Test
5638
public void testPatch() throws Exception {
57-
TestInterface api = newBuilder().target(TestInterface.class, "https://nghttp2.org/httpbin/");
39+
final TestInterface api =
40+
newBuilder().target(TestInterface.class, "https://nghttp2.org/httpbin/");
5841
Assertions.assertThat(api.patch(""))
5942
.contains("https://nghttp2.org/httpbin/patch");
6043
}
6144

45+
46+
@Override
47+
@Test
48+
public void reasonPhraseIsOptional() throws IOException, InterruptedException {
49+
server.enqueue(new MockResponse()
50+
.addHeader("Reason-Phrase", "There is A reason")
51+
.setStatus("HTTP/1.1 " + 200));
52+
53+
final AbstractClientTest.TestInterface api = newBuilder()
54+
.target(AbstractClientTest.TestInterface.class, "http://localhost:" + server.getPort());
55+
56+
final Response response = api.post("foo");
57+
58+
assertThat(response.status()).isEqualTo(200);
59+
assertThat(response.reason()).isEqualTo("There is A reason");
60+
}
61+
62+
63+
@Override
64+
@Test
65+
public void testVeryLongResponseNullLength() {
66+
// client is too smart to fall for a body that is 8 bytes long
67+
}
68+
69+
@Override
6270
public Feign.Builder newBuilder() {
6371
return Feign.builder().client(new Http2Client());
6472
}

0 commit comments

Comments
 (0)