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

Includes timeUnit attribute in @retryable (aligned with @scheduled).

See gh-34529
See gh-35110

Author: jhoeller

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 AbstractRetryInterceptor() {
 				.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 static Object adaptReactiveResult(
 				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));

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

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<? extends MethodRetryPredicate> predicateClass) {
+
+	private static MethodRetryPredicate instantiatePredicate(Class<? extends MethodRetryPredicate> predicateClass) {
 		if (predicateClass == MethodRetryPredicate.class) {
 			return (method, throwable) -> true;
 		}
@@ -79,4 +83,14 @@ private MethodRetryPredicate instantiatePredicate(Class<? extends MethodRetryPre
 		}
 	}
 
+	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());
+		}
+	}
+
 }

spring-aop/src/main/java/org/springframework/aop/retry/annotation/Retryable.java

@@ -21,6 +21,7 @@
 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 @@
 	 * 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 @@
 	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 @@
 	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;
 
 }

spring-aop/src/test/java/org/springframework/aop/retry/ReactiveRetryInterceptorTests.java

@@ -19,6 +19,7 @@
 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 @@ void withSimpleInterceptor() {
 		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())

spring-aop/src/test/java/org/springframework/aop/retry/RetryInterceptorTests.java

@@ -19,6 +19,7 @@
 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 @@ void withSimpleInterceptor() {
 		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");

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

@@ -35,7 +35,7 @@
  *
  * <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 @@ static RetryPolicy withDefaults() {
 	 * <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 @@ static Builder builder() {
 	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 @@ final class Builder {
 
 		private @Nullable BackOff backOff;
 
-		private int maxAttempts;
+		private long maxAttempts;
 
 		private @Nullable Duration delay;
 
@@ -172,7 +171,7 @@ private Builder() {
 		 * <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 Builder backOff(BackOff backOff) {
 		 * 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 Builder multiplier(double multiplier) {
 		 * @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 Builder maxDelay(Duration maxDelay) {
 		 * @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;
 		}

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 RetryTemplate(RetryPolicy retryPolicy) {
 	 * <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 void setRetryListener(RetryListener retryListener) {
 		this.retryListener = retryListener;
 	}
 
+
 	/**
 	 * Execute the supplied {@link Retryable} according to the configured retry
 	 * and backoff policies.

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 long getMaxElapsedTime() {
 	 * @since 6.1
 	 * @see #setMaxElapsedTime
 	 */
-	public void setMaxAttempts(int maxAttempts) {
+	public void setMaxAttempts(long maxAttempts) {
 		this.maxAttempts = maxAttempts;
 	}
 
@@ -243,7 +243,7 @@ public void setMaxAttempts(int maxAttempts) {
 	 * @since 6.1
 	 * @see #getMaxElapsedTime()
 	 */
-	public int getMaxAttempts() {
+	public long getMaxAttempts() {
 		return this.maxAttempts;
 	}