Redesign RetryPolicy to directly incorporate BackOff

After experimenting with our newly introduced core retry support
(RetryPolicy, RetryTemplate, etc.) and @⁠Retryable support, it
became apparent that there are overlapping concerns between the current
RetryPolicy and BackOff contracts.

- RetryPolicy and BackOff both have stateful executions: RetryExecution
  and BackOffExecution. However, only one stateful execution is
  necessary.

- FixedBackOff and ExponentialBackOff already incorporate "retry" logic
  in terms of max attempts, max elapsed time, etc. Thus, there is no
  need to duplicate such behavior in a RetryPolicy and its
  RetryExecution.

- RetryTemplate currently accepts both a RetryPolicy and a BackOff in
  order to instrument the retry algorithm. However, users would
  probably rather focus on configuring all "retry" logic via a single
  mechanism.

In light of the above, this commit directly incorporates BackOff
in RetryPolicy as follows.

- Remove the RetryExecution interface and move its shouldRetry() method
  to RetryPolicy, replacing the current RetryExecution start() method.

- Introduce a default getBackOff() method in the RetryPolicy interface.

- Introduce RetryPolicy.withDefaults() factory method.

- Completely overhaul the RetryPolicy.Builder to provide support for
  configuring a BackOff strategy.

- Remove BackOff configuration from RetryTemplate.

- Revise the method signatures of callbacks in RetryListener.

The collective result of these changes can be witnessed in the
reworked implementation of AbstractRetryInterceptor.

RetryPolicy retryPolicy = RetryPolicy.builder()
		.includes(spec.includes())
		.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()))
		.multiplier(spec.multiplier())
		.build();

RetryTemplate retryTemplate = new RetryTemplate(retryPolicy);

See gh-34716
See gh-34529
See gh-35058
Closes gh-35110

Author: sbrannen

spring-aop/src/main/java/org/springframework/aop/retry/AbstractRetryInterceptor.java

@@ -34,7 +34,6 @@
 import org.springframework.core.retry.RetryTemplate;
 import org.springframework.core.retry.Retryable;
 import org.springframework.util.ClassUtils;
