Retryer: replace an instance of Date with an epoch millisecond

core/src/main/java/feign/FeignException.java

@@ -271,12 +271,13 @@ private static FeignServerException serverErrorStatus(int status,
   }
 
   static FeignException errorExecuting(Request request, IOException cause) {
+    final Long nonRetryable = null;
     return new RetryableException(
         -1,
         format("%s executing %s %s", cause.getMessage(), request.httpMethod(), request.url()),
         request.httpMethod(),
         cause,
-        null, request);
+        nonRetryable, request);
   }
 
   public static class FeignClientException extends FeignException {

core/src/main/java/feign/RetryableException.java

@@ -24,14 +24,22 @@
  */
 public class RetryableException extends FeignException {
 
-  private static final long serialVersionUID = 1L;
+  private static final long serialVersionUID = 2L;
 
   private final Long retryAfter;
   private final HttpMethod httpMethod;
 
   /**
    * @param retryAfter usually corresponds to the {@link feign.Util#RETRY_AFTER} header.
    */
+  public RetryableException(int status, String message, HttpMethod httpMethod, Throwable cause,
+      Long retryAfter, Request request) {
+    super(status, message, request, cause);
+    this.httpMethod = httpMethod;
+    this.retryAfter = retryAfter;
+  }
+
+  @Deprecated
   public RetryableException(int status, String message, HttpMethod httpMethod, Throwable cause,
       Date retryAfter, Request request) {
     super(status, message, request, cause);
@@ -42,6 +50,14 @@ public RetryableException(int status, String message, HttpMethod httpMethod, Thr
   /**
    * @param retryAfter usually corresponds to the {@link feign.Util#RETRY_AFTER} header.
    */
+  public RetryableException(int status, String message, HttpMethod httpMethod, Long retryAfter,
+      Request request) {
+    super(status, message, request);
+    this.httpMethod = httpMethod;
+    this.retryAfter = retryAfter;
+  }
+
+  @Deprecated
   public RetryableException(int status, String message, HttpMethod httpMethod, Date retryAfter,
       Request request) {
     super(status, message, request);
@@ -52,6 +68,15 @@ public RetryableException(int status, String message, HttpMethod httpMethod, Dat
   /**
    * @param retryAfter usually corresponds to the {@link feign.Util#RETRY_AFTER} header.
    */
+  public RetryableException(int status, String message, HttpMethod httpMethod, Long retryAfter,
+      Request request, byte[] responseBody,
+      Map<String, Collection<String>> responseHeaders) {
+    super(status, message, request, responseBody, responseHeaders);
+    this.httpMethod = httpMethod;
+    this.retryAfter = retryAfter;
+  }
+
+  @Deprecated
   public RetryableException(int status, String message, HttpMethod httpMethod, Date retryAfter,
       Request request, byte[] responseBody, Map<String, Collection<String>> responseHeaders) {
     super(status, message, request, responseBody, responseHeaders);
@@ -63,8 +88,8 @@ public RetryableException(int status, String message, HttpMethod httpMethod, Dat
    * Sometimes corresponds to the {@link feign.Util#RETRY_AFTER} header present in {@code 503}
    * status. Other times parsed from an application-specific response. Null if unknown.
    */
-  public Date retryAfter() {
-    return retryAfter != null ? new Date(retryAfter) : null;
+  public Long retryAfter() {
+    return retryAfter;
   }
 
   public HttpMethod method() {

core/src/main/java/feign/Retryer.java

@@ -22,7 +22,7 @@
 public interface Retryer extends Cloneable {
 
   /**
-   * if retry is permitted, return (possibly after sleeping). Otherwise propagate the exception.
+   * if retry is permitted, return (possibly after sleeping). Otherwise, propagate the exception.
    */
   void continueOrPropagate(RetryableException e);
 
@@ -59,7 +59,7 @@ public void continueOrPropagate(RetryableException e) {
 
       long interval;
       if (e.retryAfter() != null) {
-        interval = e.retryAfter().getTime() - currentTimeMillis();
+        interval = e.retryAfter() - currentTimeMillis();
         if (interval > maxPeriod) {
           interval = maxPeriod;
         }
@@ -79,15 +79,15 @@ public void continueOrPropagate(RetryableException e) {
     }
 
     /**
-     * Calculates the time interval to a retry attempt. <br>
+     * Calculates the time interval to a retry attempt.<br>
      * The interval increases exponentially with each attempt, at a rate of nextInterval *= 1.5
      * (where 1.5 is the backoff factor), to the maximum interval.
      *
      * @return time in milliseconds from now until the next attempt.
      */
     long nextMaxInterval() {
       long interval = (long) (period * Math.pow(1.5, attempt - 1));
-      return interval > maxPeriod ? maxPeriod : interval;
+      return Math.min(interval, maxPeriod);
     }
 
     @Override

core/src/main/java/feign/codec/ErrorDecoder.java

@@ -16,16 +16,15 @@
 import static feign.FeignException.errorStatus;
 import static feign.Util.RETRY_AFTER;
 import static feign.Util.checkNotNull;
-import static java.util.Locale.US;
+import static java.time.format.DateTimeFormatter.RFC_1123_DATE_TIME;
 import static java.util.concurrent.TimeUnit.SECONDS;
 import feign.FeignException;
 import feign.Response;
 import feign.RetryableException;
-import java.text.DateFormat;
-import java.text.ParseException;
-import java.text.SimpleDateFormat;
+import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
+import java.time.format.DateTimeParseException;
 import java.util.Collection;
-import java.util.Date;
 import java.util.Map;
 
 /**
@@ -103,7 +102,7 @@ public Default(Integer maxBodyBytesLength, Integer maxBodyCharsLength) {
     public Exception decode(String methodKey, Response response) {
       FeignException exception = errorStatus(methodKey, response, maxBodyBytesLength,
           maxBodyCharsLength);
-      Date retryAfter = retryAfterDecoder.apply(firstOrNull(response.headers(), RETRY_AFTER));
+      Long retryAfter = retryAfterDecoder.apply(firstOrNull(response.headers(), RETRY_AFTER));
       if (retryAfter != null) {
         return new RetryableException(
             response.status(),
@@ -125,48 +124,44 @@ private <T> T firstOrNull(Map<String, Collection<T>> map, String key) {
   }
 
   /**
-   * Decodes a {@link feign.Util#RETRY_AFTER} header into an absolute date, if possible. <br>
+   * Decodes a {@link feign.Util#RETRY_AFTER} header into an epoch millisecond, if possible.<br>
    * See <a href="https://tools.ietf.org/html/rfc2616#section-14.37">Retry-After format</a>
    */
   static class RetryAfterDecoder {
 
-    static final DateFormat RFC822_FORMAT =
-        new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss 'GMT'", US);
-    private final DateFormat rfc822Format;
+    private final DateTimeFormatter dateTimeFormatter;
 
     RetryAfterDecoder() {
-      this(RFC822_FORMAT);
+      this(RFC_1123_DATE_TIME);
     }
 
-    RetryAfterDecoder(DateFormat rfc822Format) {
-      this.rfc822Format = checkNotNull(rfc822Format, "rfc822Format");
+    RetryAfterDecoder(DateTimeFormatter dateTimeFormatter) {
+      this.dateTimeFormatter = checkNotNull(dateTimeFormatter, "dateTimeFormatter");
     }
 
     protected long currentTimeMillis() {
       return System.currentTimeMillis();
     }
 
     /**
-     * returns a date that corresponds to the first time a request can be retried.
+     * returns an epoch millisecond that corresponds to the first time a request can be retried.
      *
      * @param retryAfter String in
      *        <a href="https://tools.ietf.org/html/rfc2616#section-14.37" >Retry-After format</a>
      */
-    public Date apply(String retryAfter) {
+    public Long apply(String retryAfter) {
       if (retryAfter == null) {
         return null;
       }
       if (retryAfter.matches("^[0-9]+\\.?0*$")) {
         retryAfter = retryAfter.replaceAll("\\.0*$", "");
         long deltaMillis = SECONDS.toMillis(Long.parseLong(retryAfter));
-        return new Date(currentTimeMillis() + deltaMillis);
+        return currentTimeMillis() + deltaMillis;
       }
-      synchronized (rfc822Format) {
-        try {
-          return rfc822Format.parse(retryAfter);
-        } catch (ParseException ignored) {
-          return null;
-        }
+      try {
+        return ZonedDateTime.parse(retryAfter, dateTimeFormatter).toInstant().toEpochMilli();
+      } catch (NullPointerException | DateTimeParseException ignored) {
+        return null;
       }
     }
   }

core/src/test/java/feign/AsyncFeignTest.java

@@ -16,7 +16,6 @@
 import static feign.ExceptionPropagationPolicy.UNWRAP;
 import static feign.Util.UTF_8;
 import static feign.assertj.MockWebServerAssertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.data.MapEntry.entry;
 import static org.hamcrest.CoreMatchers.isA;
 import static org.junit.Assert.assertEquals;
@@ -37,11 +36,13 @@
 import java.io.IOException;
 import java.lang.reflect.Type;
 import java.net.URI;
+import java.time.Clock;
+import java.time.Instant;
+import java.time.ZoneId;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.Date;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -62,12 +63,12 @@
 import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
-import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
 import org.junit.rules.ExpectedException;
 
 public class AsyncFeignTest {
 
+  private static final Long NON_RETRYABLE = null;
   @Rule
   public final ExpectedException thrown = ExpectedException.none();
   @Rule
@@ -226,9 +227,9 @@ public void customExpander() throws Exception {
     TestInterfaceAsync api =
         new TestInterfaceAsyncBuilder().target("http://localhost:" + server.getPort());
 
-    CompletableFuture<?> cf = api.expand(new Date(1234l));
+    CompletableFuture<?> cf = api.expand(new TestClock(1234l) {});
 
-    assertThat(server.takeRequest()).hasPath("/?date=1234");
+    assertThat(server.takeRequest()).hasPath("/?clock=1234");
 
     checkCFCompletedSoon(cf);
   }
@@ -240,9 +241,10 @@ public void customExpanderListParam() throws Exception {
     TestInterfaceAsync api =
         new TestInterfaceAsyncBuilder().target("http://localhost:" + server.getPort());
 
-    CompletableFuture<?> cf = api.expandList(Arrays.asList(new Date(1234l), new Date(12345l)));
+    CompletableFuture<?> cf =
+        api.expandList(Arrays.asList(new TestClock(1234l), new TestClock(12345l)));
 
-    assertThat(server.takeRequest()).hasPath("/?date=1234&date=12345");
+    assertThat(server.takeRequest()).hasPath("/?clock=1234&clock=12345");
 
     checkCFCompletedSoon(cf);
   }
@@ -254,9 +256,9 @@ public void customExpanderNullParam() throws Exception {
     TestInterfaceAsync api =
         new TestInterfaceAsyncBuilder().target("http://localhost:" + server.getPort());
 
-    CompletableFuture<?> cf = api.expandList(Arrays.asList(new Date(1234l), null));
+    CompletableFuture<?> cf = api.expandList(Arrays.asList(new TestClock(1234l), null));
 
-    assertThat(server.takeRequest()).hasPath("/?date=1234");
+    assertThat(server.takeRequest()).hasPath("/?clock=1234");
 
     checkCFCompletedSoon(cf);
   }
@@ -505,8 +507,8 @@ public void retryableExceptionInDecoder() throws Exception {
           public Object decode(Response response, Type type) throws IOException {
             String string = super.decode(response, type).toString();
             if ("retry!".equals(string)) {
-              throw new RetryableException(response.status(), string, HttpMethod.POST, null,
-                  response.request());
+              throw new RetryableException(response.status(), string, HttpMethod.POST,
+                  NON_RETRYABLE, response.request());
             }
             return string;
           }
@@ -584,7 +586,7 @@ public void ensureRetryerClonesItself() throws Throwable {
           @Override
           public Exception decode(String methodKey, Response response) {
             return new RetryableException(response.status(), "play it again sam!", HttpMethod.POST,
-                null, response.request());
+                NON_RETRYABLE, response.request());
           }
         }).target(TestInterfaceAsync.class, "http://localhost:" + server.getPort());
 
@@ -609,7 +611,7 @@ public void throwsOriginalExceptionAfterFailedRetries() throws Throwable {
           @Override
           public Exception decode(String methodKey, Response response) {
             return new RetryableException(response.status(), "play it again sam!", HttpMethod.POST,
-                new TestInterfaceException(message), null, response.request());
+                new TestInterfaceException(message), NON_RETRYABLE, response.request());
           }
         }).target(TestInterfaceAsync.class, "http://localhost:" + server.getPort());
 
@@ -631,8 +633,8 @@ public void throwsRetryableExceptionIfNoUnderlyingCause() throws Throwable {
         .errorDecoder(new ErrorDecoder() {
           @Override
           public Exception decode(String methodKey, Response response) {
-            return new RetryableException(response.status(), message, HttpMethod.POST, null,
-                response.request());
+            return new RetryableException(response.status(), message, HttpMethod.POST,
+                NON_RETRYABLE, response.request());
           }
         }).target(TestInterfaceAsync.class, "http://localhost:" + server.getPort());
 
@@ -1022,16 +1024,17 @@ CompletableFuture<Response> uriParam(@Param("1") String one,
     CompletableFuture<Response> queryParams(@Param("1") String one,
                                             @Param("2") Iterable<String> twos);
 
-    @RequestLine("POST /?date={date}")
-    CompletableFuture<Void> expand(@Param(value = "date", expander = DateToMillis.class) Date date);
+    @RequestLine("POST /?clock={clock}")
+    CompletableFuture<Void> expand(@Param(value = "clock",
+        expander = ClockToMillis.class) Clock clock);
 
-    @RequestLine("GET /?date={date}")
-    CompletableFuture<Void> expandList(@Param(value = "date",
-        expander = DateToMillis.class) List<Date> dates);
+    @RequestLine("GET /?clock={clock}")
+    CompletableFuture<Void> expandList(@Param(value = "clock",
+        expander = ClockToMillis.class) List<Clock> clocks);
 
-    @RequestLine("GET /?date={date}")
-    CompletableFuture<Void> expandArray(@Param(value = "date",
-        expander = DateToMillis.class) Date[] dates);
+    @RequestLine("GET /?clock={clock}")
+    CompletableFuture<Void> expandArray(@Param(value = "clock",
+        expander = ClockToMillis.class) Clock[] clocks);
 
     @RequestLine("GET /")
     CompletableFuture<Void> headerMap(@HeaderMap Map<String, Object> headerMap);
@@ -1062,11 +1065,11 @@ CompletableFuture<Void> queryMapWithQueryParams(@Param("name") String name,
     @RequestLine("GET /")
     CompletableFuture<Void> queryMapPropertyInheritence(@QueryMap ChildPojo object);
 
-    class DateToMillis implements Param.Expander {
+    class ClockToMillis implements Param.Expander {
 
       @Override
       public String expand(Object value) {
-        return String.valueOf(((Date) value).getTime());
+        return String.valueOf(((Clock) value).millis());
       }
     }
   }
@@ -1249,5 +1252,28 @@ static interface WildApi {
     CompletableFuture<?> x();
   }
 
+  class TestClock extends Clock {
+
+    private long millis;
+
+    public TestClock(long millis) {
+      this.millis = millis;
+    }
+
+    @Override
+    public ZoneId getZone() {
+      throw new UnsupportedOperationException("This operation is not supported.");
+    }
+
+    @Override
+    public Clock withZone(ZoneId zone) {
+      return this;
+    }
+
+    @Override
+    public Instant instant() {
+      return Instant.ofEpochMilli(millis);
+    }
+  }
 
 }