added builder methods and Request field for Response object (OpenFeig…

…n#436)

CHANGELOG.md

@@ -1,5 +1,8 @@
 ### Version 9.2
 * Supports context path when using Ribbon `LoadBalancingTarget`
+* Adds builder methods for the Response object
+* Deprecates Response factory methods
+* Adds nullable Request field to the Response object
 
 ### Version 9.1
 * Allows query parameters to match on a substring. Ex `q=body:{body}`

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

@@ -71,7 +71,7 @@ public Default(SSLSocketFactory sslContextFactory, HostnameVerifier hostnameVeri
     @Override
     public Response execute(Request request, Options options) throws IOException {
       HttpURLConnection connection = convertAndSend(request, options);
-      return convertResponse(connection);
+      return convertResponse(connection).toBuilder().request(request).build();
     }
 
     HttpURLConnection convertAndSend(Request request, Options options) throws IOException {
@@ -175,7 +175,12 @@ Response convertResponse(HttpURLConnection connection) throws IOException {
       } else {
         stream = connection.getInputStream();
       }
-      return Response.create(status, reason, headers, stream, length);
+      return Response.builder()
+              .status(status)
+              .reason(reason)
+              .headers(headers)
+              .body(stream, length)
+              .build();
     }
   }
 }

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

@@ -99,7 +99,7 @@ protected Response logAndRebufferResponse(String configKey, Level logLevel, Resp
           log(configKey, "%s", decodeOrDefault(bodyData, UTF_8, "Binary data"));
         }
         log(configKey, "<--- END HTTP (%s-byte body)", bodyLength);
-        return Response.create(response.status(), response.reason(), response.headers(), bodyData);
+        return response.toBuilder().body(bodyData).build();
       } else {
         log(configKey, "<--- END HTTP (%s-byte body)", bodyLength);
       }

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

