Skip to content

Commit d123646

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 f95cc53 commit d123646

File tree

24 files changed

+427
-215
lines changed

24 files changed

+427
-215
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,12 +289,13 @@ private static FeignServerException serverErrorStatus(
289289
}
290290

291291
static FeignException errorExecuting(Request request, IOException cause) {
292+
final Long nonRetryable = null;
292293
return new RetryableException(
293294
-1,
294295
format("%s executing %s %s", cause.getMessage(), request.httpMethod(), request.url()),
295296
request.httpMethod(),
296297
cause,
297-
null,
298+
nonRetryable,
298299
request);
299300
}
300301

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

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,27 @@
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(
36+
int status,
37+
String message,
38+
HttpMethod httpMethod,
39+
Throwable cause,
40+
Long retryAfter,
41+
Request request) {
42+
super(status, message, request, cause);
43+
this.httpMethod = httpMethod;
44+
this.retryAfter = retryAfter;
45+
}
46+
47+
@Deprecated
3548
public RetryableException(
3649
int status,
3750
String message,
@@ -47,6 +60,14 @@ public RetryableException(
4760
/**
4861
* @param retryAfter usually corresponds to the {@link feign.Util#RETRY_AFTER} header.
4962
*/
63+
public RetryableException(
64+
int status, String message, HttpMethod httpMethod, Long retryAfter, Request request) {
65+
super(status, message, request);
66+
this.httpMethod = httpMethod;
67+
this.retryAfter = retryAfter;
68+
}
69+
70+
@Deprecated
5071
public RetryableException(
5172
int status, String message, HttpMethod httpMethod, Date retryAfter, Request request) {
5273
super(status, message, request);
@@ -57,6 +78,20 @@ public RetryableException(
5778
/**
5879
* @param retryAfter usually corresponds to the {@link feign.Util#RETRY_AFTER} header.
5980
*/
81+
public RetryableException(
82+
int status,
83+
String message,
84+
HttpMethod httpMethod,
85+
Long retryAfter,
86+
Request request,
87+
byte[] responseBody,
88+
Map<String, Collection<String>> responseHeaders) {
89+
super(status, message, request, responseBody, responseHeaders);
90+
this.httpMethod = httpMethod;
91+
this.retryAfter = retryAfter;
92+
}
93+
94+
@Deprecated
6095
public RetryableException(
6196
int status,
6297
String message,
@@ -74,8 +109,8 @@ public RetryableException(
74109
* Sometimes corresponds to the {@link feign.Util#RETRY_AFTER} header present in {@code 503}
75110
* status. Other times parsed from an application-specific response. Null if unknown.
76111
*/
77-
public Date retryAfter() {
78-
return retryAfter != null ? new Date(retryAfter) : null;
112+
public Long retryAfter() {
113+
return retryAfter;
79114
}
80115

81116
public HttpMethod method() {

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@
2121
*/
2222
public interface Retryer extends Cloneable {
2323

24-
/** if retry is permitted, return (possibly after sleeping). Otherwise propagate the exception. */
24+
/**
25+
* if retry is permitted, return (possibly after sleeping). Otherwise, propagate the exception.
26+
*/
2527
void continueOrPropagate(RetryableException e);
2628

2729
Retryer clone();
@@ -57,7 +59,7 @@ public void continueOrPropagate(RetryableException e) {
5759

5860
long interval;
5961
if (e.retryAfter() != null) {
60-
interval = e.retryAfter().getTime() - currentTimeMillis();
62+
interval = e.retryAfter() - currentTimeMillis();
6163
if (interval > maxPeriod) {
6264
interval = maxPeriod;
6365
}
@@ -77,15 +79,15 @@ public void continueOrPropagate(RetryableException e) {
7779
}
7880

7981
/**
80-
* Calculates the time interval to a retry attempt. <br>
82+
* Calculates the time interval to a retry attempt.<br>
8183
* The interval increases exponentially with each attempt, at a rate of nextInterval *= 1.5
8284
* (where 1.5 is the backoff factor), to the maximum interval.
8385
*
8486
* @return time in milliseconds from now until the next attempt.
8587
*/
8688
long nextMaxInterval() {
8789
long interval = (long) (period * Math.pow(1.5, attempt - 1));
88-
return interval > maxPeriod ? maxPeriod : interval;
90+
return Math.min(interval, maxPeriod);
8991
}
9092

9193
@Override

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

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,16 @@
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

2222
import feign.FeignException;
2323
import feign.Response;
2424
import feign.RetryableException;
25-
import java.text.DateFormat;
26-
import java.text.ParseException;
27-
import java.text.SimpleDateFormat;
25+
import java.time.ZonedDateTime;
26+
import java.time.format.DateTimeFormatter;
27+
import java.time.format.DateTimeParseException;
2828
import java.util.Collection;
29-
import java.util.Date;
3029
import java.util.Map;
3130

3231
/**
@@ -100,7 +99,7 @@ public Default(Integer maxBodyBytesLength, Integer maxBodyCharsLength) {
10099
public Exception decode(String methodKey, Response response) {
101100
FeignException exception =
102101
errorStatus(methodKey, response, maxBodyBytesLength, maxBodyCharsLength);
103-
Date retryAfter = retryAfterDecoder.apply(firstOrNull(response.headers(), RETRY_AFTER));
102+
Long retryAfter = retryAfterDecoder.apply(firstOrNull(response.headers(), RETRY_AFTER));
104103
if (retryAfter != null) {
105104
return new RetryableException(
106105
response.status(),
@@ -122,48 +121,44 @@ private <T> T firstOrNull(Map<String, Collection<T>> map, String key) {
122121
}
123122

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

130-
static final DateFormat RFC822_FORMAT =
131-
new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss 'GMT'", US);
132-
private final DateFormat rfc822Format;
129+
private final DateTimeFormatter dateTimeFormatter;
133130

134131
RetryAfterDecoder() {
135-
this(RFC822_FORMAT);
132+
this(RFC_1123_DATE_TIME);
136133
}
137134

138-
RetryAfterDecoder(DateFormat rfc822Format) {
139-
this.rfc822Format = checkNotNull(rfc822Format, "rfc822Format");
135+
RetryAfterDecoder(DateTimeFormatter dateTimeFormatter) {
136+
this.dateTimeFormatter = checkNotNull(dateTimeFormatter, "dateTimeFormatter");
140137
}
141138

142139
protected long currentTimeMillis() {
143140
return System.currentTimeMillis();
144141
}
145142

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

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

Lines changed: 56 additions & 20 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;
@@ -38,11 +37,13 @@
3837
import java.io.IOException;
3938
import java.lang.reflect.Type;
4039
import java.net.URI;
40+
import java.time.Clock;
41+
import java.time.Instant;
42+
import java.time.ZoneId;
4143
import java.util.ArrayList;
4244
import java.util.Arrays;
4345
import java.util.Collection;
4446
import java.util.Collections;
45-
import java.util.Date;
4647
import java.util.HashMap;
4748
import java.util.LinkedHashMap;
4849
import java.util.List;
@@ -68,6 +69,7 @@
6869

6970
public class AsyncFeignTest {
7071

72+
private static final Long NON_RETRYABLE = null;
7173
@Rule public final ExpectedException thrown = ExpectedException.none();
7274
@Rule public final MockWebServer server = new MockWebServer();
7375

@@ -235,9 +237,9 @@ public void customExpander() throws Exception {
235237
TestInterfaceAsync api =
236238
new TestInterfaceAsyncBuilder().target("http://localhost:" + server.getPort());
237239

238-
CompletableFuture<?> cf = api.expand(new Date(1234l));
240+
CompletableFuture<?> cf = api.expand(new TestClock(1234l) {});
239241

240-
assertThat(server.takeRequest()).hasPath("/?date=1234");
242+
assertThat(server.takeRequest()).hasPath("/?clock=1234");
241243

242244
checkCFCompletedSoon(cf);
243245
}
@@ -249,9 +251,10 @@ public void customExpanderListParam() throws Exception {
249251
TestInterfaceAsync api =
250252
new TestInterfaceAsyncBuilder().target("http://localhost:" + server.getPort());
251253

252-
CompletableFuture<?> cf = api.expandList(Arrays.asList(new Date(1234l), new Date(12345l)));
254+
CompletableFuture<?> cf =
255+
api.expandList(Arrays.asList(new TestClock(1234l), new TestClock(12345l)));
253256

254-
assertThat(server.takeRequest()).hasPath("/?date=1234&date=12345");
257+
assertThat(server.takeRequest()).hasPath("/?clock=1234&clock=12345");
255258

256259
checkCFCompletedSoon(cf);
257260
}
@@ -263,9 +266,9 @@ public void customExpanderNullParam() throws Exception {
263266
TestInterfaceAsync api =
264267
new TestInterfaceAsyncBuilder().target("http://localhost:" + server.getPort());
265268

266-
CompletableFuture<?> cf = api.expandList(Arrays.asList(new Date(1234l), null));
269+
CompletableFuture<?> cf = api.expandList(Arrays.asList(new TestClock(1234l), null));
267270

268-
assertThat(server.takeRequest()).hasPath("/?date=1234");
271+
assertThat(server.takeRequest()).hasPath("/?clock=1234");
269272

270273
checkCFCompletedSoon(cf);
271274
}
@@ -527,7 +530,11 @@ public Object decode(Response response, Type type) throws IOException {
527530
String string = super.decode(response, type).toString();
528531
if ("retry!".equals(string)) {
529532
throw new RetryableException(
530-
response.status(), string, HttpMethod.POST, null, response.request());
533+
response.status(),
534+
string,
535+
HttpMethod.POST,
536+
NON_RETRYABLE,
537+
response.request());
531538
}
532539
return string;
533540
}
@@ -623,7 +630,7 @@ public Exception decode(String methodKey, Response response) {
623630
response.status(),
624631
"play it again sam!",
625632
HttpMethod.POST,
626-
null,
633+
NON_RETRYABLE,
627634
response.request());
628635
}
629636
})
@@ -656,7 +663,7 @@ public Exception decode(String methodKey, Response response) {
656663
"play it again sam!",
657664
HttpMethod.POST,
658665
new TestInterfaceException(message),
659-
null,
666+
NON_RETRYABLE,
660667
response.request());
661668
}
662669
})
@@ -683,7 +690,11 @@ public void throwsRetryableExceptionIfNoUnderlyingCause() throws Throwable {
683690
@Override
684691
public Exception decode(String methodKey, Response response) {
685692
return new RetryableException(
686-
response.status(), message, HttpMethod.POST, null, response.request());
693+
response.status(),
694+
message,
695+
HttpMethod.POST,
696+
NON_RETRYABLE,
697+
response.request());
687698
}
688699
})
689700
.target(TestInterfaceAsync.class, "http://localhost:" + server.getPort());
@@ -1117,16 +1128,17 @@ CompletableFuture<Response> uriParam(
11171128
CompletableFuture<Response> queryParams(
11181129
@Param("1") String one, @Param("2") Iterable<String> twos);
11191130

1120-
@RequestLine("POST /?date={date}")
1121-
CompletableFuture<Void> expand(@Param(value = "date", expander = DateToMillis.class) Date date);
1131+
@RequestLine("POST /?clock={clock}")
1132+
CompletableFuture<Void> expand(
1133+
@Param(value = "clock", expander = ClockToMillis.class) Clock clock);
11221134

1123-
@RequestLine("GET /?date={date}")
1135+
@RequestLine("GET /?clock={clock}")
11241136
CompletableFuture<Void> expandList(
1125-
@Param(value = "date", expander = DateToMillis.class) List<Date> dates);
1137+
@Param(value = "clock", expander = ClockToMillis.class) List<Clock> clocks);
11261138

1127-
@RequestLine("GET /?date={date}")
1139+
@RequestLine("GET /?clock={clock}")
11281140
CompletableFuture<Void> expandArray(
1129-
@Param(value = "date", expander = DateToMillis.class) Date[] dates);
1141+
@Param(value = "clock", expander = ClockToMillis.class) Clock[] clocks);
11301142

11311143
@RequestLine("GET /")
11321144
CompletableFuture<Void> headerMap(@HeaderMap Map<String, Object> headerMap);
@@ -1158,11 +1170,11 @@ CompletableFuture<Void> queryMapWithQueryParams(
11581170
@RequestLine("GET /")
11591171
CompletableFuture<Void> queryMapPropertyInheritence(@QueryMap ChildPojo object);
11601172

1161-
class DateToMillis implements Param.Expander {
1173+
class ClockToMillis implements Param.Expander {
11621174

11631175
@Override
11641176
public String expand(Object value) {
1165-
return String.valueOf(((Date) value).getTime());
1177+
return String.valueOf(((Clock) value).millis());
11661178
}
11671179
}
11681180
}
@@ -1344,4 +1356,28 @@ static interface WildApi {
13441356
@RequestLine("GET /")
13451357
CompletableFuture<?> x();
13461358
}
1359+
1360+
class TestClock extends Clock {
1361+
1362+
private long millis;
1363+
1364+
public TestClock(long millis) {
1365+
this.millis = millis;
1366+
}
1367+
1368+
@Override
1369+
public ZoneId getZone() {
1370+
throw new UnsupportedOperationException("This operation is not supported.");
1371+
}
1372+
1373+
@Override
1374+
public Clock withZone(ZoneId zone) {
1375+
return this;
1376+
}
1377+
1378+
@Override
1379+
public Instant instant() {
1380+
return Instant.ofEpochMilli(millis);
1381+
}
1382+
}
13471383
}

0 commit comments

Comments
 (0)