From cc31bf3c3338bafdf5843782668b2c7e8b5e1e0d Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Tue, 1 Jul 2025 17:40:15 +0200 Subject: [PATCH] Accept zero for RetryPolicy.Builder.delay() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This aligns the programmatic RetryPolicy configuration option with the delay support in @Retryable. See gh-35110 --- .../resilience/annotation/Retryable.java | 2 +- .../core/retry/RetryPolicy.java | 8 +++--- .../retry/MaxAttemptsRetryPolicyTests.java | 27 ++++++++++--------- .../core/retry/RetryPolicyTests.java | 5 +--- .../core/retry/RetryTemplateTests.java | 8 +++--- 5 files changed, 25 insertions(+), 25 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/resilience/annotation/Retryable.java b/spring-context/src/main/java/org/springframework/resilience/annotation/Retryable.java index 869d58578b5..3f27d2083d2 100644 --- a/spring-context/src/main/java/org/springframework/resilience/annotation/Retryable.java +++ b/spring-context/src/main/java/org/springframework/resilience/annotation/Retryable.java @@ -111,7 +111,7 @@ public @interface Retryable { * this serves as the initial delay to multiply from. *
The time unit is milliseconds by default but can be overridden via * {@link #timeUnit}. - *
The default is 1000. + *
Must be greater than or equal to zero. The default is 1000. * @see #jitter() * @see #multiplier() * @see #maxDelay() diff --git a/spring-core/src/main/java/org/springframework/core/retry/RetryPolicy.java b/spring-core/src/main/java/org/springframework/core/retry/RetryPolicy.java index c2a351f3f7b..c99c11e6778 100644 --- a/spring-core/src/main/java/org/springframework/core/retry/RetryPolicy.java +++ b/spring-core/src/main/java/org/springframework/core/retry/RetryPolicy.java @@ -209,14 +209,15 @@ public interface RetryPolicy { *
You should not specify this configuration option if you have * configured a custom {@link #backOff(BackOff) BackOff} strategy. * @param delay the base delay, typically in milliseconds or seconds; - * must be positive + * must be greater than or equal to zero * @return this {@code Builder} instance for chained method invocations * @see #jitter(Duration) * @see #multiplier(double) * @see #maxDelay(Duration) */ public Builder delay(Duration delay) { - assertIsPositive("delay", delay); + Assert.isTrue(!delay.isNegative(), + () -> "Invalid delay (%dms): must be >= 0.".formatted(delay.toMillis())); this.delay = delay; return this; } @@ -233,7 +234,8 @@ public interface RetryPolicy { *
The supplied value will override any previously configured value. *
You should not specify this configuration option if you have * configured a custom {@link #backOff(BackOff) BackOff} strategy. - * @param jitter the jitter value, typically in milliseconds; must be positive + * @param jitter the jitter value, typically in milliseconds; must be + * greater than or equal to zero * @return this {@code Builder} instance for chained method invocations * @see #delay(Duration) * @see #multiplier(double) diff --git a/spring-core/src/test/java/org/springframework/core/retry/MaxAttemptsRetryPolicyTests.java b/spring-core/src/test/java/org/springframework/core/retry/MaxAttemptsRetryPolicyTests.java index 8a28d519d9a..1fadc24f235 100644 --- a/spring-core/src/test/java/org/springframework/core/retry/MaxAttemptsRetryPolicyTests.java +++ b/spring-core/src/test/java/org/springframework/core/retry/MaxAttemptsRetryPolicyTests.java @@ -25,6 +25,7 @@ import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; +import static org.springframework.core.retry.RetryPolicy.Builder.DEFAULT_DELAY; import static org.springframework.util.backoff.BackOffExecution.STOP; /** @@ -38,14 +39,14 @@ class MaxAttemptsRetryPolicyTests { @Test void maxAttempts() { - var retryPolicy = RetryPolicy.builder().maxAttempts(2).delay(Duration.ofMillis(1)).build(); + var retryPolicy = RetryPolicy.builder().maxAttempts(2).delay(Duration.ofMillis(0)).build(); var backOffExecution = retryPolicy.getBackOff().start(); var throwable = mock(Throwable.class); assertThat(retryPolicy.shouldRetry(throwable)).isTrue(); - assertThat(backOffExecution.nextBackOff()).isGreaterThan(0); + assertThat(backOffExecution.nextBackOff()).isZero(); assertThat(retryPolicy.shouldRetry(throwable)).isTrue(); - assertThat(backOffExecution.nextBackOff()).isGreaterThan(0); + assertThat(backOffExecution.nextBackOff()).isZero(); assertThat(retryPolicy.shouldRetry(throwable)).isTrue(); assertThat(backOffExecution.nextBackOff()).isEqualTo(STOP); @@ -65,13 +66,13 @@ class MaxAttemptsRetryPolicyTests { // 4 retries assertThat(retryPolicy.shouldRetry(new NumberFormatException())).isTrue(); - assertThat(backOffExecution.nextBackOff()).isGreaterThan(0); + assertThat(backOffExecution.nextBackOff()).isEqualTo(1); assertThat(retryPolicy.shouldRetry(new IllegalStateException())).isFalse(); - assertThat(backOffExecution.nextBackOff()).isGreaterThan(0); + assertThat(backOffExecution.nextBackOff()).isEqualTo(1); assertThat(retryPolicy.shouldRetry(new IllegalStateException())).isFalse(); - assertThat(backOffExecution.nextBackOff()).isGreaterThan(0); + assertThat(backOffExecution.nextBackOff()).isEqualTo(1); assertThat(retryPolicy.shouldRetry(new CustomNumberFormatException())).isTrue(); - assertThat(backOffExecution.nextBackOff()).isGreaterThan(0); + assertThat(backOffExecution.nextBackOff()).isEqualTo(1); // After policy exhaustion assertThat(retryPolicy.shouldRetry(new NumberFormatException())).isTrue(); @@ -92,17 +93,17 @@ class MaxAttemptsRetryPolicyTests { // 6 retries assertThat(retryPolicy.shouldRetry(new IOException())).isTrue(); - assertThat(backOffExecution.nextBackOff()).isGreaterThan(0); + assertThat(backOffExecution.nextBackOff()).isEqualTo(DEFAULT_DELAY); assertThat(retryPolicy.shouldRetry(new RuntimeException())).isTrue(); - assertThat(backOffExecution.nextBackOff()).isGreaterThan(0); + assertThat(backOffExecution.nextBackOff()).isEqualTo(DEFAULT_DELAY); assertThat(retryPolicy.shouldRetry(new FileNotFoundException())).isFalse(); - assertThat(backOffExecution.nextBackOff()).isGreaterThan(0); + assertThat(backOffExecution.nextBackOff()).isEqualTo(DEFAULT_DELAY); assertThat(retryPolicy.shouldRetry(new FileSystemException("file"))).isTrue(); - assertThat(backOffExecution.nextBackOff()).isGreaterThan(0); + assertThat(backOffExecution.nextBackOff()).isEqualTo(DEFAULT_DELAY); assertThat(retryPolicy.shouldRetry(new CustomFileSystemException("file"))).isFalse(); - assertThat(backOffExecution.nextBackOff()).isGreaterThan(0); + assertThat(backOffExecution.nextBackOff()).isEqualTo(DEFAULT_DELAY); assertThat(retryPolicy.shouldRetry(new IOException())).isTrue(); - assertThat(backOffExecution.nextBackOff()).isGreaterThan(0); + assertThat(backOffExecution.nextBackOff()).isEqualTo(DEFAULT_DELAY); // After policy exhaustion assertThat(retryPolicy.shouldRetry(new IOException())).isTrue(); diff --git a/spring-core/src/test/java/org/springframework/core/retry/RetryPolicyTests.java b/spring-core/src/test/java/org/springframework/core/retry/RetryPolicyTests.java index 8053335d9e9..60f2da526d3 100644 --- a/spring-core/src/test/java/org/springframework/core/retry/RetryPolicyTests.java +++ b/spring-core/src/test/java/org/springframework/core/retry/RetryPolicyTests.java @@ -162,12 +162,9 @@ class RetryPolicyTests { @Test void delayPreconditions() { - assertThatIllegalArgumentException() - .isThrownBy(() -> RetryPolicy.builder().delay(Duration.ofMillis(0))) - .withMessage("Invalid duration (0ms): delay must be positive."); assertThatIllegalArgumentException() .isThrownBy(() -> RetryPolicy.builder().delay(Duration.ofMillis(-1))) - .withMessage("Invalid duration (-1ms): delay must be positive."); + .withMessage("Invalid delay (-1ms): must be >= 0."); } @Test diff --git a/spring-core/src/test/java/org/springframework/core/retry/RetryTemplateTests.java b/spring-core/src/test/java/org/springframework/core/retry/RetryTemplateTests.java index 27995a45d70..d7a40388558 100644 --- a/spring-core/src/test/java/org/springframework/core/retry/RetryTemplateTests.java +++ b/spring-core/src/test/java/org/springframework/core/retry/RetryTemplateTests.java @@ -51,7 +51,7 @@ class RetryTemplateTests { void configureRetryTemplate() { var retryPolicy = RetryPolicy.builder() .maxAttempts(3) - .delay(Duration.ofMillis(1)) + .delay(Duration.ofMillis(0)) .build(); retryTemplate.setRetryPolicy(retryPolicy); @@ -171,7 +171,7 @@ class RetryTemplateTests { var retryPolicy = RetryPolicy.builder() .maxAttempts(Integer.MAX_VALUE) - .delay(Duration.ofMillis(1)) + .delay(Duration.ofMillis(0)) .includes(IOException.class) .build(); @@ -194,13 +194,13 @@ class RetryTemplateTests { argumentSet("Excludes", RetryPolicy.builder() .maxAttempts(Integer.MAX_VALUE) - .delay(Duration.ofMillis(1)) + .delay(Duration.ofMillis(0)) .excludes(FileNotFoundException.class) .build()), argumentSet("Includes & Excludes", RetryPolicy.builder() .maxAttempts(Integer.MAX_VALUE) - .delay(Duration.ofMillis(1)) + .delay(Duration.ofMillis(0)) .includes(IOException.class) .excludes(FileNotFoundException.class) .build())