Modernize StripeResponse (#932)

Author: ob-stripe

Diff for: src/main/java/com/stripe/net/HttpClient.java

@@ -114,7 +114,7 @@ public StripeResponse requestWithRetries(StripeRequest request) throws StripeExc
       throw requestException;
     }
 
-    response.setNumRetries(retry);
+    response.numRetries(retry);
 
     return response;
   }

Diff for: src/main/java/com/stripe/net/HttpURLConnectionClient.java

@@ -50,18 +50,16 @@ public StripeResponse request(StripeRequest request) throws ApiConnectionExcepti
 
       // trigger the request
       int responseCode = conn.getResponseCode();
+      HttpHeaders headers = HttpHeaders.of(conn.getHeaderFields());
       String responseBody;
-      Map<String, List<String>> headers;
 
       if (responseCode >= 200 && responseCode < 300) {
         responseBody = getResponseBody(conn.getInputStream());
       } else {
         responseBody = getResponseBody(conn.getErrorStream());
       }
 
-      headers = conn.getHeaderFields();
-
-      return new StripeResponse(responseCode, responseBody, headers);
+      return new StripeResponse(responseCode, headers, responseBody);
 
     } catch (IOException e) {
       throw new ApiConnectionException(

Diff for: src/main/java/com/stripe/net/StripeResponse.java

@@ -1,54 +1,82 @@
 package com.stripe.net;
 
-import java.util.List;
-import java.util.Map;
+import static java.util.Objects.requireNonNull;
 
-public class StripeResponse {
+import java.time.Instant;
+import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.Optional;
+import lombok.AccessLevel;
+import lombok.Getter;
+import lombok.Setter;
+import lombok.Value;
+import lombok.experimental.Accessors;
+import lombok.experimental.NonFinal;
 
+/** A response from Stripe's API. */
+@Value
+@Accessors(fluent = true)
+public class StripeResponse {
+  /** The HTTP status code of the response. */
   int code;
-  String body;
+
+  /** The HTTP headers of the response. */
   HttpHeaders headers;
+
+  /** The body of the response. */
+  String body;
+
+  /** Number of times the request was retried. Used for internal tests only. */
+  @NonFinal
+  @Getter(AccessLevel.PACKAGE)
+  @Setter(AccessLevel.PACKAGE)
   int numRetries;
 
-  /** Constructs a Stripe response with the specified status code and body. */
-  public StripeResponse(int code, String body) {
-    this.code = code;
-    this.body = body;
-    this.headers = null;
-  }
+  /**
+   * Initializes a new instance of the {@link StripeResponse} class.
+   *
+   * @param code the HTTP status code of the response
+   * @param headers the HTTP headers of the response
+   * @param body the body of the response
+   * @throws NullPointerException if {@code headers} or {@code body} is {@code null}
+   */
+  public StripeResponse(int code, HttpHeaders headers, String body) {
+    requireNonNull(headers);
+    requireNonNull(body);
 
-  /** Constructs a Stripe response with the specified status code, body and headers. */
-  public StripeResponse(int code, String body, Map<String, List<String>> headers) {
     this.code = code;
+    this.headers = headers;
     this.body = body;
-    this.headers = HttpHeaders.of(headers);
-  }
-
-  public int code() {
-    return this.code;
-  }
-
-  public String body() {
-    return this.body;
   }
 
-  public HttpHeaders headers() {
-    return headers;
+  /**
+   * Gets the date of the request, as returned by Stripe.
+   *
+   * @return the date of the request, as returned by Stripe
+   */
+  public Instant date() {
+    Optional<String> dateStr = this.headers.firstValue("Date");
+    if (!dateStr.isPresent()) {
+      return null;
+    }
+    return ZonedDateTime.parse(dateStr.get(), DateTimeFormatter.RFC_1123_DATE_TIME).toInstant();
   }
 
+  /**
+   * Gets the idempotency key of the request, as returned by Stripe.
+   *
+   * @return the idempotency key of the request, as returned by Stripe
+   */
   public String idempotencyKey() {
-    return (headers != null) ? headers.firstValue("Idempotency-Key").orElse(null) : null;
+    return this.headers.firstValue("Idempotency-Key").orElse(null);
   }
 
+  /**
+   * Gets the ID of the request, as returned by Stripe.
+   *
+   * @return the ID of the request, as returned by Stripe
+   */
   public String requestId() {
-    return (headers != null) ? headers.firstValue("Request-Id").orElse(null) : null;
-  }
-
-  public int numRetries() {
-    return this.numRetries;
-  }
-
-  void setNumRetries(int numRetries) {
-    this.numRetries = numRetries;
+    return this.headers.firstValue("Request-Id").orElse(null);
   }
 }

Diff for: src/test/java/com/stripe/net/HttpClientTest.java

@@ -12,6 +12,7 @@
 import com.stripe.exception.ApiConnectionException;
 import com.stripe.exception.StripeException;
 import java.net.ConnectException;
+import java.util.Collections;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
@@ -21,6 +22,8 @@ public class HttpClientTest extends BaseStripeTest {
 
   private StripeRequest request;
 
+  private HttpHeaders emptyHeaders = HttpHeaders.of(Collections.emptyMap());
+
   @BeforeEach
   public void setUpFixtures() throws StripeException {
     this.client =
@@ -37,7 +40,7 @@ public void setUpFixtures() throws StripeException {
   public void testRequestWithRetriesConnectException() throws StripeException {
     Mockito.when(this.client.request(this.request))
         .thenThrow(new ApiConnectionException("foo", new ConnectException("timeout or something")))
-        .thenReturn(new StripeResponse(200, "{}"));
+        .thenReturn(new StripeResponse(200, emptyHeaders, "{}"));
 
     StripeResponse response = this.client.requestWithRetries(this.request);
 
@@ -71,8 +74,10 @@ public void testRequestWithRetriesStripeShouldRetryTrue() throws StripeException
     Mockito.when(this.client.request(this.request))
         .thenReturn(
             new StripeResponse(
-                400, "{}", ImmutableMap.of("Stripe-Should-Retry", ImmutableList.of("true"))))
-        .thenReturn(new StripeResponse(200, "{}"));
+                400,
+                HttpHeaders.of(ImmutableMap.of("Stripe-Should-Retry", ImmutableList.of("true"))),
+                "{}"))
+        .thenReturn(new StripeResponse(200, emptyHeaders, "{}"));
 
     StripeResponse response = this.client.requestWithRetries(this.request);
 
@@ -86,7 +91,9 @@ public void testRequestWithRetriesStripeShouldRetryFalse() throws StripeExceptio
     Mockito.when(this.client.request(this.request))
         .thenReturn(
             new StripeResponse(
-                400, "{}", ImmutableMap.of("Stripe-Should-Retry", ImmutableList.of("false"))));
+                400,
+                HttpHeaders.of(ImmutableMap.of("Stripe-Should-Retry", ImmutableList.of("false"))),
+                "{}"));
 
     StripeResponse response = this.client.requestWithRetries(this.request);
 
@@ -98,8 +105,8 @@ public void testRequestWithRetriesStripeShouldRetryFalse() throws StripeExceptio
   @Test
   public void testRequestWithRetriesConflict() throws StripeException {
     Mockito.when(this.client.request(this.request))
-        .thenReturn(new StripeResponse(409, "{}"))
-        .thenReturn(new StripeResponse(200, "{}"));
+        .thenReturn(new StripeResponse(409, emptyHeaders, "{}"))
+        .thenReturn(new StripeResponse(200, emptyHeaders, "{}"));
 
     StripeResponse response = this.client.requestWithRetries(this.request);
 
@@ -111,8 +118,8 @@ public void testRequestWithRetriesConflict() throws StripeException {
   @Test
   public void testRequestWithRetriesConflictServiceUnavailable() throws StripeException {
     Mockito.when(this.client.request(this.request))
-        .thenReturn(new StripeResponse(503, "{}"))
-        .thenReturn(new StripeResponse(200, "{}"));
+        .thenReturn(new StripeResponse(503, emptyHeaders, "{}"))
+        .thenReturn(new StripeResponse(200, emptyHeaders, "{}"));
 
     StripeResponse response = this.client.requestWithRetries(this.request);
 
@@ -124,8 +131,8 @@ public void testRequestWithRetriesConflictServiceUnavailable() throws StripeExce
   @Test
   public void testRequestWithRetriesConflictInternalServerError() throws StripeException {
     Mockito.when(this.client.request(this.request))
-        .thenReturn(new StripeResponse(500, "{}"))
-        .thenReturn(new StripeResponse(200, "{}"));
+        .thenReturn(new StripeResponse(500, emptyHeaders, "{}"))
+        .thenReturn(new StripeResponse(200, emptyHeaders, "{}"));
 
     StripeResponse response = this.client.requestWithRetries(this.request);
 

Diff for: src/test/java/com/stripe/net/StripeResponseTest.java

@@ -2,72 +2,85 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.stripe.BaseStripeTest;
-import java.util.ArrayList;
-import java.util.HashMap;
+import java.time.Instant;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import org.junit.jupiter.api.Test;
 
 public class StripeResponseTest extends BaseStripeTest {
-  String chargeBody;
+  private HttpHeaders emptyHeaders = HttpHeaders.of(Collections.emptyMap());
 
-  private Map<String, List<String>> generateHeaderMap() {
-    final List<String> idempotencyHeader = new ArrayList<>();
-    idempotencyHeader.add("12345");
-
-    final List<String> requestIdHeader = new ArrayList<>();
-    requestIdHeader.add("req_12345");
-
-    final Map<String, List<String>> headerMap = new HashMap<>();
-    headerMap.put("Idempotency-Key", idempotencyHeader);
-    headerMap.put("Request-Id", requestIdHeader);
+  @Test
+  public void testCtorNullHeaders() {
+    assertThrows(
+        NullPointerException.class,
+        () -> {
+          new StripeResponse(200, null, "");
+        });
+  }
 
-    return headerMap;
+  @Test
+  public void testCtorNullBody() {
+    assertThrows(
+        NullPointerException.class,
+        () -> {
+          new StripeResponse(200, emptyHeaders, null);
+        });
   }
 
   @Test
   public void testCode() {
-    StripeResponse stripeResponse = new StripeResponse(200, chargeBody);
+    StripeResponse stripeResponse = new StripeResponse(200, emptyHeaders, "");
     assertEquals(200, stripeResponse.code());
-    stripeResponse = new StripeResponse(201, chargeBody);
+    stripeResponse = new StripeResponse(201, emptyHeaders, "");
     assertEquals(201, stripeResponse.code());
   }
 
   @Test
   public void testBody() {
-    final StripeResponse stripeResponse = new StripeResponse(200, chargeBody);
+    final StripeResponse stripeResponse = new StripeResponse(200, emptyHeaders, "Response body");
     assertEquals(200, stripeResponse.code());
-    assertEquals(chargeBody, stripeResponse.body());
+    assertEquals("Response body", stripeResponse.body());
   }
 
   @Test
   public void testHeaders() {
-    final Map<String, List<String>> headerMap = generateHeaderMap();
-    final StripeResponse stripeResponse = new StripeResponse(200, chargeBody, headerMap);
+    Map<String, List<String>> headerMap =
+        ImmutableMap.of("Some-Header", ImmutableList.of("First value", "Second value"));
+    final StripeResponse stripeResponse = new StripeResponse(200, HttpHeaders.of(headerMap), "");
     assertNotNull(stripeResponse.headers());
+    assertTrue(stripeResponse.headers().firstValue("Some-Header").isPresent());
+    assertEquals("First value", stripeResponse.headers().firstValue("Some-Header").get());
   }
 
   @Test
-  public void testNoHeaders() {
-    final StripeResponse stripeResponse = new StripeResponse(200, chargeBody);
-    assertEquals(stripeResponse.headers(), null);
-    assertEquals(stripeResponse.idempotencyKey(), null);
-    assertEquals(stripeResponse.requestId(), null);
+  public void testDate() {
+    Map<String, List<String>> headerMap =
+        ImmutableMap.of("Date", ImmutableList.of("Fri, 13 Feb 2009 23:31:30 GMT"));
+    final StripeResponse stripeResponse = new StripeResponse(200, HttpHeaders.of(headerMap), "");
+    assertEquals(Instant.ofEpochSecond(1234567890), stripeResponse.date());
   }
 
   @Test
-  public void testGetIdempotencyKey() {
-    final Map<String, List<String>> headerMap = generateHeaderMap();
-    final StripeResponse stripeResponse = new StripeResponse(200, chargeBody, headerMap);
+  public void testIdempotencyKey() {
+    Map<String, List<String>> headerMap =
+        ImmutableMap.of("Idempotency-Key", ImmutableList.of("12345"));
+    final StripeResponse stripeResponse = new StripeResponse(200, HttpHeaders.of(headerMap), "");
     assertEquals("12345", stripeResponse.idempotencyKey());
   }
 
   @Test
   public void testRequestId() {
-    final Map<String, List<String>> headerMap = generateHeaderMap();
-    final StripeResponse stripeResponse = new StripeResponse(200, chargeBody, headerMap);
+    Map<String, List<String>> headerMap =
+        ImmutableMap.of("Request-Id", ImmutableList.of("req_12345"));
+    final StripeResponse stripeResponse = new StripeResponse(200, HttpHeaders.of(headerMap), "");
     assertEquals("req_12345", stripeResponse.requestId());
   }
 }