-import org.springframework.util.backoff.ExponentialBackOff;
 
 /**
  * Abstract retry interceptor implementation, adapting a given
@@ -89,26 +88,17 @@ public AbstractRetryInterceptor() {
 			}
 		}
 
-		RetryTemplate retryTemplate = new RetryTemplate();
-
-		RetryPolicy.Builder policyBuilder = RetryPolicy.builder();
-		for (Class<? extends Throwable> include : spec.includes()) {
-			policyBuilder.includes(include);
-		}
-		for (Class<? extends Throwable> exclude : spec.excludes()) {
-			policyBuilder.excludes(exclude);
-		}
-		policyBuilder.predicate(spec.predicate().forMethod(method));
-		policyBuilder.maxAttempts(spec.maxAttempts());
-		retryTemplate.setRetryPolicy(policyBuilder.build());
-
-		ExponentialBackOff backOff = new ExponentialBackOff();
-		backOff.setInitialInterval(spec.delay());
-		backOff.setJitter(spec.jitter());
-		backOff.setMultiplier(spec.multiplier());
-		backOff.setMaxInterval(spec.maxDelay());
-		backOff.setMaxAttempts(spec.maxAttempts());
-		retryTemplate.setBackOffPolicy(backOff);
+		RetryPolicy retryPolicy = RetryPolicy.builder()
+				.includes(spec.includes())
+				.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()))
+				.multiplier(spec.multiplier())
+				.build();
+		RetryTemplate retryTemplate = new RetryTemplate(retryPolicy);
 
 		try {
 			return retryTemplate.execute(new Retryable<>() {

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

@@ -16,15 +16,13 @@
 
 package org.springframework.core.retry;
 
-import java.time.Duration;
-import java.time.LocalDateTime;
 import java.util.Set;
 import java.util.StringJoiner;
 import java.util.function.Predicate;
 
 import org.jspecify.annotations.Nullable;
 
-import org.springframework.util.Assert;
+import org.springframework.util.backoff.BackOff;
 
 /**
  * Default {@link RetryPolicy} created by {@link RetryPolicy.Builder}.
@@ -35,44 +33,58 @@
  */
 class DefaultRetryPolicy implements RetryPolicy {
 
-	private final int maxAttempts;
-
-	private final @Nullable Duration maxDuration;
-
 	private final Set<Class<? extends Throwable>> includes;
 
 	private final Set<Class<? extends Throwable>> excludes;
 
 	private final @Nullable Predicate<Throwable> predicate;
 
+	private final BackOff backOff;
+
 
-	DefaultRetryPolicy(int maxAttempts, @Nullable Duration maxDuration, Set<Class<? extends Throwable>> includes,
-			Set<Class<? extends Throwable>> excludes, @Nullable Predicate<Throwable> predicate) {
 
-		Assert.isTrue((maxAttempts > 0 || maxDuration != null), "Max attempts or max duration must be specified");
+	DefaultRetryPolicy(Set<Class<? extends Throwable>> includes, Set<Class<? extends Throwable>> excludes,
+			@Nullable Predicate<Throwable> predicate, BackOff backOff) {
 
-		this.maxAttempts = maxAttempts;
-		this.maxDuration = maxDuration;
 		this.includes = includes;
 		this.excludes = excludes;
 		this.predicate = predicate;
+		this.backOff = backOff;
 	}
 
 
 	@Override
-	public RetryExecution start() {
-		return new DefaultRetryPolicyExecution();
+	public boolean shouldRetry(Throwable throwable) {
+		if (!this.excludes.isEmpty()) {
+			for (Class<? extends Throwable> excludedType : this.excludes) {
+				if (excludedType.isInstance(throwable)) {
+					return false;
+				}
+			}
+		}
+		if (!this.includes.isEmpty()) {
+			boolean included = false;
+			for (Class<? extends Throwable> includedType : this.includes) {
+				if (includedType.isInstance(throwable)) {
+					included = true;
+					break;
+				}
+			}
+			if (!included) {
+				return false;
+			}
+		}
+		return this.predicate == null || this.predicate.test(throwable);
+	}
+
+	@Override
+	public BackOff getBackOff() {
+		return this.backOff;
 	}
 
 	@Override
 	public String toString() {
 		StringJoiner result = new StringJoiner(", ", "DefaultRetryPolicy[", "]");
-		if (this.maxAttempts > 0) {
-			result.add("maxAttempts=" + this.maxAttempts);
-		}
-		if (this.maxDuration != null) {
-			result.add("maxDuration=" + this.maxDuration.toMillis() + "ms");
-		}
 		if (!this.includes.isEmpty()) {
 			result.add("includes=" + names(this.includes));
 		}
@@ -82,6 +94,7 @@ public String toString() {
 		if (this.predicate != null) {
 			result.add("predicate=" + this.predicate.getClass().getSimpleName());
 		}
+		result.add("backOff=" + this.backOff);
 		return result.toString();
 	}
 
@@ -95,73 +108,4 @@ private static String names(Set<Class<? extends Throwable>> types) {
 		return result.toString();
 	}
 
-
-	/**
-	 * {@link RetryExecution} for {@link DefaultRetryPolicy}.
-	 */
-	private class DefaultRetryPolicyExecution implements RetryExecution {
-
-		private final LocalDateTime retryStartTime = LocalDateTime.now();
-
-		private int retryCount;
-
-
-		@Override
-		public boolean shouldRetry(Throwable throwable) {
-			if (DefaultRetryPolicy.this.maxAttempts > 0 &&
-					this.retryCount++ >= DefaultRetryPolicy.this.maxAttempts) {
-				return false;
-			}
-			if (DefaultRetryPolicy.this.maxDuration != null) {
-				Duration retryDuration = Duration.between(this.retryStartTime, LocalDateTime.now());
-				if (retryDuration.compareTo(DefaultRetryPolicy.this.maxDuration) > 0) {
-					return false;
-				}
-			}
-			if (!DefaultRetryPolicy.this.excludes.isEmpty()) {
-				for (Class<? extends Throwable> excludedType : DefaultRetryPolicy.this.excludes) {
-					if (excludedType.isInstance(throwable)) {
-						return false;
-					}
-				}
-			}
-			if (!DefaultRetryPolicy.this.includes.isEmpty()) {
-				boolean included = false;
-				for (Class<? extends Throwable> includedType : DefaultRetryPolicy.this.includes) {
-					if (includedType.isInstance(throwable)) {
-						included = true;
-						break;
-					}
-				}
-				if (!included) {
-					return false;
-				}
-			}
-			return DefaultRetryPolicy.this.predicate == null || DefaultRetryPolicy.this.predicate.test(throwable);
-		}
-
-		@Override
-		public String toString() {
-			StringJoiner result = new StringJoiner(", ", "DefaultRetryPolicyExecution[", "]");
-			if (DefaultRetryPolicy.this.maxAttempts > 0) {
-				result.add("maxAttempts=" + DefaultRetryPolicy.this.maxAttempts);
-				result.add("retryCount=" + this.retryCount);
-			}
-			if (DefaultRetryPolicy.this.maxDuration != null) {
-				result.add("maxDuration=" + DefaultRetryPolicy.this.maxDuration.toMillis() + "ms");
-				result.add("retryStartTime=" + this.retryStartTime);
-			}
-			if (!DefaultRetryPolicy.this.includes.isEmpty()) {
-				result.add("includes=" + names(DefaultRetryPolicy.this.includes));
-			}
-			if (!DefaultRetryPolicy.this.excludes.isEmpty()) {
-				result.add("excludes=" + names(DefaultRetryPolicy.this.excludes));
-			}
-			if (DefaultRetryPolicy.this.predicate != null) {
-				result.add("predicate=" + DefaultRetryPolicy.this.predicate.getClass().getSimpleName());
-			}
-			return result.toString();
-		}
-	}
-
 }

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

@@ -21,46 +21,52 @@
 import org.springframework.core.retry.support.CompositeRetryListener;
 
 /**
- * An extension point that allows to inject code during key retry phases.
+ * {@code RetryListener} defines a <em>listener</em> API for reacting to events
+ * published during the execution of a {@link Retryable} operation.
  *
  * <p>Typically registered in a {@link RetryTemplate}, and can be composed using
  * a {@link CompositeRetryListener}.
  *
  * @author Mahmoud Ben Hassine
+ * @author Sam Brannen
  * @since 7.0
  * @see CompositeRetryListener
  */
 public interface RetryListener {
 
 	/**
 	 * Called before every retry attempt.
-	 * @param retryExecution the retry execution
+	 * @param retryPolicy the {@link RetryPolicy}
+	 * @param retryable the {@link Retryable} operation
 	 */
-	default void beforeRetry(RetryExecution retryExecution) {
+	default void beforeRetry(RetryPolicy retryPolicy, Retryable<?> retryable) {
 	}
 
 	/**
 	 * Called after the first successful retry attempt.
-	 * @param retryExecution the retry execution
-	 * @param result the result of the {@link Retryable}
+	 * @param retryPolicy the {@link RetryPolicy}
+	 * @param retryable the {@link Retryable} operation
+	 * @param result the result of the {@code Retryable} operation
 	 */
-	default void onRetrySuccess(RetryExecution retryExecution, @Nullable Object result) {
+	default void onRetrySuccess(RetryPolicy retryPolicy, Retryable<?> retryable, @Nullable Object result) {
 	}
 
 	/**
 	 * Called every time a retry attempt fails.
-	 * @param retryExecution the retry execution
-	 * @param throwable the exception thrown by the {@link Retryable}
+	 * @param retryPolicy the {@link RetryPolicy}
+	 * @param retryable the {@link Retryable} operation
+	 * @param throwable the exception thrown by the {@code Retryable} operation
 	 */
-	default void onRetryFailure(RetryExecution retryExecution, Throwable throwable) {
+	default void onRetryFailure(RetryPolicy retryPolicy, Retryable<?> retryable, Throwable throwable) {
 	}
 
 	/**
 	 * Called if the {@link RetryPolicy} is exhausted.
-	 * @param retryExecution the retry execution
-	 * @param throwable the last exception thrown by the {@link Retryable}
+	 * @param retryPolicy the {@code RetryPolicy}
+	 * @param retryable the {@code Retryable} operation
+	 * @param throwable the last exception thrown by the {@link Retryable} operation
 	 */
-	default void onRetryPolicyExhaustion(RetryExecution retryExecution, Throwable throwable) {
+	default void onRetryPolicyExhaustion(RetryPolicy retryPolicy, Retryable<?> retryable, Throwable throwable) {
 	}
 
 }