Skip to content

Commit cc31bf3

Browse files
committed
Accept zero for RetryPolicy.Builder.delay()
This aligns the programmatic RetryPolicy configuration option with the delay support in @⁠Retryable. See gh-35110
1 parent c9078bf commit cc31bf3

File tree

5 files changed

+25
-25
lines changed

5 files changed

+25
-25
lines changed

spring-context/src/main/java/org/springframework/resilience/annotation/Retryable.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@
111111
* this serves as the initial delay to multiply from.
112112
* <p>The time unit is milliseconds by default but can be overridden via
113113
* {@link #timeUnit}.
114-
* <p>The default is 1000.
114+
* <p>Must be greater than or equal to zero. The default is 1000.
115115
* @see #jitter()
116116
* @see #multiplier()
117117
* @see #maxDelay()

spring-core/src/main/java/org/springframework/core/retry/RetryPolicy.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,14 +209,15 @@ public Builder maxAttempts(long maxAttempts) {
209209
* <p>You should not specify this configuration option if you have
210210
* configured a custom {@link #backOff(BackOff) BackOff} strategy.
211211
* @param delay the base delay, typically in milliseconds or seconds;
212-
* must be positive
212+
* must be greater than or equal to zero
213213
* @return this {@code Builder} instance for chained method invocations
214214
* @see #jitter(Duration)
215215
* @see #multiplier(double)
216216
* @see #maxDelay(Duration)
217217
*/
218218
public Builder delay(Duration delay) {
219-
assertIsPositive("delay", delay);
219+
Assert.isTrue(!delay.isNegative(),
220+
() -> "Invalid delay (%dms): must be >= 0.".formatted(delay.toMillis()));
220221
this.delay = delay;
221222
return this;
222223
}
@@ -233,7 +234,8 @@ public Builder delay(Duration delay) {
233234
* <p>The supplied value will override any previously configured value.
234235
* <p>You should not specify this configuration option if you have
235236
* configured a custom {@link #backOff(BackOff) BackOff} strategy.
236-
* @param jitter the jitter value, typically in milliseconds; must be positive
237+
* @param jitter the jitter value, typically in milliseconds; must be
238+
* greater than or equal to zero
237239
* @return this {@code Builder} instance for chained method invocations
238240
* @see #delay(Duration)
239241
* @see #multiplier(double)

spring-core/src/test/java/org/springframework/core/retry/MaxAttemptsRetryPolicyTests.java

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
import static org.assertj.core.api.Assertions.assertThat;
2727
import static org.mockito.Mockito.mock;
28+
import static org.springframework.core.retry.RetryPolicy.Builder.DEFAULT_DELAY;
2829
import static org.springframework.util.backoff.BackOffExecution.STOP;
2930

3031
/**
@@ -38,14 +39,14 @@ class MaxAttemptsRetryPolicyTests {
3839

3940
@Test
4041
void maxAttempts() {
41-
var retryPolicy = RetryPolicy.builder().maxAttempts(2).delay(Duration.ofMillis(1)).build();
42+
var retryPolicy = RetryPolicy.builder().maxAttempts(2).delay(Duration.ofMillis(0)).build();
4243
var backOffExecution = retryPolicy.getBackOff().start();
4344
var throwable = mock(Throwable.class);
4445

4546
assertThat(retryPolicy.shouldRetry(throwable)).isTrue();
46-
assertThat(backOffExecution.nextBackOff()).isGreaterThan(0);
47+
assertThat(backOffExecution.nextBackOff()).isZero();
4748
assertThat(retryPolicy.shouldRetry(throwable)).isTrue();
48-
assertThat(backOffExecution.nextBackOff()).isGreaterThan(0);
49+
assertThat(backOffExecution.nextBackOff()).isZero();
4950

5051
assertThat(retryPolicy.shouldRetry(throwable)).isTrue();
5152
assertThat(backOffExecution.nextBackOff()).isEqualTo(STOP);
@@ -65,13 +66,13 @@ void maxAttemptsAndPredicate() {
6566

6667
// 4 retries
6768
assertThat(retryPolicy.shouldRetry(new NumberFormatException())).isTrue();
68-
assertThat(backOffExecution.nextBackOff()).isGreaterThan(0);
69+
assertThat(backOffExecution.nextBackOff()).isEqualTo(1);
6970
assertThat(retryPolicy.shouldRetry(new IllegalStateException())).isFalse();
70-
assertThat(backOffExecution.nextBackOff()).isGreaterThan(0);
71+
assertThat(backOffExecution.nextBackOff()).isEqualTo(1);
7172
assertThat(retryPolicy.shouldRetry(new IllegalStateException())).isFalse();
72-
assertThat(backOffExecution.nextBackOff()).isGreaterThan(0);
73+
assertThat(backOffExecution.nextBackOff()).isEqualTo(1);
7374
assertThat(retryPolicy.shouldRetry(new CustomNumberFormatException())).isTrue();
74-
assertThat(backOffExecution.nextBackOff()).isGreaterThan(0);
75+
assertThat(backOffExecution.nextBackOff()).isEqualTo(1);
7576

7677
// After policy exhaustion
7778
assertThat(retryPolicy.shouldRetry(new NumberFormatException())).isTrue();
@@ -92,17 +93,17 @@ void maxAttemptsWithIncludesAndExcludes() {
9293

9394
// 6 retries
9495
assertThat(retryPolicy.shouldRetry(new IOException())).isTrue();
95-
assertThat(backOffExecution.nextBackOff()).isGreaterThan(0);
96+
assertThat(backOffExecution.nextBackOff()).isEqualTo(DEFAULT_DELAY);
9697
assertThat(retryPolicy.shouldRetry(new RuntimeException())).isTrue();
97-
assertThat(backOffExecution.nextBackOff()).isGreaterThan(0);
98+
assertThat(backOffExecution.nextBackOff()).isEqualTo(DEFAULT_DELAY);
9899
assertThat(retryPolicy.shouldRetry(new FileNotFoundException())).isFalse();
99-
assertThat(backOffExecution.nextBackOff()).isGreaterThan(0);
100+
assertThat(backOffExecution.nextBackOff()).isEqualTo(DEFAULT_DELAY);
100101
assertThat(retryPolicy.shouldRetry(new FileSystemException("file"))).isTrue();
101-
assertThat(backOffExecution.nextBackOff()).isGreaterThan(0);
102+
assertThat(backOffExecution.nextBackOff()).isEqualTo(DEFAULT_DELAY);
102103
assertThat(retryPolicy.shouldRetry(new CustomFileSystemException("file"))).isFalse();
103-
assertThat(backOffExecution.nextBackOff()).isGreaterThan(0);
104+
assertThat(backOffExecution.nextBackOff()).isEqualTo(DEFAULT_DELAY);
104105
assertThat(retryPolicy.shouldRetry(new IOException())).isTrue();
105-
assertThat(backOffExecution.nextBackOff()).isGreaterThan(0);
106+
assertThat(backOffExecution.nextBackOff()).isEqualTo(DEFAULT_DELAY);
106107

107108
// After policy exhaustion
108109
assertThat(retryPolicy.shouldRetry(new IOException())).isTrue();

spring-core/src/test/java/org/springframework/core/retry/RetryPolicyTests.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -162,12 +162,9 @@ void maxAttempts() {
162162

163163
@Test
164164
void delayPreconditions() {
165-
assertThatIllegalArgumentException()
166-
.isThrownBy(() -> RetryPolicy.builder().delay(Duration.ofMillis(0)))
167-
.withMessage("Invalid duration (0ms): delay must be positive.");
168165
assertThatIllegalArgumentException()
169166
.isThrownBy(() -> RetryPolicy.builder().delay(Duration.ofMillis(-1)))
170-
.withMessage("Invalid duration (-1ms): delay must be positive.");
167+
.withMessage("Invalid delay (-1ms): must be >= 0.");
171168
}
172169

173170
@Test

spring-core/src/test/java/org/springframework/core/retry/RetryTemplateTests.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ class RetryTemplateTests {
5151
void configureRetryTemplate() {
5252
var retryPolicy = RetryPolicy.builder()
5353
.maxAttempts(3)
54-
.delay(Duration.ofMillis(1))
54+
.delay(Duration.ofMillis(0))
5555
.build();
5656

5757
retryTemplate.setRetryPolicy(retryPolicy);
@@ -171,7 +171,7 @@ public String getName() {
171171

172172
var retryPolicy = RetryPolicy.builder()
173173
.maxAttempts(Integer.MAX_VALUE)
174-
.delay(Duration.ofMillis(1))
174+
.delay(Duration.ofMillis(0))
175175
.includes(IOException.class)
176176
.build();
177177

@@ -194,13 +194,13 @@ public String getName() {
194194
argumentSet("Excludes",
195195
RetryPolicy.builder()
196196
.maxAttempts(Integer.MAX_VALUE)
197-
.delay(Duration.ofMillis(1))
197+
.delay(Duration.ofMillis(0))
198198
.excludes(FileNotFoundException.class)
199199
.build()),
200200
argumentSet("Includes & Excludes",
201201
RetryPolicy.builder()
202202
.maxAttempts(Integer.MAX_VALUE)
203-
.delay(Duration.ofMillis(1))
203+
.delay(Duration.ofMillis(0))
204204
.includes(IOException.class)
205205
.excludes(FileNotFoundException.class)
206206
.build())

0 commit comments

Comments
 (0)