Skip to content

Commit cc650a0

Browse files
Nick Fiaccoadriancole
Nick Fiacco
authored and
adriancole
committed
added builder methods and Request field for Response object (OpenFeign#436)
1 parent 30753bf commit cc650a0

17 files changed

Lines changed: 337 additions & 104 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
### Version 9.2
22
* Supports context path when using Ribbon `LoadBalancingTarget`
3+
* Adds builder methods for the Response object
4+
* Deprecates Response factory methods
5+
* Adds nullable Request field to the Response object
36

47
### Version 9.1
58
* Allows query parameters to match on a substring. Ex `q=body:{body}`

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public Default(SSLSocketFactory sslContextFactory, HostnameVerifier hostnameVeri
7171
@Override
7272
public Response execute(Request request, Options options) throws IOException {
7373
HttpURLConnection connection = convertAndSend(request, options);
74-
return convertResponse(connection);
74+
return convertResponse(connection).toBuilder().request(request).build();
7575
}
7676

7777
HttpURLConnection convertAndSend(Request request, Options options) throws IOException {
@@ -175,7 +175,12 @@ Response convertResponse(HttpURLConnection connection) throws IOException {
175175
} else {
176176
stream = connection.getInputStream();
177177
}
178-
return Response.create(status, reason, headers, stream, length);
178+
return Response.builder()
179+
.status(status)
180+
.reason(reason)
181+
.headers(headers)
182+
.body(stream, length)
183+
.build();
179184
}
180185
}
181186
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ protected Response logAndRebufferResponse(String configKey, Level logLevel, Resp
9999
log(configKey, "%s", decodeOrDefault(bodyData, UTF_8, "Binary data"));
100100
}
101101
log(configKey, "<--- END HTTP (%s-byte body)", bodyLength);
102-
return Response.create(response.status(), response.reason(), response.headers(), bodyData);
102+
return response.toBuilder().body(bodyData).build();
103103
} else {
104104
log(configKey, "<--- END HTTP (%s-byte body)", bodyLength);
105105
}

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

