Skip to content

Commit be25759

Browse files
vitalijr2vitalijr2
authored
Retryer: replace an instance of Date with an epoch millisecond (#2170)
* Retryer: replace an instance of Date with an epoch millisecond * Style issue: unnecessary explicit casting * Add another check to RetryableException's test * Update serialization ID. Resolve some deprecation issues of Integer. * Remove obsolete Date * Remove obsolete Date 2 * Resolve issue with overrided method of a mock class
1 parent c7ca6e0 commit be25759

File tree

24 files changed

+415
-224
lines changed

24 files changed

+415
-224
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,12 +271,13 @@ private static FeignServerException serverErrorStatus(int status,
271271
}
272272

273273
static FeignException errorExecuting(Request request, IOException cause) {
274+
final Long nonRetryable = null;
274275
return new RetryableException(
275276
-1,
276277
format("%s executing %s %s", cause.getMessage(), request.httpMethod(), request.url()),
277278
request.httpMethod(),
278279
cause,
279-
null, request);
280+
nonRetryable, request);
280281
}
281282

282283
public static class FeignClientException extends FeignException {

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

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,22 @@
2424
*/
2525
public class RetryableException extends FeignException {
2626

27-
private static final long serialVersionUID = 1L;
27+
private static final long serialVersionUID = 2L;
2828

2929
private final Long retryAfter;
3030
private final HttpMethod httpMethod;
3131

3232
/**
3333
* @param retryAfter usually corresponds to the {@link feign.Util#RETRY_AFTER} header.
3434
*/
35+
public RetryableException(int status, String message, HttpMethod httpMethod, Throwable cause,
36+
Long retryAfter, Request request) {
37+
super(status, message, request, cause);
38+
this.httpMethod = httpMethod;
39+
this.retryAfter = retryAfter;
40+
}
41+
42+
@Deprecated
3543
public RetryableException(int status, String message, HttpMethod httpMethod, Throwable cause,
3644
Date retryAfter, Request request) {
3745
super(status, message, request, cause);
@@ -42,6 +50,14 @@ public RetryableException(int status, String message, HttpMethod httpMethod, Thr
4250
/**
4351
* @param retryAfter usually corresponds to the {@link feign.Util#RETRY_AFTER} header.
4452
*/
53+
public RetryableException(int status, String message, HttpMethod httpMethod, Long retryAfter,
54+
Request request) {
55+
super(status, message, request);
56+
this.httpMethod = httpMethod;
57+
this.retryAfter = retryAfter;
58+
}
59+
60+
@Deprecated
4561
public RetryableException(int status, String message, HttpMethod httpMethod, Date retryAfter,
4662
Request request) {
4763
super(status, message, request);
@@ -52,6 +68,15 @@ public RetryableException(int status, String message, HttpMethod httpMethod, Dat
5268
/**
5369
* @param retryAfter usually corresponds to the {@link feign.Util#RETRY_AFTER} header.
5470
*/
71+
public RetryableException(int status, String message, HttpMethod httpMethod, Long retryAfter,
72+
Request request, byte[] responseBody,
73+
Map<String, Collection<String>> responseHeaders) {
74+
super(status, message, request, responseBody, responseHeaders);
75+
this.httpMethod = httpMethod;
76+
this.retryAfter = retryAfter;
77+
}
78+
79+
@Deprecated
5580
public RetryableException(int status, String message, HttpMethod httpMethod, Date retryAfter,
5681
Request request, byte[] responseBody, Map<String, Collection<String>> responseHeaders) {
5782
super(status, message, request, responseBody, responseHeaders);
@@ -63,8 +88,8 @@ public RetryableException(int status, String message, HttpMethod httpMethod, Dat
6388
* Sometimes corresponds to the {@link feign.Util#RETRY_AFTER} header present in {@code 503}
6489
* status. Other times parsed from an application-specific response. Null if unknown.
6590
*/
66-
public Date retryAfter() {
67-
return retryAfter != null ? new Date(retryAfter) : null;
91+
public Long retryAfter() {
92+
return retryAfter;
6893
}
6994

7095
public HttpMethod method() {

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
public interface Retryer extends Cloneable {
2323

2424
/**
25-
* if retry is permitted, return (possibly after sleeping). Otherwise propagate the exception.
25+
* if retry is permitted, return (possibly after sleeping). Otherwise, propagate the exception.
2626
*/
2727
void continueOrPropagate(RetryableException e);
2828

@@ -59,7 +59,7 @@ public void continueOrPropagate(RetryableException e) {
5959

6060
long interval;
6161
if (e.retryAfter() != null) {
62-
interval = e.retryAfter().getTime() - currentTimeMillis();
62+
interval = e.retryAfter() - currentTimeMillis();
6363
if (interval > maxPeriod) {
6464
interval = maxPeriod;
6565
}
@@ -79,15 +79,15 @@ public void continueOrPropagate(RetryableException e) {
7979
}
8080

8181
/**
82-
* Calculates the time interval to a retry attempt. <br>
82+
* Calculates the time interval to a retry attempt.<br>
8383
* The interval increases exponentially with each attempt, at a rate of nextInterval *= 1.5
8484
* (where 1.5 is the backoff factor), to the maximum interval.
8585
*
8686
* @return time in milliseconds from now until the next attempt.
8787
*/
8888
long nextMaxInterval() {
8989
long interval = (long) (period * Math.pow(1.5, attempt - 1));
90-
return interval > maxPeriod ? maxPeriod : interval;
90+
return Math.min(interval, maxPeriod);
9191
}
9292

9393
@Override

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

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,15 @@
1616
import static feign.FeignException.errorStatus;
1717
import static feign.Util.RETRY_AFTER;
1818
import static feign.Util.checkNotNull;
19-
import static java.util.Locale.US;
19+
import static java.time.format.DateTimeFormatter.RFC_1123_DATE_TIME;
2020
import static java.util.concurrent.TimeUnit.SECONDS;
2121
import feign.FeignException;
2222
import feign.Response;
2323
import feign.RetryableException;
24-
import java.text.DateFormat;
25-
import java.text.ParseException;
26-
import java.text.SimpleDateFormat;
24+
import java.time.ZonedDateTime;
25+
import java.time.format.DateTimeFormatter;
26+
import java.time.format.DateTimeParseException;
2727
import java.util.Collection;
28-
import java.util.Date;
2928
import java.util.Map;
3029

3130
/**
@@ -103,7 +102,7 @@ public Default(Integer maxBodyBytesLength, Integer maxBodyCharsLength) {
103102
public Exception decode(String methodKey, Response response) {
104103
FeignException exception = errorStatus(methodKey, response, maxBodyBytesLength,
105104
maxBodyCharsLength);
106-
Date retryAfter = retryAfterDecoder.apply(firstOrNull(response.headers(), RETRY_AFTER));
105+
Long retryAfter = retryAfterDecoder.apply(firstOrNull(response.headers(), RETRY_AFTER));
107106
if (retryAfter != null) {
108107
return new RetryableException(
109108
response.status(),
@@ -125,48 +124,44 @@ private <T> T firstOrNull(Map<String, Collection<T>> map, String key) {
125124
}
126125

127126
/**
128-
* Decodes a {@link feign.Util#RETRY_AFTER} header into an absolute date, if possible. <br>
127+
* Decodes a {@link feign.Util#RETRY_AFTER} header into an epoch millisecond, if possible.<br>
129128
* See <a href="https://tools.ietf.org/html/rfc2616#section-14.37">Retry-After format</a>
130129
*/
131130
static class RetryAfterDecoder {
132131

133-
static final DateFormat RFC822_FORMAT =
134-
new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss 'GMT'", US);
135-
private final DateFormat rfc822Format;
132+
private final DateTimeFormatter dateTimeFormatter;
136133

137134
RetryAfterDecoder() {
138-
this(RFC822_FORMAT);
135+
this(RFC_1123_DATE_TIME);
139136
}
140137

141-
RetryAfterDecoder(DateFormat rfc822Format) {
142-
this.rfc822Format = checkNotNull(rfc822Format, "rfc822Format");
138+
RetryAfterDecoder(DateTimeFormatter dateTimeFormatter) {
139+
this.dateTimeFormatter = checkNotNull(dateTimeFormatter, "dateTimeFormatter");
143140
}
144141

145142
protected long currentTimeMillis() {
146143
return System.currentTimeMillis();
147144
}
148145

149146
/**
150-
* returns a date that corresponds to the first time a request can be retried.
147+
* returns an epoch millisecond that corresponds to the first time a request can be retried.
151148
*
152149
* @param retryAfter String in
153150
* <a href="https://tools.ietf.org/html/rfc2616#section-14.37" >Retry-After format</a>
154151
*/
155-
public Date apply(String retryAfter) {
152+
public Long apply(String retryAfter) {
156153
if (retryAfter == null) {
157154
return null;
158155
}
159156
if (retryAfter.matches("^[0-9]+\\.?0*$")) {
160157
retryAfter = retryAfter.replaceAll("\\.0*$", "");
161158
long deltaMillis = SECONDS.toMillis(Long.parseLong(retryAfter));
162-
return new Date(currentTimeMillis() + deltaMillis);
159+
return currentTimeMillis() + deltaMillis;
163160
}
164-
synchronized (rfc822Format) {
165-
try {
166-
return rfc822Format.parse(retryAfter);
167-
} catch (ParseException ignored) {
168-
return null;
169-
}
161+
try {
162+
return ZonedDateTime.parse(retryAfter, dateTimeFormatter).toInstant().toEpochMilli();
163+
} catch (NullPointerException | DateTimeParseException ignored) {
164+
return null;
170165
}
171166
}
172167
}

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

Lines changed: 51 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import static feign.ExceptionPropagationPolicy.UNWRAP;
1717
import static feign.Util.UTF_8;
1818
import static feign.assertj.MockWebServerAssertions.assertThat;
19-
import static org.assertj.core.api.Assertions.assertThat;
2019
import static org.assertj.core.data.MapEntry.entry;
2120
import static org.hamcrest.CoreMatchers.isA;
2221
import static org.junit.Assert.assertEquals;
@@ -37,11 +36,13 @@
3736
import java.io.IOException;
3837
import java.lang.reflect.Type;
3938
import java.net.URI;
39+
import java.time.Clock;
40+
import java.time.Instant;
41+
import java.time.ZoneId;
4042
import java.util.ArrayList;
4143
import java.util.Arrays;
4244
import java.util.Collection;
4345
import java.util.Collections;
44-
import java.util.Date;
4546
import java.util.HashMap;
4647
import java.util.LinkedHashMap;
4748
import java.util.List;
@@ -62,12 +63,12 @@
6263
import org.junit.Ignore;
6364
import org.junit.Rule;
6465
import org.junit.Test;
65-
import org.junit.jupiter.params.ParameterizedTest;
6666
import org.junit.jupiter.params.provider.ValueSource;
6767
import org.junit.rules.ExpectedException;
6868

6969
public class AsyncFeignTest {
7070

71+
private static final Long NON_RETRYABLE = null;
7172
@Rule
7273
public final ExpectedException thrown = ExpectedException.none();
7374
@Rule
@@ -226,9 +227,9 @@ public void customExpander() throws Exception {
226227
TestInterfaceAsync api =
227228
new TestInterfaceAsyncBuilder().target("http://localhost:" + server.getPort());
228229

229-
CompletableFuture<?> cf = api.expand(new Date(1234l));
230+
CompletableFuture<?> cf = api.expand(new TestClock(1234l) {});
230231

231-
assertThat(server.takeRequest()).hasPath("/?date=1234");
232+
assertThat(server.takeRequest()).hasPath("/?clock=1234");
232233

233234
checkCFCompletedSoon(cf);
234235
}
@@ -240,9 +241,10 @@ public void customExpanderListParam() throws Exception {
240241
TestInterfaceAsync api =
241242
new TestInterfaceAsyncBuilder().target("http://localhost:" + server.getPort());
242243

243-
CompletableFuture<?> cf = api.expandList(Arrays.asList(new Date(1234l), new Date(12345l)));
244+
CompletableFuture<?> cf =
245+
api.expandList(Arrays.asList(new TestClock(1234l), new TestClock(12345l)));
244246

245-
assertThat(server.takeRequest()).hasPath("/?date=1234&date=12345");
247+
assertThat(server.takeRequest()).hasPath("/?clock=1234&clock=12345");
246248

247249
checkCFCompletedSoon(cf);
248250
}
@@ -254,9 +256,9 @@ public void customExpanderNullParam() throws Exception {
254256
TestInterfaceAsync api =
255257
new TestInterfaceAsyncBuilder().target("http://localhost:" + server.getPort());
256258

257-
CompletableFuture<?> cf = api.expandList(Arrays.asList(new Date(1234l), null));
259+
CompletableFuture<?> cf = api.expandList(Arrays.asList(new TestClock(1234l), null));
258260

259-
assertThat(server.takeRequest()).hasPath("/?date=1234");
261+
assertThat(server.takeRequest()).hasPath("/?clock=1234");
260262

261263
checkCFCompletedSoon(cf);
262264
}
@@ -505,8 +507,8 @@ public void retryableExceptionInDecoder() throws Exception {
505507
public Object decode(Response response, Type type) throws IOException {
506508
String string = super.decode(response, type).toString();
507509
if ("retry!".equals(string)) {
508-
throw new RetryableException(response.status(), string, HttpMethod.POST, null,
509-
response.request());
510+
throw new RetryableException(response.status(), string, HttpMethod.POST,
511+
NON_RETRYABLE, response.request());
510512
}
511513
return string;
512514
}
@@ -584,7 +586,7 @@ public void ensureRetryerClonesItself() throws Throwable {
584586
@Override
585587
public Exception decode(String methodKey, Response response) {
586588
return new RetryableException(response.status(), "play it again sam!", HttpMethod.POST,
587-
null, response.request());
589+
NON_RETRYABLE, response.request());
588590
}
589591
}).target(TestInterfaceAsync.class, "http://localhost:" + server.getPort());
590592

@@ -609,7 +611,7 @@ public void throwsOriginalExceptionAfterFailedRetries() throws Throwable {
609611
@Override
610612
public Exception decode(String methodKey, Response response) {
611613
return new RetryableException(response.status(), "play it again sam!", HttpMethod.POST,
612-
new TestInterfaceException(message), null, response.request());
614+
new TestInterfaceException(message), NON_RETRYABLE, response.request());
613615
}
614616
}).target(TestInterfaceAsync.class, "http://localhost:" + server.getPort());
615617

@@ -631,8 +633,8 @@ public void throwsRetryableExceptionIfNoUnderlyingCause() throws Throwable {
631633
.errorDecoder(new ErrorDecoder() {
632634
@Override
633635
public Exception decode(String methodKey, Response response) {
634-
return new RetryableException(response.status(), message, HttpMethod.POST, null,
635-
response.request());
636+
return new RetryableException(response.status(), message, HttpMethod.POST,
637+
NON_RETRYABLE, response.request());
636638
}
637639
}).target(TestInterfaceAsync.class, "http://localhost:" + server.getPort());
638640

@@ -1022,16 +1024,17 @@ CompletableFuture<Response> uriParam(@Param("1") String one,
10221024
CompletableFuture<Response> queryParams(@Param("1") String one,
10231025
@Param("2") Iterable<String> twos);
10241026

1025-
@RequestLine("POST /?date={date}")
1026-
CompletableFuture<Void> expand(@Param(value = "date", expander = DateToMillis.class) Date date);
1027+
@RequestLine("POST /?clock={clock}")
1028+
CompletableFuture<Void> expand(@Param(value = "clock",
1029+
expander = ClockToMillis.class) Clock clock);
10271030

1028-
@RequestLine("GET /?date={date}")
1029-
CompletableFuture<Void> expandList(@Param(value = "date",
1030-
expander = DateToMillis.class) List<Date> dates);
1031+
@RequestLine("GET /?clock={clock}")
1032+
CompletableFuture<Void> expandList(@Param(value = "clock",
1033+
expander = ClockToMillis.class) List<Clock> clocks);
10311034

1032-
@RequestLine("GET /?date={date}")
1033-
CompletableFuture<Void> expandArray(@Param(value = "date",
1034-
expander = DateToMillis.class) Date[] dates);
1035+
@RequestLine("GET /?clock={clock}")
1036+
CompletableFuture<Void> expandArray(@Param(value = "clock",
1037+
expander = ClockToMillis.class) Clock[] clocks);
10351038

10361039
@RequestLine("GET /")
10371040
CompletableFuture<Void> headerMap(@HeaderMap Map<String, Object> headerMap);
@@ -1062,11 +1065,11 @@ CompletableFuture<Void> queryMapWithQueryParams(@Param("name") String name,
10621065
@RequestLine("GET /")
10631066
CompletableFuture<Void> queryMapPropertyInheritence(@QueryMap ChildPojo object);
10641067

1065-
class DateToMillis implements Param.Expander {
1068+
class ClockToMillis implements Param.Expander {
10661069

10671070
@Override
10681071
public String expand(Object value) {
1069-
return String.valueOf(((Date) value).getTime());
1072+
return String.valueOf(((Clock) value).millis());
10701073
}
10711074
}
10721075
}
@@ -1249,5 +1252,28 @@ static interface WildApi {
12491252
CompletableFuture<?> x();
12501253
}
12511254

1255+
class TestClock extends Clock {
1256+
1257+
private long millis;
1258+
1259+
public TestClock(long millis) {
1260+
this.millis = millis;
1261+
}
1262+
1263+
@Override
1264+
public ZoneId getZone() {
1265+
throw new UnsupportedOperationException("This operation is not supported.");
1266+
}
1267+
1268+
@Override
1269+
public Clock withZone(ZoneId zone) {
1270+
return this;
1271+
}
1272+
1273+
@Override
1274+
public Instant instant() {
1275+
return Instant.ofEpochMilli(millis);
1276+
}
1277+
}
12521278

12531279
}

0 commit comments

Comments
 (0)