@@ -44,33 +44,154 @@ public final class Response implements Closeable {
   private final String reason;
   private final Map<String, Collection<String>> headers;
   private final Body body;
-
-  private Response(int status, String reason, Map<String, Collection<String>> headers, Body body) {
-    checkState(status >= 200, "Invalid status code: %s", status);
-    this.status = status;
-    this.reason = reason; //nullable
-    this.headers = Collections.unmodifiableMap(caseInsensitiveCopyOf(headers));
-    this.body = body; //nullable
+  private final Request request;
+
+  private Response(Builder builder) {
+    checkState(builder.status >= 200, "Invalid status code: %s", builder.status);
+    this.status = builder.status;
+    this.reason = builder.reason; //nullable
+    this.headers = Collections.unmodifiableMap(caseInsensitiveCopyOf(builder.headers));
+    this.body = builder.body; //nullable
+    this.request = builder.request; //nullable
   }
 
+  /**
+   * @deprecated  To be removed in Feign 10
+   */
+  @Deprecated
   public static Response create(int status, String reason, Map<String, Collection<String>> headers,
                                 InputStream inputStream, Integer length) {
-    return new Response(status, reason, headers, InputStreamBody.orNull(inputStream, length));
+    return Response.builder()
+            .status(status)
+            .reason(reason)
+            .headers(headers)
+            .body(InputStreamBody.orNull(inputStream, length))
+            .build();
   }
 
+  /**
+   * @deprecated  To be removed in Feign 10
+   */
+  @Deprecated
   public static Response create(int status, String reason, Map<String, Collection<String>> headers,
                                 byte[] data) {
-    return new Response(status, reason, headers, ByteArrayBody.orNull(data));
+    return Response.builder()
+            .status(status)
+            .reason(reason)
+            .headers(headers)
+            .body(ByteArrayBody.orNull(data))
+            .build();
   }
 
+  /**
+   * @deprecated  To be removed in Feign 10
+   */
+  @Deprecated
   public static Response create(int status, String reason, Map<String, Collection<String>> headers,
                                 String text, Charset charset) {
-    return new Response(status, reason, headers, ByteArrayBody.orNull(text, charset));
+    return Response.builder()
+            .status(status)
+            .reason(reason)
+            .headers(headers)
+            .body(ByteArrayBody.orNull(text, charset))
+            .build();
   }
 
+  /**
+   * @deprecated  To be removed in Feign 10
+   */
+  @Deprecated
   public static Response create(int status, String reason, Map<String, Collection<String>> headers,
                                 Body body) {
-    return new Response(status, reason, headers, body);
+    return Response.builder()
+            .status(status)
+            .reason(reason)
+            .headers(headers)
+            .body(body)
+            .build();
+  }
+
+  public Builder toBuilder(){
+    return new Builder(this);
+  }
+
+  public static Builder builder(){
+    return new Builder();
+  }
+
+  public static final class Builder {
+    int status;
+    String reason;
+    Map<String, Collection<String>> headers;
+    Body body;
+    Request request;
+
+    Builder() {
+    }
+
+    Builder(Response source) {
+      this.status = source.status;
+      this.reason = source.reason;
+      this.headers = source.headers;
+      this.body = source.body;
+      this.request = source.request;
+    }
+
+    /** @see Response#status*/
+    public Builder status(int status) {
+      this.status = status;
+      return this;
+    }
+
+    /** @see Response#reason */
+    public Builder reason(String reason) {
+      this.reason = reason;
+      return this;
+    }
+
+    /** @see Response#headers */
+    public Builder headers(Map<String, Collection<String>> headers) {
+      this.headers = headers;
+      return this;
+    }
+
+    /** @see Response#body */
+    public Builder body(Body body) {
+      this.body = body;
+      return this;
+    }
+
+    /** @see Response#body */
+    public Builder body(InputStream inputStream, Integer length) {
+      this.body = InputStreamBody.orNull(inputStream, length);
+      return this;
+    }
+
+    /** @see Response#body */
+    public Builder body(byte[] data) {
+      this.body = ByteArrayBody.orNull(data);
+      return this;
+    }
+
+    /** @see Response#body */
+    public Builder body(String text, Charset charset) {
+      this.body = ByteArrayBody.orNull(text, charset);
+      return this;
+    }
+
+    /** @see Response#request
+    *
+    * NOTE: will add null check in version 10 which may require changes
+    * to custom feign.Client or loggers
+    */
+    public Builder request(Request request) {
+      this.request = request;
+      return this;
+    }
+
+    public Response build() {
+      return new Response(this);
+    }
   }
 
   /**
@@ -105,6 +226,13 @@ public Body body() {
     return body;
   }
 
+  /**
+   * if present, the request that generated this response
+   */
+  public Request request() {
+    return request;
+  }
+
   @Override
   public String toString() {
     StringBuilder builder = new StringBuilder("HTTP/1.1 ").append(status);

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

@@ -95,6 +95,8 @@ Object executeAndDecode(RequestTemplate template) throws Throwable {
     long start = System.nanoTime();
     try {
       response = client.execute(request, options);
+      // ensure the request is set. TODO: remove in Feign 10
+      response.toBuilder().request(request).build();
     } catch (IOException e) {
       if (logLevel != Logger.Level.NONE) {
         logger.logIOException(metadata.configKey(), logLevel, e, elapsedTime(start));
@@ -108,6 +110,8 @@ Object executeAndDecode(RequestTemplate template) throws Throwable {
       if (logLevel != Logger.Level.NONE) {
         response =
             logger.logAndRebufferResponse(metadata.configKey(), logLevel, response, elapsedTime);
+        // ensure the request is set. TODO: remove in Feign 10
+        response.toBuilder().request(request).build();
       }
       if (Response.class == metadata.returnType()) {
         if (response.body() == null) {
@@ -120,7 +124,7 @@ Object executeAndDecode(RequestTemplate template) throws Throwable {
         }
         // Ensure the response body is disconnected
         byte[] bodyData = Util.toByteArray(response.body().asInputStream());
-        return Response.create(response.status(), response.reason(), response.headers(), bodyData);
+        return response.toBuilder().body(bodyData).build();
       }
       if (response.status() >= 200 && response.status() < 300) {
         if (void.class == metadata.returnType()) {

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

@@ -492,7 +492,12 @@ public Exception decode(String methodKey, Response response)
   public void whenReturnTypeIsResponseNoErrorHandling() {
     Map<String, Collection<String>> headers = new LinkedHashMap<String, Collection<String>>();
     headers.put("Location", Arrays.asList("http://bar.com"));
-    final Response response = Response.create(302, "Found", headers, new byte[0]);
+    final Response response = Response.builder()
+            .status(302)
+            .reason("Found")
+            .headers(headers)
+            .body(new byte[0])
+            .build();
 
     TestInterface api = Feign.builder()
         .client(new Client() { // fake client as Client.Default follows redirects.

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

@@ -31,41 +31,41 @@ public class ResponseTest {
 
   @Test
   public void reasonPhraseIsOptional() {
-    Response response = Response.create(200, null /* reason phrase */, Collections.
-        <String, Collection<String>>emptyMap(), new byte[0]);
+    Response response = Response.builder()
+            .status(200)
+            .headers(Collections.<String, Collection<String>>emptyMap())
+            .body(new byte[0])
+            .build();
 
     assertThat(response.reason()).isNull();
     assertThat(response.toString()).isEqualTo("HTTP/1.1 200\n\n");
   }
 
-  @Test
-  public void lowerCasesNamesOfHeaders() {
-    Response response = Response.create(200,
-                                        null,
-                                        Collections.singletonMap("Content-Type",
-                                                                 Collections.singletonList("application/json")),
-                                        new byte[0]);
-    assertThat(response.headers()).containsOnly(entry(("content-type"), Collections.singletonList("application/json")));
-  }
-
   @Test
   public void canAccessHeadersCaseInsensitively() {
+    Map<String, Collection<String>> headersMap = new LinkedHashMap();
     List<String> valueList = Collections.singletonList("application/json");
-    Response response = Response.create(200,
-                                        null,
-                                        Collections.singletonMap("Content-Type", valueList),
-                                        new byte[0]);
+    headersMap.put("Content-Type", valueList);
+    Response response = Response.builder()
+            .status(200)
+            .headers(headersMap)
+            .body(new byte[0])
+            .build();
     assertThat(response.headers().get("content-type")).isEqualTo(valueList);
     assertThat(response.headers().get("Content-Type")).isEqualTo(valueList);
   }
 
   @Test
   public void headerValuesWithSameNameOnlyVaryingInCaseAreMerged() {
-    Map<String, Collection<String>> headersMap = new LinkedHashMap<>();
+    Map<String, Collection<String>> headersMap = new LinkedHashMap();
     headersMap.put("Set-Cookie", Arrays.asList("Cookie-A=Value", "Cookie-B=Value"));
     headersMap.put("set-cookie", Arrays.asList("Cookie-C=Value"));
 
-    Response response = Response.create(200, null, headersMap, new byte[0]);
+    Response response = Response.builder()
+            .status(200)
+            .headers(headersMap)
+            .body(new byte[0])
+            .build();
 
     List<String> expectedHeaderValue = Arrays.asList("Cookie-A=Value", "Cookie-B=Value", "Cookie-C=Value");
     assertThat(response.headers()).containsOnly(entry(("set-cookie"), expectedHeaderValue));

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

@@ -74,11 +74,19 @@ private Response knownResponse() {
     InputStream inputStream = new ByteArrayInputStream(content.getBytes(UTF_8));
     Map<String, Collection<String>> headers = new HashMap<String, Collection<String>>();
     headers.put("Content-Type", Collections.singleton("text/plain"));
-    return Response.create(200, "OK", headers, inputStream, content.length());
+    return Response.builder()
+            .status(200)
+            .reason("OK")
+            .headers(headers)
+            .body(inputStream, content.length())
+            .build();
   }
 
   private Response nullBodyResponse() {
-    return Response
-        .create(200, "OK", Collections.<String, Collection<String>>emptyMap(), (byte[]) null);
+    return Response.builder()
+            .status(200)
+            .reason("OK")
+            .headers(Collections.<String, Collection<String>>emptyMap())
+            .build();
   }
 }

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

@@ -45,7 +45,11 @@ public void throwsFeignException() throws Throwable {
     thrown.expect(FeignException.class);
     thrown.expectMessage("status 500 reading Service#foo()");
 
-    Response response = Response.create(500, "Internal server error", headers, (byte[]) null);
+    Response response = Response.builder()
+            .status(500)
+            .reason("Internal server error")
+            .headers(headers)
+            .build();
 
     throw errorDecoder.decode("Service#foo()", response);
   }
@@ -55,14 +59,23 @@ public void throwsFeignExceptionIncludingBody() throws Throwable {
     thrown.expect(FeignException.class);
     thrown.expectMessage("status 500 reading Service#foo(); content:\nhello world");
 
-    Response response = Response.create(500, "Internal server error", headers, "hello world", UTF_8);
+    Response response = Response.builder()
+            .status(500)
+            .reason("Internal server error")
+            .headers(headers)
+            .body("hello world", UTF_8)
+            .build();
 
     throw errorDecoder.decode("Service#foo()", response);
   }
 
   @Test
   public void testFeignExceptionIncludesStatus() throws Throwable {
-    Response response = Response.create(400, "Bad request", headers, (byte[]) null);
+    Response response = Response.builder()
+            .status(400)
+            .reason("Bad request")
+            .headers(headers)
+            .build();
 
     Exception exception = errorDecoder.decode("Service#foo()", response);
 
@@ -76,7 +89,11 @@ public void retryAfterHeaderThrowsRetryableException() throws Throwable {
     thrown.expectMessage("status 503 reading Service#foo()");
 
     headers.put(RETRY_AFTER, Arrays.asList("Sat, 1 Jan 2000 00:00:00 GMT"));
-    Response response = Response.create(503, "Service Unavailable", headers, (byte[]) null);
+    Response response = Response.builder()
+            .status(503)
+            .reason("Service Unavailable")
+            .headers(headers)
+            .build();
 
     throw errorDecoder.decode("Service#foo()", response);
   }