diff --git a/spring-aop/src/main/java/org/springframework/aop/retry/AbstractRetryInterceptor.java b/spring-aop/src/main/java/org/springframework/aop/retry/AbstractRetryInterceptor.java index bb253ffea32..6278b79e9d7 100644 --- a/spring-aop/src/main/java/org/springframework/aop/retry/AbstractRetryInterceptor.java +++ b/spring-aop/src/main/java/org/springframework/aop/retry/AbstractRetryInterceptor.java @@ -17,7 +17,6 @@ package org.springframework.aop.retry; import java.lang.reflect.Method; -import java.time.Duration; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; @@ -93,10 +92,10 @@ public abstract class AbstractRetryInterceptor implements MethodInterceptor { .excludes(spec.excludes()) .predicate(spec.predicate().forMethod(method)) .maxAttempts(spec.maxAttempts()) - .delay(Duration.ofMillis(spec.delay())) - .maxDelay(Duration.ofMillis(spec.maxDelay())) - .jitter(Duration.ofMillis(spec.jitter())) + .delay(spec.delay()) + .jitter(spec.jitter()) .multiplier(spec.multiplier()) + .maxDelay(spec.maxDelay()) .build(); RetryTemplate retryTemplate = new RetryTemplate(retryPolicy); @@ -136,10 +135,10 @@ public abstract class AbstractRetryInterceptor implements MethodInterceptor { Object result, ReactiveAdapter adapter, MethodRetrySpec spec, Method method) { Publisher publisher = adapter.toPublisher(result); - Retry retry = Retry.backoff(spec.maxAttempts(), Duration.ofMillis(spec.delay())) - .jitter((double) spec.jitter() / spec.delay()) + Retry retry = Retry.backoff(spec.maxAttempts(), spec.delay()) + .jitter((double) spec.jitter().toMillis() / spec.delay().toMillis()) .multiplier(spec.multiplier()) - .maxBackoff(Duration.ofMillis(spec.maxDelay())) + .maxBackoff(spec.maxDelay()) .filter(spec.combinedPredicate().forMethod(method)); publisher = (adapter.isMultiValue() ? Flux.from(publisher).retryWhen(retry) : Mono.from(publisher).retryWhen(retry)); diff --git a/spring-aop/src/main/java/org/springframework/aop/retry/MethodRetrySpec.java b/spring-aop/src/main/java/org/springframework/aop/retry/MethodRetrySpec.java index ff0e8060baa..76b32330dba 100644 --- a/spring-aop/src/main/java/org/springframework/aop/retry/MethodRetrySpec.java +++ b/spring-aop/src/main/java/org/springframework/aop/retry/MethodRetrySpec.java @@ -16,6 +16,7 @@ package org.springframework.aop.retry; +import java.time.Duration; import java.util.Collection; import java.util.Collections; @@ -42,18 +43,18 @@ public record MethodRetrySpec( Collection> includes, Collection> excludes, MethodRetryPredicate predicate, - int maxAttempts, - long delay, - long jitter, + long maxAttempts, + Duration delay, + Duration jitter, double multiplier, - long maxDelay) { + Duration maxDelay) { - public MethodRetrySpec(MethodRetryPredicate predicate, int maxAttempts, long delay) { - this(predicate, maxAttempts, delay, 0, 1.0, Integer.MAX_VALUE); + public MethodRetrySpec(MethodRetryPredicate predicate, long maxAttempts, Duration delay) { + this(predicate, maxAttempts, delay, Duration.ofMillis(0), 1.0, Duration.ofMillis(Long.MAX_VALUE)); } - public MethodRetrySpec(MethodRetryPredicate predicate, int maxAttempts, long delay, - long jitter, double multiplier, long maxDelay) { + public MethodRetrySpec(MethodRetryPredicate predicate, long maxAttempts, Duration delay, + Duration jitter, double multiplier, Duration maxDelay) { this(Collections.emptyList(), Collections.emptyList(), predicate, maxAttempts, delay, jitter, multiplier, maxDelay); diff --git a/spring-aop/src/main/java/org/springframework/aop/retry/annotation/RetryAnnotationInterceptor.java b/spring-aop/src/main/java/org/springframework/aop/retry/annotation/RetryAnnotationInterceptor.java index 99b27c1d012..13332583f6d 100644 --- a/spring-aop/src/main/java/org/springframework/aop/retry/annotation/RetryAnnotationInterceptor.java +++ b/spring-aop/src/main/java/org/springframework/aop/retry/annotation/RetryAnnotationInterceptor.java @@ -17,9 +17,11 @@ package org.springframework.aop.retry.annotation; import java.lang.reflect.Method; +import java.time.Duration; import java.util.Arrays; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; import org.jspecify.annotations.Nullable; @@ -57,17 +59,19 @@ public class RetryAnnotationInterceptor extends AbstractRetryInterceptor { } } + TimeUnit timeUnit = retryable.timeUnit(); retrySpec = new MethodRetrySpec( Arrays.asList(retryable.includes()), Arrays.asList(retryable.excludes()), instantiatePredicate(retryable.predicate()), retryable.maxAttempts(), - retryable.delay(), retryable.jitter(), - retryable.multiplier(), retryable.maxDelay()); + toDuration(retryable.delay(), timeUnit), toDuration(retryable.jitter(), timeUnit), + retryable.multiplier(), toDuration(retryable.maxDelay(), timeUnit)); MethodRetrySpec existing = this.retrySpecCache.putIfAbsent(cacheKey, retrySpec); return (existing != null ? existing : retrySpec); } - private MethodRetryPredicate instantiatePredicate(Class predicateClass) { + + private static MethodRetryPredicate instantiatePredicate(Class predicateClass) { if (predicateClass == MethodRetryPredicate.class) { return (method, throwable) -> true; } @@ -79,4 +83,14 @@ public class RetryAnnotationInterceptor extends AbstractRetryInterceptor { } } + private static Duration toDuration(long value, TimeUnit timeUnit) { + try { + return Duration.of(value, timeUnit.toChronoUnit()); + } + catch (Exception ex) { + throw new IllegalArgumentException( + "Unsupported unit " + timeUnit + " for value \"" + value + "\": " + ex.getMessage()); + } + } + } diff --git a/spring-aop/src/main/java/org/springframework/aop/retry/annotation/Retryable.java b/spring-aop/src/main/java/org/springframework/aop/retry/annotation/Retryable.java index fc036e3df9c..4868b45722b 100644 --- a/spring-aop/src/main/java/org/springframework/aop/retry/annotation/Retryable.java +++ b/spring-aop/src/main/java/org/springframework/aop/retry/annotation/Retryable.java @@ -21,6 +21,7 @@ import java.lang.annotation.ElementType; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +import java.util.concurrent.TimeUnit; import org.springframework.aop.retry.MethodRetryPredicate; import org.springframework.aot.hint.annotation.Reflective; @@ -91,11 +92,13 @@ public @interface Retryable { * The maximum number of retry attempts, in addition to the initial invocation. *

The default is 3. */ - int maxAttempts() default 3; + long maxAttempts() default 3; /** - * The base delay after the initial invocation in milliseconds. - * If a multiplier is specified, this serves as the initial delay to multiply from. + * The base delay after the initial invocation in milliseconds. If a multiplier + * is specified, 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. * @see #jitter() * @see #multiplier() @@ -104,11 +107,13 @@ public @interface Retryable { long delay() default 1000; /** - * A jitter value (in milliseconds) for the base retry attempt, randomly - * subtracted or added to the calculated delay, resulting in a value between - * {@code delay - jitter} and {@code delay + jitter} but never below the base - * {@link #delay()} or above {@link #maxDelay()}. - *

If a multiplier is specified, it is applied to the jitter value as well. + * A jitter value for the base retry attempt, randomly subtracted or added to + * the calculated delay, resulting in a value between {@code delay - jitter} + * and {@code delay + jitter} but never below the base {@link #delay()} or + * above {@link #maxDelay()}. If a multiplier is specified, it is applied + * to the jitter value as well. + *

The time unit is milliseconds by default but can be overridden via + * {@link #timeUnit}. *

The default is 0 (no jitter). * @see #delay() * @see #multiplier() @@ -128,14 +133,22 @@ public @interface Retryable { double multiplier() default 1.0; /** - * The maximum delay for any retry attempt (in milliseconds), limiting - * how far {@link #jitter()} and {@link #multiplier()} can increase the - * {@linkplain #delay() delay}. + * The maximum delay for any retry attempt, limiting how far {@link #jitter()} + * and {@link #multiplier()} can increase the {@linkplain #delay() delay}. + *

The time unit is milliseconds by default but can be overridden via + * {@link #timeUnit}. *

The default is unlimited. * @see #delay() * @see #jitter() * @see #multiplier() */ - long maxDelay() default Integer.MAX_VALUE; + long maxDelay() default Long.MAX_VALUE; + + /** + * The {@link TimeUnit} to use for {@link #delay}, {@link #jitter}, + * and {@link #maxDelay}. + *

Defaults to {@link TimeUnit#MILLISECONDS}. + */ + TimeUnit timeUnit() default TimeUnit.MILLISECONDS; } diff --git a/spring-aop/src/test/java/org/springframework/aop/retry/ReactiveRetryInterceptorTests.java b/spring-aop/src/test/java/org/springframework/aop/retry/ReactiveRetryInterceptorTests.java index 87ead86fe12..b0aa0f34029 100644 --- a/spring-aop/src/test/java/org/springframework/aop/retry/ReactiveRetryInterceptorTests.java +++ b/spring-aop/src/test/java/org/springframework/aop/retry/ReactiveRetryInterceptorTests.java @@ -19,6 +19,7 @@ package org.springframework.aop.retry; import java.io.IOException; import java.lang.reflect.Method; import java.nio.file.AccessDeniedException; +import java.time.Duration; import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.Test; @@ -48,7 +49,8 @@ public class ReactiveRetryInterceptorTests { NonAnnotatedBean target = new NonAnnotatedBean(); ProxyFactory pf = new ProxyFactory(); pf.setTarget(target); - pf.addAdvice(new SimpleRetryInterceptor(new MethodRetrySpec((m, t) -> true, 5, 10))); + pf.addAdvice(new SimpleRetryInterceptor( + new MethodRetrySpec((m, t) -> true, 5, Duration.ofMillis(10)))); NonAnnotatedBean proxy = (NonAnnotatedBean) pf.getProxy(); assertThatIllegalStateException().isThrownBy(() -> proxy.retryOperation().block()) diff --git a/spring-aop/src/test/java/org/springframework/aop/retry/RetryInterceptorTests.java b/spring-aop/src/test/java/org/springframework/aop/retry/RetryInterceptorTests.java index 19df78a2c7a..0bd79549d4e 100644 --- a/spring-aop/src/test/java/org/springframework/aop/retry/RetryInterceptorTests.java +++ b/spring-aop/src/test/java/org/springframework/aop/retry/RetryInterceptorTests.java @@ -19,6 +19,7 @@ package org.springframework.aop.retry; import java.io.IOException; import java.lang.reflect.Method; import java.nio.file.AccessDeniedException; +import java.time.Duration; import org.junit.jupiter.api.Test; @@ -44,7 +45,8 @@ public class RetryInterceptorTests { NonAnnotatedBean target = new NonAnnotatedBean(); ProxyFactory pf = new ProxyFactory(); pf.setTarget(target); - pf.addAdvice(new SimpleRetryInterceptor(new MethodRetrySpec((m, t) -> true, 5, 10))); + pf.addAdvice(new SimpleRetryInterceptor( + new MethodRetrySpec((m, t) -> true, 5, Duration.ofMillis(10)))); NonAnnotatedBean proxy = (NonAnnotatedBean) pf.getProxy(); assertThatIOException().isThrownBy(proxy::retryOperation).withMessage("6"); 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 018731119b3..c2a351f3f7b 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 @@ -35,7 +35,7 @@ import org.springframework.util.backoff.FixedBackOff; * *

Also provides factory methods and a fluent builder API for creating retry * policies with common configurations. See {@link #withDefaults()}, - * {@link #withMaxAttempts(int)}, {@link #withMaxElapsedTime(Duration)}, + * {@link #withMaxAttempts(long)}, {@link #withMaxElapsedTime(Duration)}, * {@link #builder()}, and the configuration options in {@link Builder} for details. * * @author Sam Brannen @@ -55,7 +55,6 @@ public interface RetryPolicy { */ boolean shouldRetry(Throwable throwable); - /** * Get the {@link BackOff} strategy to use for this retry policy. *

Defaults to a fixed backoff of {@value Builder#DEFAULT_DELAY} milliseconds @@ -84,10 +83,10 @@ public interface RetryPolicy { *

The returned policy uses a fixed backoff of {@value Builder#DEFAULT_DELAY} * milliseconds. * @param maxAttempts the maximum number of retry attempts; must be greater than zero - * @see Builder#maxAttempts(int) + * @see Builder#maxAttempts(long) * @see FixedBackOff */ - static RetryPolicy withMaxAttempts(int maxAttempts) { + static RetryPolicy withMaxAttempts(long maxAttempts) { Assert.isTrue(maxAttempts > 0, "Max attempts must be greater than zero"); return builder().backOff(new FixedBackOff(Builder.DEFAULT_DELAY, maxAttempts)).build(); } @@ -120,9 +119,9 @@ public interface RetryPolicy { final class Builder { /** - * The default {@linkplain #maxAttempts(int) max attempts}: {@value}. + * The default {@linkplain #maxAttempts(long) max attempts}: {@value}. */ - public static final int DEFAULT_MAX_ATTEMPTS = 3; + public static final long DEFAULT_MAX_ATTEMPTS = 3; /** * The default {@linkplain #delay(Duration) delay}: {@value} ms. @@ -143,7 +142,7 @@ public interface RetryPolicy { private @Nullable BackOff backOff; - private int maxAttempts; + private long maxAttempts; private @Nullable Duration delay; @@ -172,7 +171,7 @@ public interface RetryPolicy { *

The supplied value will override any previously configured value. *

WARNING: If you configure a custom {@code BackOff} * strategy, you should not configure any of the following: - * {@link #maxAttempts(int) maxAttempts}, {@link #delay(Duration) delay}, + * {@link #maxAttempts(long) maxAttempts}, {@link #delay(Duration) delay}, * {@link #jitter(Duration) jitter}, {@link #multiplier(double) multiplier}, * {@link #maxDelay(Duration) maxDelay}, or {@link #maxElapsedTime(Duration) * maxElapsedTime}. @@ -195,7 +194,7 @@ public interface RetryPolicy { * greater than zero * @return this {@code Builder} instance for chained method invocations */ - public Builder maxAttempts(int maxAttempts) { + public Builder maxAttempts(long maxAttempts) { Assert.isTrue(maxAttempts > 0, "Max attempts must be greater than zero"); this.maxAttempts = maxAttempts; return this; @@ -285,7 +284,7 @@ public interface RetryPolicy { * @see #multiplier(double) */ public Builder maxDelay(Duration maxDelay) { - assertIsPositive("max delay", maxDelay); + assertIsPositive("maxDelay", maxDelay); this.maxDelay = maxDelay; return this; } @@ -300,7 +299,7 @@ public interface RetryPolicy { * @return this {@code Builder} instance for chained method invocations */ public Builder maxElapsedTime(Duration maxElapsedTime) { - assertIsPositive("max elapsed time", maxElapsedTime); + assertIsPositive("maxElapsedTime", maxElapsedTime); this.maxElapsedTime = maxElapsedTime; return this; } diff --git a/spring-core/src/main/java/org/springframework/core/retry/RetryTemplate.java b/spring-core/src/main/java/org/springframework/core/retry/RetryTemplate.java index 09329303f5d..98f1043bb98 100644 --- a/spring-core/src/main/java/org/springframework/core/retry/RetryTemplate.java +++ b/spring-core/src/main/java/org/springframework/core/retry/RetryTemplate.java @@ -55,7 +55,6 @@ public class RetryTemplate implements RetryOperations { private static final LogAccessor logger = new LogAccessor(RetryTemplate.class); - private RetryPolicy retryPolicy = RetryPolicy.withDefaults(); private RetryListener retryListener = new RetryListener() {}; @@ -84,7 +83,7 @@ public class RetryTemplate implements RetryOperations { *

Defaults to {@code RetryPolicy.withDefaults()}. * @param retryPolicy the retry policy to use * @see RetryPolicy#withDefaults() - * @see RetryPolicy#withMaxAttempts(int) + * @see RetryPolicy#withMaxAttempts(long) * @see RetryPolicy#withMaxElapsedTime(Duration) * @see RetryPolicy#builder() */ @@ -105,6 +104,7 @@ public class RetryTemplate implements RetryOperations { this.retryListener = retryListener; } + /** * Execute the supplied {@link Retryable} according to the configured retry * and backoff policies. diff --git a/spring-core/src/main/java/org/springframework/util/backoff/ExponentialBackOff.java b/spring-core/src/main/java/org/springframework/util/backoff/ExponentialBackOff.java index 845d9c92796..0236497bcb0 100644 --- a/spring-core/src/main/java/org/springframework/util/backoff/ExponentialBackOff.java +++ b/spring-core/src/main/java/org/springframework/util/backoff/ExponentialBackOff.java @@ -103,7 +103,7 @@ public class ExponentialBackOff implements BackOff { private long maxElapsedTime = DEFAULT_MAX_ELAPSED_TIME; - private int maxAttempts = DEFAULT_MAX_ATTEMPTS; + private long maxAttempts = DEFAULT_MAX_ATTEMPTS; /** @@ -232,7 +232,7 @@ public class ExponentialBackOff implements BackOff { * @since 6.1 * @see #setMaxElapsedTime */ - public void setMaxAttempts(int maxAttempts) { + public void setMaxAttempts(long maxAttempts) { this.maxAttempts = maxAttempts; } @@ -243,7 +243,7 @@ public class ExponentialBackOff implements BackOff { * @since 6.1 * @see #getMaxElapsedTime() */ - public int getMaxAttempts() { + public long getMaxAttempts() { return this.maxAttempts; } diff --git a/spring-core/src/main/java/org/springframework/util/backoff/FixedBackOff.java b/spring-core/src/main/java/org/springframework/util/backoff/FixedBackOff.java index 380f652cda8..b75522857d3 100644 --- a/spring-core/src/main/java/org/springframework/util/backoff/FixedBackOff.java +++ b/spring-core/src/main/java/org/springframework/util/backoff/FixedBackOff.java @@ -16,8 +16,6 @@ package org.springframework.util.backoff; -import java.time.Duration; - /** * A simple {@link BackOff} implementation that provides a fixed interval * between two attempts and a maximum number of retries. @@ -52,28 +50,6 @@ public class FixedBackOff implements BackOff { public FixedBackOff() { } - /** - * Create an instance with the supplied interval and an unlimited number of - * attempts. - * @param interval the interval between two attempts in milliseconds - * @since 7.0 - * @see #setMaxAttempts(long) - */ - public FixedBackOff(long interval) { - this.interval = interval; - } - - /** - * Create an instance with the supplied interval and an unlimited number of - * attempts. - * @param interval the interval between two attempts - * @since 7.0 - * @see #setMaxAttempts(long) - */ - public FixedBackOff(Duration interval) { - this.interval = interval.toMillis(); - } - /** * Create an instance with the supplied interval and maximum number of attempts. * @param interval the interval between two attempts in milliseconds @@ -127,6 +103,7 @@ public class FixedBackOff implements BackOff { ", maxAttempts=" + attemptValue + ']'; } + private class FixedBackOffExecution implements BackOffExecution { private long currentAttempts = 0; @@ -148,8 +125,7 @@ public class FixedBackOff implements BackOff { "unlimited" : String.valueOf(FixedBackOff.this.maxAttempts)); return "FixedBackOffExecution[interval=" + FixedBackOff.this.interval + ", currentAttempts=" + this.currentAttempts + - ", maxAttempts=" + attemptValue + - ']'; + ", maxAttempts=" + attemptValue + ']'; } } 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 b0e4568401f..8053335d9e9 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 @@ -92,10 +92,10 @@ class RetryPolicyTests { void withMaxElapsedTimePreconditions() { assertThatIllegalArgumentException() .isThrownBy(() -> RetryPolicy.withMaxElapsedTime(Duration.ofMillis(0))) - .withMessage("Invalid duration (0ms): max elapsed time must be positive."); + .withMessage("Invalid duration (0ms): maxElapsedTime must be positive."); assertThatIllegalArgumentException() .isThrownBy(() -> RetryPolicy.withMaxElapsedTime(Duration.ofMillis(-1))) - .withMessage("Invalid duration (-1ms): max elapsed time must be positive."); + .withMessage("Invalid duration (-1ms): maxElapsedTime must be positive."); } @Test @@ -110,9 +110,9 @@ class RetryPolicyTests { .satisfies(hasDefaultMaxAttemptsAndDelay()) .extracting(ExponentialBackOff::getMaxElapsedTime).isEqualTo(42L); } - } + @Nested class BuilderTests { @@ -236,10 +236,10 @@ class RetryPolicyTests { void maxDelayPreconditions() { assertThatIllegalArgumentException() .isThrownBy(() -> RetryPolicy.builder().maxDelay(Duration.ofMillis(0))) - .withMessage("Invalid duration (0ms): max delay must be positive."); + .withMessage("Invalid duration (0ms): maxDelay must be positive."); assertThatIllegalArgumentException() .isThrownBy(() -> RetryPolicy.builder().maxDelay(Duration.ofMillis(-1))) - .withMessage("Invalid duration (-1ms): max delay must be positive."); + .withMessage("Invalid duration (-1ms): maxDelay must be positive."); } @Test @@ -258,10 +258,10 @@ class RetryPolicyTests { void maxElapsedTimePreconditions() { assertThatIllegalArgumentException() .isThrownBy(() -> RetryPolicy.builder().maxElapsedTime(Duration.ofMillis(0))) - .withMessage("Invalid duration (0ms): max elapsed time must be positive."); + .withMessage("Invalid duration (0ms): maxElapsedTime must be positive."); assertThatIllegalArgumentException() .isThrownBy(() -> RetryPolicy.builder().maxElapsedTime(Duration.ofMillis(-1))) - .withMessage("Invalid duration (-1ms): max elapsed time must be positive."); + .withMessage("Invalid duration (-1ms): maxElapsedTime must be positive."); } @Test @@ -399,6 +399,7 @@ class RetryPolicyTests { .matches("DefaultRetryPolicy\\[predicate=Predicate.+?Lambda.+?, backOff=ExponentialBackOff\\[.+?]]"); } + private static void assertToString(RetryPolicy policy, long initialInterval, long jitter, double multiplier, long maxInterval, long maxElapsedTime, int maxAttempts) { @@ -431,9 +432,9 @@ class RetryPolicyTests { } return result.toString(); } - } + private static ThrowingConsumer hasDefaultMaxAttemptsAndDelay() { return backOff -> { assertThat(backOff.getMaxAttempts()).isEqualTo(3); @@ -441,6 +442,7 @@ class RetryPolicyTests { }; } + @SuppressWarnings("serial") private static class CustomNumberFormatException extends NumberFormatException {