Skip to content

Commit 206193d

Browse files
vitalijr2vitalijr2
authored
HTTP protocol version on the logging (OpenFeign#1543)
* [HTTP version] Add HTTP version to Response, the default client; Update SLF4J unit-test * [HTTP version] Mock client * [HTTP version] Apache HTTP Client * [HTTP version] protocol -> protocolVersion; Replace protocol number with full name * [HTTP version] Code style, rollback to old one * [HTTP version] Google HTTP Client * [HTTP version] HTTP_PROTOCOL -> HTTP_PROTOCOL_VERSION * [HTTP version] HC5 * [HTTP version] Java11 Client * [HTTP version] OkHttpClient * [HTTP version] Code style, rollback to old one * [HTTP version] Make some required changes: restore log messages for back compatibility, replace string protocol version with enum, replace fragile conversion of alien enums by string case-insensitive comparision * [HTTP version] Code style, rollback to old one; Remove unused constants * [HTTP version] Update imports * [HTTP version] Test coverage * [HTTP version] Fix license issue * [HTTP version] Beatify and simplify the unit-test
1 parent 851749e commit 206193d

12 files changed

Lines changed: 302 additions & 10 deletions

File tree

‎core/src/main/java/feign/Logger.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
package feign;
1515

1616
import static feign.Util.*;
17+
import static java.util.Objects.nonNull;
1718

1819
import java.io.IOException;
1920
import java.io.PrintWriter;
@@ -60,7 +61,8 @@ protected boolean shouldLogResponseHeader(String header) {
6061
}
6162

6263
protected void logRequest(String configKey, Level logLevel, Request request) {
63-
log(configKey, "---> %s %s HTTP/1.1", request.httpMethod().name(), request.url());
64+
String protocolVersion = resolveProtocolVersion(request.protocolVersion());
65+
log(configKey, "---> %s %s %s", request.httpMethod().name(), request.url(), protocolVersion);
6466
if (logLevel.ordinal() >= Level.HEADERS.ordinal()) {
6567

6668
for (String field : request.headers().keySet()) {
@@ -91,12 +93,13 @@ protected void logRetry(String configKey, Level logLevel) {
9193

9294
protected Response logAndRebufferResponse(
9395
String configKey, Level logLevel, Response response, long elapsedTime) throws IOException {
96+
String protocolVersion = resolveProtocolVersion(response.protocolVersion());
9497
String reason =
9598
response.reason() != null && logLevel.compareTo(Level.NONE) > 0
9699
? " " + response.reason()
97100
: "";
98101
int status = response.status();
99-
log(configKey, "<--- HTTP/1.1 %s%s (%sms)", status, reason, elapsedTime);
102+
log(configKey, "<--- %s %s%s (%sms)", protocolVersion, status, reason, elapsedTime);
100103
if (logLevel.ordinal() >= Level.HEADERS.ordinal()) {
101104

102105
for (String field : response.headers().keySet()) {
@@ -145,6 +148,13 @@ protected IOException logIOException(
145148
return ioe;
146149
}
147150

151+
protected static String resolveProtocolVersion(Request.ProtocolVersion protocolVersion) {
152+
if (nonNull(protocolVersion)) {
153+
return protocolVersion.toString();
154+
}
155+
return "UNKNOWN";
156+
}
157+
148158
/** Controls the level of logging. */
149159
public enum Level {
150160
/** No logging. */

‎core/src/main/java/feign/Request.java‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2012-2020 The Feign Authors
2+
* Copyright 2012-2021 The Feign Authors
33
*
44
* <p>Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file
55
* except in compliance with the License. You may obtain a copy of the License at
@@ -40,6 +40,28 @@ public enum HttpMethod {
4040
PATCH
4141
}
4242

43+
public enum ProtocolVersion {
44+
HTTP_1_0("HTTP/1.0"),
45+
HTTP_1_1("HTTP/1.1"),
46+
HTTP_2("HTTP/2.0"),
47+
MOCK;
48+
49+
String protocolVersion;
50+
51+
ProtocolVersion() {
52+
protocolVersion = name();
53+
}
54+
55+
ProtocolVersion(String protocolVersion) {
56+
this.protocolVersion = protocolVersion;
57+
}
58+
59+
@Override
60+
public String toString() {
61+
return protocolVersion;
62+
}
63+
}
64+
4365
/**
4466
* No parameters can be null except {@code body} and {@code charset}. All parameters must be
4567
* effectively immutable, via safe copies, not mutating or otherwise.
@@ -121,6 +143,7 @@ public static Request create(
121143
private final Map<String, Collection<String>> headers;
122144
private final Body body;
123145
private final RequestTemplate requestTemplate;
146+
private final ProtocolVersion protocolVersion;
124147

125148
/**
126149
* Creates a new Request.
@@ -142,6 +165,7 @@ public static Request create(
142165
this.headers = checkNotNull(headers, "headers of %s %s", method, url);
143166
this.body = body;
144167
this.requestTemplate = requestTemplate;
168+
protocolVersion = ProtocolVersion.HTTP_1_1;
145169
}
146170

147171
/**
@@ -214,6 +238,15 @@ public int length() {
214238
return this.body.length();
215239
}
216240

241+
/**
242+
* Request HTTP protocol version
243+
*
244+
* @return HTTP protocol version
245+
*/
246+
public ProtocolVersion protocolVersion() {
247+
return protocolVersion;
248+
}
249+
217250
/**
218251
* Request as an HTTP/1.1 request.
219252
*

‎core/src/main/java/feign/Response.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
import static feign.Util.*;
1717

18+
import feign.Request.ProtocolVersion;
1819
import java.io.*;
1920
import java.nio.charset.Charset;
2021
import java.nio.charset.StandardCharsets;
@@ -28,6 +29,7 @@ public final class Response implements Closeable {
2829
private final Map<String, Collection<String>> headers;
2930
private final Body body;
3031
private final Request request;
32+
private final ProtocolVersion protocolVersion;
3133

3234
private Response(Builder builder) {
3335
checkState(builder.request != null, "original request is required");
@@ -36,6 +38,7 @@ private Response(Builder builder) {
3638
this.reason = builder.reason; // nullable
3739
this.headers = caseInsensitiveCopyOf(builder.headers);
3840
this.body = builder.body; // nullable
41+
this.protocolVersion = builder.protocolVersion;
3942
}
4043

4144
public Builder toBuilder() {
@@ -53,6 +56,7 @@ public static final class Builder {
5356
Body body;
5457
Request request;
5558
private RequestTemplate requestTemplate;
59+
private ProtocolVersion protocolVersion = ProtocolVersion.HTTP_1_1;
5660

5761
Builder() {}
5862

@@ -62,6 +66,7 @@ public static final class Builder {
6266
this.headers = source.headers;
6367
this.body = source.body;
6468
this.request = source.request;
69+
this.protocolVersion = source.protocolVersion;
6570
}
6671

6772
/**
@@ -129,6 +134,12 @@ public Builder request(Request request) {
129134
return this;
130135
}
131136

137+
/** HTTP protocol version */
138+
public Builder protocolVersion(ProtocolVersion protocolVersion) {
139+
this.protocolVersion = protocolVersion;
140+
return this;
141+
}
142+
132143
/**
133144
* The Request Template used for the original request.
134145
*
@@ -179,6 +190,15 @@ public Request request() {
179190
return request;
180191
}
181192

193+
/**
194+
* the HTTP protocol version
195+
*
196+
* @return HTTP protocol version or empty if a client does not provide it
197+
*/
198+
public ProtocolVersion protocolVersion() {
199+
return protocolVersion;
200+
}
201+
182202
public Charset charset() {
183203

184204
Collection<String> contentTypeHeaders = headers().get("Content-Type");

‎core/src/main/java/feign/Util.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
package feign;
1515

1616
import static java.lang.String.format;
17+
import static java.util.Objects.nonNull;
1718

1819
import java.io.ByteArrayOutputStream;
1920
import java.io.Closeable;
@@ -346,4 +347,14 @@ public static Map<String, Collection<String>> caseInsensitiveCopyOf(
346347

347348
return Collections.unmodifiableMap(result);
348349
}
350+
351+
public static <T extends Enum<?>> T enumForName(Class<T> enumClass, Object object) {
352+
String name = (nonNull(object)) ? object.toString() : null;
353+
for (T enumItem : enumClass.getEnumConstants()) {
354+
if (enumItem.name().equalsIgnoreCase(name) || enumItem.toString().equalsIgnoreCase(name)) {
355+
return enumItem;
356+
}
357+
}
358+
return null;
359+
}
349360
}

‎core/src/test/java/feign/EnumForNameTest.java‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/**
2+
* Copyright 2012-2021 The Feign Authors
3+
*
4+
* <p>Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file
5+
* except in compliance with the License. You may obtain a copy of the License at
6+
*
7+
* <p>http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* <p>Unless required by applicable law or agreed to in writing, software distributed under the
10+
* License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
11+
* express or implied. See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
*/
14+
package feign;
15+
16+
import static feign.Util.enumForName;
17+
import static org.junit.Assert.*;
18+
19+
import feign.Request.ProtocolVersion;
20+
import java.util.Arrays;
21+
import org.junit.Test;
22+
import org.junit.runner.RunWith;
23+
import org.junit.runners.Parameterized;
24+
import org.junit.runners.Parameterized.Parameter;
25+
import org.junit.runners.Parameterized.Parameters;
26+
27+
public class EnumForNameTest {
28+
29+
@RunWith(Parameterized.class)
30+
public static class KnownEnumValues {
31+
32+
@Parameter public Object name;
33+
34+
@Parameter(1)
35+
public ProtocolVersion expectedProtocolVersion;
36+
37+
@Parameters
38+
public static Iterable<Object[]> data() {
39+
return Arrays.asList(
40+
new Object[][] {
41+
{ProtocolVersion.HTTP_1_0, ProtocolVersion.HTTP_1_0},
42+
{"HTTP/1.0", ProtocolVersion.HTTP_1_0},
43+
{ProtocolVersion.HTTP_1_1, ProtocolVersion.HTTP_1_1},
44+
{"HTTP/1.1", ProtocolVersion.HTTP_1_1},
45+
{ProtocolVersion.HTTP_2, ProtocolVersion.HTTP_2},
46+
{"HTTP/2.0", ProtocolVersion.HTTP_2}
47+
});
48+
}
49+
50+
@Test
51+
public void getKnownEnumValue() {
52+
assertEquals(
53+
"known enum value: " + name,
54+
expectedProtocolVersion,
55+
enumForName(ProtocolVersion.class, name));
56+
}
57+
}
58+
59+
@RunWith(Parameterized.class)
60+
public static class UnknownEnumValues {
61+
62+
@Parameter public Object name;
63+
64+
@Parameters
65+
public static Iterable<Object[]> data() {
66+
return Arrays.asList(
67+
new Object[][] {{Request.HttpMethod.GET}, {"SPDY/3"}, {null}, {"HTTP/2"}});
68+
}
69+
70+
@Test
71+
public void getKnownEnumValue() {
72+
assertNull("unknown enum value: " + name, enumForName(ProtocolVersion.class, name));
73+
}
74+
}
75+
}

‎core/src/test/java/feign/LoggerTest.java‎

Lines changed: 91 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,14 @@
1313
*/
1414
package feign;
1515

16+
import static feign.Util.enumForName;
17+
import static java.util.Objects.nonNull;
18+
1619
import feign.Logger.Level;
17-
import java.util.ArrayList;
18-
import java.util.Arrays;
19-
import java.util.Collections;
20-
import java.util.List;
20+
import feign.Request.ProtocolVersion;
21+
import java.io.IOException;
22+
import java.net.HttpURLConnection;
23+
import java.util.*;
2124
import java.util.concurrent.TimeUnit;
2225
import okhttp3.mockwebserver.MockResponse;
2326
import okhttp3.mockwebserver.MockWebServer;
@@ -159,6 +162,69 @@ public void reasonPhraseOptional() {
159162
}
160163
}
161164

165+
@RunWith(Parameterized.class)
166+
public static class HttpProtocolVersionTest extends LoggerTest {
167+
168+
private final Level logLevel;
169+
private final String protocolVersionName;
170+
171+
public HttpProtocolVersionTest(
172+
Level logLevel, String protocolVersionName, List<String> expectedMessages) {
173+
this.logLevel = logLevel;
174+
this.protocolVersionName = protocolVersionName;
175+
logger.expectMessages(expectedMessages);
176+
}
177+
178+
@Parameters
179+
public static Iterable<Object[]> data() {
180+
return Arrays.asList(
181+
new Object[][] {
182+
{
183+
Level.BASIC,
184+
null,
185+
Arrays.asList(
186+
"\\[SendsStuff#login\\] ---> POST http://localhost:[0-9]+/ HTTP/1.1",
187+
"\\[SendsStuff#login\\] <--- HTTP/1.1 200 \\([0-9]+ms\\)")
188+
},
189+
{
190+
Level.BASIC,
191+
"HTTP/1.1",
192+
Arrays.asList(
193+
"\\[SendsStuff#login\\] ---> POST http://localhost:[0-9]+/ HTTP/1.1",
194+
"\\[SendsStuff#login\\] <--- HTTP/1.1 200 \\([0-9]+ms\\)")
195+
},
196+
{
197+
Level.BASIC,
198+
"HTTP/2.0",
199+
Arrays.asList(
200+
"\\[SendsStuff#login\\] ---> POST http://localhost:[0-9]+/ HTTP/1.1",
201+
"\\[SendsStuff#login\\] <--- HTTP/2.0 200 \\([0-9]+ms\\)")
202+
},
203+
{
204+
Level.BASIC,
205+
"HTTP-XYZ",
206+
Arrays.asList(
207+
"\\[SendsStuff#login\\] ---> POST http://localhost:[0-9]+/ HTTP/1.1",
208+
"\\[SendsStuff#login\\] <--- UNKNOWN 200 \\([0-9]+ms\\)")
209+
}
210+
});
211+
}
212+
213+
@Test
214+
public void testHttpProtocolVersion() {
215+
server.enqueue(new MockResponse().setStatus("HTTP/1.1 " + 200));
216+
217+
SendsStuff api =
218+
Feign.builder()
219+
.client(new TestProtocolVersionClient(protocolVersionName))
220+
.logger(logger)
221+
.logLevel(logLevel)
222+
.target(SendsStuff.class, "http://localhost:" + server.getPort());
223+
224+
api.login("netflix", "denominator", "password");
225+
}
226+
}
227+
162228
@RunWith(Parameterized.class)
163229
public static class ReadTimeoutEmitsTest extends LoggerTest {
164230

@@ -495,4 +561,25 @@ public void evaluate() throws Throwable {
495561
};
496562
}
497563
}
564+
565+
private static final class TestProtocolVersionClient extends Client.Default {
566+
private final String protocolVersionName;
567+
568+
public TestProtocolVersionClient(String protocolVersionName) {
569+
super(null, null);
570+
this.protocolVersionName = protocolVersionName;
571+
}
572+
573+
@Override
574+
Response convertResponse(HttpURLConnection connection, Request request) throws IOException {
575+
Response response = super.convertResponse(connection, request);
576+
if (nonNull((protocolVersionName))) {
577+
response =
578+
response.toBuilder()
579+
.protocolVersion(enumForName(ProtocolVersion.class, protocolVersionName))
580+
.build();
581+
}
582+
return response;
583+
}
584+
}
498585
}

0 commit comments

Comments
 (0)