Lines changed: 139 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -44,33 +44,154 @@ public final class Response implements Closeable {
4444
private final String reason;
4545
private final Map<String, Collection<String>> headers;
4646
private final Body body;
47-
48-
private Response(int status, String reason, Map<String, Collection<String>> headers, Body body) {
49-
checkState(status >= 200, "Invalid status code: %s", status);
50-
this.status = status;
51-
this.reason = reason; //nullable
52-
this.headers = Collections.unmodifiableMap(caseInsensitiveCopyOf(headers));
53-
this.body = body; //nullable
47+
private final Request request;
48+
49+
private Response(Builder builder) {
50+
checkState(builder.status >= 200, "Invalid status code: %s", builder.status);
51+
this.status = builder.status;
52+
this.reason = builder.reason; //nullable
53+
this.headers = Collections.unmodifiableMap(caseInsensitiveCopyOf(builder.headers));
54+
this.body = builder.body; //nullable
55+
this.request = builder.request; //nullable
5456
}
5557

58+
/**
59+
* @deprecated To be removed in Feign 10
60+
*/
61+
@Deprecated
5662
public static Response create(int status, String reason, Map<String, Collection<String>> headers,
5763
InputStream inputStream, Integer length) {
58-
return new Response(status, reason, headers, InputStreamBody.orNull(inputStream, length));
64+
return Response.builder()
65+
.status(status)
66+
.reason(reason)
67+
.headers(headers)
68+
.body(InputStreamBody.orNull(inputStream, length))
69+
.build();
5970
}
6071

72+
/**
73+
* @deprecated To be removed in Feign 10
74+
*/
75+
@Deprecated
6176
public static Response create(int status, String reason, Map<String, Collection<String>> headers,
6277
byte[] data) {
63-
return new Response(status, reason, headers, ByteArrayBody.orNull(data));
78+
return Response.builder()
79+
.status(status)
80+
.reason(reason)
81+
.headers(headers)
82+
.body(ByteArrayBody.orNull(data))
83+
.build();
6484
}
6585

86+
/**
87+
* @deprecated To be removed in Feign 10
88+
*/
89+
@Deprecated
6690
public static Response create(int status, String reason, Map<String, Collection<String>> headers,
6791
String text, Charset charset) {
68-
return new Response(status, reason, headers, ByteArrayBody.orNull(text, charset));
92+
return Response.builder()
93+
.status(status)
94+
.reason(reason)
95+
.headers(headers)
96+
.body(ByteArrayBody.orNull(text, charset))
97+
.build();
6998
}
7099

100+
/**
101+
* @deprecated To be removed in Feign 10
102+
*/
103+
@Deprecated
71104
public static Response create(int status, String reason, Map<String, Collection<String>> headers,
72105
Body body) {
73-
return new Response(status, reason, headers, body);
106+
return Response.builder()
107+
.status(status)
108+
.reason(reason)
109+
.headers(headers)
110+
.body(body)
111+
.build();
112+
}
113+
114+
public Builder toBuilder(){
115+
return new Builder(this);
116+
}
117+
118+
public static Builder builder(){
119+
return new Builder();
120+
}
121+
122+
public static final class Builder {
123+
int status;
124+
String reason;
125+
Map<String, Collection<String>> headers;
126+
Body body;
127+
Request request;
128+
129+
Builder() {
130+
}
131+
132+
Builder(Response source) {
133+
this.status = source.status;
134+
this.reason = source.reason;
135+
this.headers = source.headers;
136+
this.body = source.body;
137+
this.request = source.request;
138+
}
139+
140+
/** @see Response#status*/
141+
public Builder status(int status) {
142+
this.status = status;
143+
return this;
144+
}
145+
146+
/** @see Response#reason */
147+
public Builder reason(String reason) {
148+
this.reason = reason;
149+
return this;
150+
}
151+
152+
/** @see Response#headers */
153+
public Builder headers(Map<String, Collection<String>> headers) {
154+
this.headers = headers;
155+
return this;
156+
}
157+
158+
/** @see Response#body */
159+
public Builder body(Body body) {
160+
this.body = body;
161+
return this;
162+
}
163+
164+
/** @see Response#body */
165+
public Builder body(InputStream inputStream, Integer length) {
166+
this.body = InputStreamBody.orNull(inputStream, length);
167+
return this;
168+
}
169+
170+
/** @see Response#body */
171+
public Builder body(byte[] data) {
172+
this.body = ByteArrayBody.orNull(data);
173+
return this;
174+
}
175+
176+
/** @see Response#body */
177+
public Builder body(String text, Charset charset) {
178+
this.body = ByteArrayBody.orNull(text, charset);
179+
return this;
180+
}
181+
182+
/** @see Response#request
183+
*
184+
* NOTE: will add null check in version 10 which may require changes
185+
* to custom feign.Client or loggers
186+
*/
187+
public Builder request(Request request) {
188+
this.request = request;
189+
return this;
190+
}
191+
192+
public Response build() {
193+
return new Response(this);
194+
}
74195
}
75196

76197
/**
@@ -105,6 +226,13 @@ public Body body() {
105226
return body;
106227
}
107228

229+
/**
230+
* if present, the request that generated this response
231+
*/
232+
public Request request() {
233+
return request;
234+
}
235+
108236
@Override
109237
public String toString() {
110238
StringBuilder builder = new StringBuilder("HTTP/1.1 ").append(status);

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,8 @@ Object executeAndDecode(RequestTemplate template) throws Throwable {
9595
long start = System.nanoTime();
9696
try {
9797
response = client.execute(request, options);
98+
// ensure the request is set. TODO: remove in Feign 10
99+
response.toBuilder().request(request).build();
98100
} catch (IOException e) {
99101
if (logLevel != Logger.Level.NONE) {
100102
logger.logIOException(metadata.configKey(), logLevel, e, elapsedTime(start));
@@ -108,6 +110,8 @@ Object executeAndDecode(RequestTemplate template) throws Throwable {
108110
if (logLevel != Logger.Level.NONE) {
109111
response =
110112
logger.logAndRebufferResponse(metadata.configKey(), logLevel, response, elapsedTime);
113+
// ensure the request is set. TODO: remove in Feign 10
114+
response.toBuilder().request(request).build();
111115
}
112116
if (Response.class == metadata.returnType()) {
113117
if (response.body() == null) {
@@ -120,7 +124,7 @@ Object executeAndDecode(RequestTemplate template) throws Throwable {
120124
}
121125
// Ensure the response body is disconnected
122126
byte[] bodyData = Util.toByteArray(response.body().asInputStream());
123-
return Response.create(response.status(), response.reason(), response.headers(), bodyData);
127+
return response.toBuilder().body(bodyData).build();
124128
}
125129
if (response.status() >= 200 && response.status() < 300) {
126130
if (void.class == metadata.returnType()) {

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,12 @@ public Exception decode(String methodKey, Response response)
492492
public void whenReturnTypeIsResponseNoErrorHandling() {
493493
Map<String, Collection<String>> headers = new LinkedHashMap<String, Collection<String>>();
494494
headers.put("Location", Arrays.asList("http://bar.com"));
495-
final Response response = Response.create(302, "Found", headers, new byte[0]);
495+
final Response response = Response.builder()
496+
.status(302)
497+
.reason("Found")
498+
.headers(headers)
499+
.body(new byte[0])
500+
.build();
496501

497502
TestInterface api = Feign.builder()
498503
.client(new Client() { // fake client as Client.Default follows redirects.

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

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -31,41 +31,41 @@ public class ResponseTest {
3131

3232
@Test
3333
public void reasonPhraseIsOptional() {
34-
Response response = Response.create(200, null /* reason phrase */, Collections.
35-
<String, Collection<String>>emptyMap(), new byte[0]);
34+
Response response = Response.builder()
35+
.status(200)
36+
.headers(Collections.<String, Collection<String>>emptyMap())
37+
.body(new byte[0])
38+
.build();
3639

3740
assertThat(response.reason()).isNull();
3841
assertThat(response.toString()).isEqualTo("HTTP/1.1 200\n\n");
3942
}
4043

41-
@Test
42-
public void lowerCasesNamesOfHeaders() {
43-
Response response = Response.create(200,
44-
null,
45-
Collections.singletonMap("Content-Type",
46-
Collections.singletonList("application/json")),
47-
new byte[0]);
48-
assertThat(response.headers()).containsOnly(entry(("content-type"), Collections.singletonList("application/json")));
49-
}
50-
5144
@Test
5245
public void canAccessHeadersCaseInsensitively() {
46+
Map<String, Collection<String>> headersMap = new LinkedHashMap();
5347
List<String> valueList = Collections.singletonList("application/json");
54-
Response response = Response.create(200,
55-
null,
56-
Collections.singletonMap("Content-Type", valueList),
57-
new byte[0]);
48+
headersMap.put("Content-Type", valueList);
49+
Response response = Response.builder()
50+
.status(200)
51+
.headers(headersMap)
52+
.body(new byte[0])
53+
.build();
5854
assertThat(response.headers().get("content-type")).isEqualTo(valueList);
5955
assertThat(response.headers().get("Content-Type")).isEqualTo(valueList);
6056
}
6157

6258
@Test
6359
public void headerValuesWithSameNameOnlyVaryingInCaseAreMerged() {
64-
Map<String, Collection<String>> headersMap = new LinkedHashMap<>();
60+
Map<String, Collection<String>> headersMap = new LinkedHashMap();
6561
headersMap.put("Set-Cookie", Arrays.asList("Cookie-A=Value", "Cookie-B=Value"));
6662
headersMap.put("set-cookie", Arrays.asList("Cookie-C=Value"));
6763

68-
Response response = Response.create(200, null, headersMap, new byte[0]);
64+
Response response = Response.builder()
65+
.status(200)
66+
.headers(headersMap)
67+
.body(new byte[0])
68+
.build();
6969

7070
List<String> expectedHeaderValue = Arrays.asList("Cookie-A=Value", "Cookie-B=Value", "Cookie-C=Value");
7171
assertThat(response.headers()).containsOnly(entry(("set-cookie"), expectedHeaderValue));

‎core/src/test/java/feign/codec/DefaultDecoderTest.java‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,19 @@ private Response knownResponse() {
7474
InputStream inputStream = new ByteArrayInputStream(content.getBytes(UTF_8));
7575
Map<String, Collection<String>> headers = new HashMap<String, Collection<String>>();
7676
headers.put("Content-Type", Collections.singleton("text/plain"));
77-
return Response.create(200, "OK", headers, inputStream, content.length());
77+
return Response.builder()
78+
.status(200)
79+
.reason("OK")
80+
.headers(headers)
81+
.body(inputStream, content.length())
82+
.build();
7883
}
7984

8085
private Response nullBodyResponse() {
81-
return Response
82-
.create(200, "OK", Collections.<String, Collection<String>>emptyMap(), (byte[]) null);
86+
return Response.builder()
87+
.status(200)
88+
.reason("OK")
89+
.headers(Collections.<String, Collection<String>>emptyMap())
90+
.build();
8391
}
8492
}

‎core/src/test/java/feign/codec/DefaultErrorDecoderTest.java‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,11 @@ public void throwsFeignException() throws Throwable {
4545
thrown.expect(FeignException.class);
4646
thrown.expectMessage("status 500 reading Service#foo()");
4747

48-
Response response = Response.create(500, "Internal server error", headers, (byte[]) null);
48+
Response response = Response.builder()
49+
.status(500)
50+
.reason("Internal server error")
51+
.headers(headers)
52+
.build();
4953

5054
throw errorDecoder.decode("Service#foo()", response);
5155
}
@@ -55,14 +59,23 @@ public void throwsFeignExceptionIncludingBody() throws Throwable {
5559
thrown.expect(FeignException.class);
5660
thrown.expectMessage("status 500 reading Service#foo(); content:\nhello world");
5761

58-
Response response = Response.create(500, "Internal server error", headers, "hello world", UTF_8);
62+
Response response = Response.builder()
63+
.status(500)
64+
.reason("Internal server error")
65+
.headers(headers)
66+
.body("hello world", UTF_8)
67+
.build();
5968

6069
throw errorDecoder.decode("Service#foo()", response);
6170
}
6271

6372
@Test
6473
public void testFeignExceptionIncludesStatus() throws Throwable {
65-
Response response = Response.create(400, "Bad request", headers, (byte[]) null);
74+
Response response = Response.builder()
75+
.status(400)
76+
.reason("Bad request")
77+
.headers(headers)
78+
.build();
6679

6780
Exception exception = errorDecoder.decode("Service#foo()", response);
6881

@@ -76,7 +89,11 @@ public void retryAfterHeaderThrowsRetryableException() throws Throwable {
7689
thrown.expectMessage("status 503 reading Service#foo()");
7790

7891
headers.put(RETRY_AFTER, Arrays.asList("Sat, 1 Jan 2000 00:00:00 GMT"));
79-
Response response = Response.create(503, "Service Unavailable", headers, (byte[]) null);
92+
Response response = Response.builder()
93+
.status(503)
94+
.reason("Service Unavailable")
95+
.headers(headers)
96+
.build();
8097

8198
throw errorDecoder.decode("Service#foo()", response);
8299
}

0 commit comments

Comments
 (0)