Consistent maxAttempts (long) and delay/maxDelay (Duration) declarations

Includes timeUnit attribute in @Retryable (aligned with @Scheduled).

See gh-34529
See gh-35110
This commit is contained in:
Juergen Hoeller 2025-06-28 11:40:00 +02:00
parent bcdf26d492
commit 15dd320b95
11 changed files with 90 additions and 82 deletions

View File

@ -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));

View File

@ -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<Class<? extends Throwable>> includes,
Collection<Class<? extends Throwable>> 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);

View File

@ -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<? extends MethodRetryPredicate> predicateClass) {
private static MethodRetryPredicate instantiatePredicate(Class<? extends MethodRetryPredicate> 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());
}
}
}

View File

@ -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.
* <p>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.
* <p>The time unit is milliseconds by default but can be overridden via
* {@link #timeUnit}.
* <p>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()}.
* <p>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.
* <p>The time unit is milliseconds by default but can be overridden via
* {@link #timeUnit}.
* <p>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}.
* <p>The time unit is milliseconds by default but can be overridden via
* {@link #timeUnit}.
* <p>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}.
* <p>Defaults to {@link TimeUnit#MILLISECONDS}.
*/
TimeUnit timeUnit() default TimeUnit.MILLISECONDS;
}

View File

@ -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())

View File

@ -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");

View File

@ -35,7 +35,7 @@ import org.springframework.util.backoff.FixedBackOff;
*
* <p>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.
* <p>Defaults to a fixed backoff of {@value Builder#DEFAULT_DELAY} milliseconds
@ -84,10 +83,10 @@ public interface RetryPolicy {
* <p>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 {
* <p>The supplied value will override any previously configured value.
* <p><strong>WARNING</strong>: 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;
}

View File

@ -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 {
* <p>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.

View File

@ -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;
}

View File

@ -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 + ']';
}
}

View File

@ -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<? super ExponentialBackOff> hasDefaultMaxAttemptsAndDelay() {
return backOff -> {
assertThat(backOff.getMaxAttempts()).isEqualTo(3);
@ -441,6 +442,7 @@ class RetryPolicyTests {
};
}
@SuppressWarnings("serial")
private static class CustomNumberFormatException extends NumberFormatException {