Expose RetryException#getRetryCount() and accept maxAttempts(0)

Closes gh-35351
Closes gh-35362

Author: jhoeller

spring-context/src/test/java/org/springframework/resilience/ReactiveRetryInterceptorTests.java

@@ -189,6 +189,26 @@ void adaptReactiveResultWithMinimalRetrySpec() {
 		assertThat(target.counter.get()).isEqualTo(2);
 	}
 
+	@Test
+	void adaptReactiveResultWithZeroAttempts() {
+		// Test minimal retry configuration: maxAttempts=1, delay=0, jitter=0, multiplier=1.0, maxDelay=0
+		MinimalRetryBean target = new MinimalRetryBean();
+		ProxyFactory pf = new ProxyFactory();
+		pf.setTarget(target);
+		pf.addAdvice(new SimpleRetryInterceptor(
+				new MethodRetrySpec((m, t) -> true, 0, Duration.ZERO, Duration.ZERO, 1.0, Duration.ZERO)));
+		MinimalRetryBean proxy = (MinimalRetryBean) pf.getProxy();
+
+		// Should execute only 1 time, because maxAttempts=0 means initial call only
+		assertThatIllegalStateException()
+				.isThrownBy(() -> proxy.retryOperation().block())
+				.satisfies(isRetryExhaustedException())
+				.havingCause()
+				.isInstanceOf(IOException.class)
+				.withMessage("1");
+		assertThat(target.counter.get()).isEqualTo(1);
+	}
+
 	@Test
 	void adaptReactiveResultWithZeroDelayAndJitter() {
 		// Test case where delay=0 and jitter>0

spring-context/src/test/java/org/springframework/resilience/RetryInterceptorTests.java

@@ -194,6 +194,31 @@ void withPostProcessorForClassWithStrings() {
 		assertThat(target.counter).isEqualTo(6);
 	}
 
+	@Test
+	void withPostProcessorForClassWithZeroAttempts() {
+		Properties props = new Properties();
+		props.setProperty("delay", "10");
+		props.setProperty("jitter", "5");
+		props.setProperty("multiplier", "2.0");
+		props.setProperty("maxDelay", "40");
+		props.setProperty("limitedAttempts", "0");
+
+		GenericApplicationContext ctx = new GenericApplicationContext();
+		ctx.getEnvironment().getPropertySources().addFirst(new PropertiesPropertySource("props", props));
+		ctx.registerBeanDefinition("bean", new RootBeanDefinition(AnnotatedClassBeanWithStrings.class));
+		ctx.registerBeanDefinition("bpp", new RootBeanDefinition(RetryAnnotationBeanPostProcessor.class));
+		ctx.refresh();
+		AnnotatedClassBeanWithStrings proxy = ctx.getBean(AnnotatedClassBeanWithStrings.class);
+		AnnotatedClassBeanWithStrings target = (AnnotatedClassBeanWithStrings) AopProxyUtils.getSingletonTarget(proxy);
+
+		assertThatIOException().isThrownBy(proxy::retryOperation).withMessage("3");
+		assertThat(target.counter).isEqualTo(3);
+		assertThatIOException().isThrownBy(proxy::otherOperation);
+		assertThat(target.counter).isEqualTo(4);
+		assertThatIOException().isThrownBy(proxy::overrideOperation);
+		assertThat(target.counter).isEqualTo(5);
+	}
+
 	@Test
 	void withEnableAnnotation() throws Exception {
 		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();

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

@@ -45,7 +45,6 @@ class DefaultRetryPolicy implements RetryPolicy {
 	private final BackOff backOff;
 
 
-
 	DefaultRetryPolicy(Set<Class<? extends Throwable>> includes, Set<Class<? extends Throwable>> excludes,
 			@Nullable Predicate<Throwable> predicate, BackOff backOff) {
 
@@ -59,8 +58,8 @@ class DefaultRetryPolicy implements RetryPolicy {
 
 	@Override
 	public boolean shouldRetry(Throwable throwable) {
-		return this.exceptionFilter.match(throwable) &&
-				(this.predicate == null || this.predicate.test(throwable));
+		return (this.exceptionFilter.match(throwable) &&
+				(this.predicate == null || this.predicate.test(throwable)));
 	}
 
 	@Override

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

@@ -28,6 +28,7 @@
  * exceptions}.
  *
  * @author Mahmoud Ben Hassine
+ * @author Juergen Hoeller
  * @since 7.0
  * @see RetryOperations
  */
@@ -51,8 +52,16 @@ public RetryException(String message, Throwable cause) {
 	 * Get the last exception thrown by the {@link Retryable} operation.
 	 */
 	@Override
-	public final synchronized Throwable getCause() {
+	public final Throwable getCause() {
 		return Objects.requireNonNull(super.getCause());
 	}
 
+	/**
+	 * Return the number of retry attempts, or 0 if no retry has been attempted
+	 * after the initial invocation at all.
+	 */
+	public int getRetryCount() {
+		return getSuppressed().length;
+	}
+
 }

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

@@ -82,12 +82,13 @@ static RetryPolicy withDefaults() {
 	 * Create a {@link RetryPolicy} configured with a maximum number of retry attempts.
 	 * <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
+	 * @param maxAttempts the maximum number of retry attempts;
+	 * must be positive (or zero for no retry)
 	 * @see Builder#maxAttempts(long)
 	 * @see FixedBackOff
 	 */
 	static RetryPolicy withMaxAttempts(long maxAttempts) {
-		assertMaxAttemptsIsPositive(maxAttempts);
+		assertMaxAttemptsIsNotNegative(maxAttempts);
 		return builder().backOff(new FixedBackOff(Builder.DEFAULT_DELAY, maxAttempts)).build();
 	}
 
@@ -100,21 +101,21 @@ static Builder builder() {
 	}
 
 
-	private static void assertMaxAttemptsIsPositive(long maxAttempts) {
-		Assert.isTrue(maxAttempts > 0,
-				() -> "Invalid maxAttempts (%d): must be greater than zero.".formatted(maxAttempts));
-	}
-
-	private static void assertIsPositive(String name, Duration duration) {
-		Assert.isTrue((!duration.isNegative() && !duration.isZero()),
-				() -> "Invalid %s (%dms): must be greater than zero.".formatted(name, duration.toMillis()));
+	private static void assertMaxAttemptsIsNotNegative(long maxAttempts) {
+		Assert.isTrue(maxAttempts >= 0,
+				() -> "Invalid maxAttempts (%d): must be positive or zero for no retry.".formatted(maxAttempts));
 	}
 
 	private static void assertIsNotNegative(String name, Duration duration) {
 		Assert.isTrue(!duration.isNegative(),
 				() -> "Invalid %s (%dms): must be greater than or equal to zero.".formatted(name, duration.toMillis()));
 	}
 
+	private static void assertIsPositive(String name, Duration duration) {
+		Assert.isTrue((!duration.isNegative() && !duration.isZero()),
+				() -> "Invalid %s (%dms): must be greater than zero.".formatted(name, duration.toMillis()));
+	}
+
 
 	/**
 	 * Fluent API for configuring a {@link RetryPolicy} with common configuration
@@ -146,13 +147,13 @@ final class Builder {
 
 		private @Nullable BackOff backOff;
 
-		private long maxAttempts;
+		private @Nullable Long maxAttempts;
 
 		private @Nullable Duration delay;
 
 		private @Nullable Duration jitter;
 
-		private double multiplier;
+		private @Nullable Double multiplier;
 
 		private @Nullable Duration maxDelay;
 
@@ -191,12 +192,12 @@ public Builder backOff(BackOff backOff) {
 		 * <p>The supplied value will override any previously configured value.
 		 * <p>You should not specify this configuration option if you have
 		 * configured a custom {@link #backOff(BackOff) BackOff} strategy.
-		 * @param maxAttempts the maximum number of retry attempts; must be
-		 * greater than zero
+		 * @param maxAttempts the maximum number of retry attempts;
+		 * must be positive (or zero for no retry)
 		 * @return this {@code Builder} instance for chained method invocations
 		 */
 		public Builder maxAttempts(long maxAttempts) {
-			assertMaxAttemptsIsPositive(maxAttempts);
+			assertMaxAttemptsIsNotNegative(maxAttempts);
 			this.maxAttempts = maxAttempts;
 			return this;
 		}
@@ -399,18 +400,18 @@ public Builder predicate(Predicate<Throwable> predicate) {
 		public RetryPolicy build() {
 			BackOff backOff = this.backOff;
 			if (backOff != null) {
-				boolean misconfigured = (this.maxAttempts != 0) || (this.delay != null) || (this.jitter != null) ||
-						(this.multiplier != 0) || (this.maxDelay != null);
+				boolean misconfigured = (this.maxAttempts != null || this.delay != null || this.jitter != null ||
+						this.multiplier != null || this.maxDelay != null);
 				Assert.state(!misconfigured, """
 						The following configuration options are not supported with a custom BackOff strategy: \
 						maxAttempts, delay, jitter, multiplier, or maxDelay.""");
 			}
 			else {
 				ExponentialBackOff exponentialBackOff = new ExponentialBackOff();
-				exponentialBackOff.setMaxAttempts(this.maxAttempts > 0 ? this.maxAttempts : DEFAULT_MAX_ATTEMPTS);
+				exponentialBackOff.setMaxAttempts(this.maxAttempts != null ? this.maxAttempts : DEFAULT_MAX_ATTEMPTS);
 				exponentialBackOff.setInitialInterval(this.delay != null ? this.delay.toMillis() : DEFAULT_DELAY);
 				exponentialBackOff.setMaxInterval(this.maxDelay != null ? this.maxDelay.toMillis() : DEFAULT_MAX_DELAY);
-				exponentialBackOff.setMultiplier(this.multiplier > 1 ? this.multiplier : DEFAULT_MULTIPLIER);
+				exponentialBackOff.setMultiplier(this.multiplier != null ? this.multiplier : DEFAULT_MULTIPLIER);
 				if (this.jitter != null) {
 					exponentialBackOff.setJitter(this.jitter.toMillis());
 				}

spring-core/src/test/java/org/springframework/core/retry/MaxAttemptsRetryPolicyTests.java

@@ -54,6 +54,18 @@ void maxAttempts() {
 		assertThat(backOffExecution.nextBackOff()).isEqualTo(STOP);
 	}
 
+	@Test
+	void maxAttemptsZero() {
+		var retryPolicy = RetryPolicy.builder().maxAttempts(0).delay(Duration.ZERO).build();
+		var backOffExecution = retryPolicy.getBackOff().start();
+		var throwable = mock(Throwable.class);
+
+		assertThat(retryPolicy.shouldRetry(throwable)).isTrue();
+		assertThat(backOffExecution.nextBackOff()).isEqualTo(STOP);
+		assertThat(retryPolicy.shouldRetry(throwable)).isTrue();
+		assertThat(backOffExecution.nextBackOff()).isEqualTo(STOP);
+	}
+
 	@Test
 	void maxAttemptsAndPredicate() {
 		var retryPolicy = RetryPolicy.builder()
@@ -115,6 +127,7 @@ void maxAttemptsWithIncludesAndExcludes() {
 	private static class CustomNumberFormatException extends NumberFormatException {
 	}
 
+
 	@SuppressWarnings("serial")
 	private static class CustomFileSystemException extends FileSystemException {
 

spring-core/src/test/java/org/springframework/core/retry/RetryPolicyTests.java

@@ -65,12 +65,9 @@ void withDefaults() {
 
 		@Test
 		void withMaxAttemptsPreconditions() {
-			assertThatIllegalArgumentException()
-					.isThrownBy(() -> RetryPolicy.withMaxAttempts(0))
-					.withMessage("Invalid maxAttempts (0): must be greater than zero.");
 			assertThatIllegalArgumentException()
 					.isThrownBy(() -> RetryPolicy.withMaxAttempts(-1))
-					.withMessage("Invalid maxAttempts (-1): must be greater than zero.");
+					.withMessageStartingWith("Invalid maxAttempts (-1)");
 		}
 
 		@Test
@@ -115,12 +112,9 @@ void backOff() {
 
 		@Test
 		void maxAttemptsPreconditions() {
-			assertThatIllegalArgumentException()
-					.isThrownBy(() -> RetryPolicy.builder().maxAttempts(0))
-					.withMessage("Invalid maxAttempts (0): must be greater than zero.");
 			assertThatIllegalArgumentException()
 					.isThrownBy(() -> RetryPolicy.builder().maxAttempts(-1))
-					.withMessage("Invalid maxAttempts (-1): must be greater than zero.");
+					.withMessageStartingWith("Invalid maxAttempts (-1)");
 		}
 
 		@Test

spring-core/src/test/java/org/springframework/core/retry/RetryTemplateTests.java

@@ -32,9 +32,6 @@
 import org.junit.jupiter.params.provider.FieldSource;
 import org.mockito.InOrder;
 
-import org.springframework.util.backoff.BackOff;
-import org.springframework.util.backoff.FixedBackOff;
-
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.junit.jupiter.params.provider.Arguments.argumentSet;
@@ -51,16 +48,13 @@
  *
  * @author Mahmoud Ben Hassine
  * @author Sam Brannen
+ * @author Juergen Hoeller
  * @since 7.0
  * @see RetryPolicyTests
  */
 class RetryTemplateTests {
 
-	private final RetryPolicy retryPolicy =
-			RetryPolicy.builder()
-				.maxAttempts(3)
-				.delay(Duration.ZERO)
-				.build();
+	private final RetryPolicy retryPolicy = RetryPolicy.builder().maxAttempts(3).delay(Duration.ZERO).build();
 
 	private final RetryTemplate retryTemplate = new RetryTemplate(retryPolicy);
 
@@ -104,28 +98,41 @@ void retryWithInitialFailureAndZeroRetriesRetryPolicy() {
 				.isThrownBy(() -> retryTemplate.execute(retryable))
 				.withMessageMatching("Retry policy for operation '.+?' exhausted; aborting execution")
 				.withCause(exception)
-				.satisfies(throwable -> assertThat(throwable.getSuppressed()).isEmpty());
+				.satisfies(throwable -> assertThat(throwable.getSuppressed()).isEmpty())
+				.satisfies(throwable -> assertThat(throwable.getRetryCount()).isZero());
 
 		// RetryListener interactions:
 		inOrder.verify(retryListener).onRetryPolicyExhaustion(retryPolicy, retryable, exception);
 		verifyNoMoreInteractions(retryListener);
 	}
 
 	@Test
-	void retryWithInitialFailureAndZeroRetriesBackOffPolicy() {
-		RetryPolicy retryPolicy = new RetryPolicy() {
+	void retryWithInitialFailureAndZeroRetriesFixedBackOffPolicy() {
+		RetryPolicy retryPolicy = RetryPolicy.withMaxAttempts(0);
 
-			@Override
-			public boolean shouldRetry(Throwable throwable) {
-				return true;
-			}
-
-			@Override
-			public BackOff getBackOff() {
-				return new FixedBackOff(10, 0); // Zero retries
-			}
+		RetryTemplate retryTemplate = new RetryTemplate(retryPolicy);
+		retryTemplate.setRetryListener(retryListener);
+		Exception exception = new RuntimeException("Boom!");
+		Retryable<String> retryable = () -> {
+			throw exception;
 		};
 
+		assertThatExceptionOfType(RetryException.class)
+				.isThrownBy(() -> retryTemplate.execute(retryable))
+				.withMessageMatching("Retry policy for operation '.+?' exhausted; aborting execution")
+				.withCause(exception)
+				.satisfies(throwable -> assertThat(throwable.getSuppressed()).isEmpty())
+				.satisfies(throwable -> assertThat(throwable.getRetryCount()).isZero());
+
+		// RetryListener interactions:
+		inOrder.verify(retryListener).onRetryPolicyExhaustion(retryPolicy, retryable, exception);
+		verifyNoMoreInteractions(retryListener);
+	}
+
+	@Test
+	void retryWithInitialFailureAndZeroRetriesBackOffPolicyFromBuilder() {
+		RetryPolicy retryPolicy = RetryPolicy.builder().maxAttempts(0).build();
+
 		RetryTemplate retryTemplate = new RetryTemplate(retryPolicy);
 		retryTemplate.setRetryListener(retryListener);
 		Exception exception = new RuntimeException("Boom!");
@@ -137,7 +144,8 @@ public BackOff getBackOff() {
 				.isThrownBy(() -> retryTemplate.execute(retryable))
 				.withMessageMatching("Retry policy for operation '.+?' exhausted; aborting execution")
 				.withCause(exception)
-				.satisfies(throwable -> assertThat(throwable.getSuppressed()).isEmpty());
+				.satisfies(throwable -> assertThat(throwable.getSuppressed()).isEmpty())
+				.satisfies(throwable -> assertThat(throwable.getRetryCount()).isZero());
 
 		// RetryListener interactions:
 		inOrder.verify(retryListener).onRetryPolicyExhaustion(retryPolicy, retryable, exception);
@@ -282,7 +290,8 @@ public String getName() {
 				.satisfies(hasSuppressedExceptionsSatisfyingExactly(
 						suppressed1 -> assertThat(suppressed1).isExactlyInstanceOf(FileNotFoundException.class),
 						suppressed2 -> assertThat(suppressed2).isExactlyInstanceOf(IOException.class)
-				));
+				))
+				.satisfies(throwable -> assertThat(throwable.getRetryCount()).isEqualTo(2));
 		// 3 = 1 initial invocation + 2 retry attempts
 		assertThat(invocationCount).hasValue(3);
 
@@ -344,7 +353,8 @@ public String getName() {
 				.satisfies(hasSuppressedExceptionsSatisfyingExactly(
 					suppressed1 -> assertThat(suppressed1).isExactlyInstanceOf(IOException.class),
 					suppressed2 -> assertThat(suppressed2).isExactlyInstanceOf(IOException.class)
-				));
+				))
+				.satisfies(throwable -> assertThat(throwable.getRetryCount()).isEqualTo(2));
 		// 3 = 1 initial invocation + 2 retry attempts
 		assertThat(invocationCount).hasValue(3);
 
@@ -366,8 +376,9 @@ private static void repeat(int times, Runnable runnable) {
 	}
 
 	@SafeVarargs
-	private static final Consumer<Throwable> hasSuppressedExceptionsSatisfyingExactly(
+	private static Consumer<Throwable> hasSuppressedExceptionsSatisfyingExactly(
 			ThrowingConsumer<? super Throwable>... requirements) {
+
 		return throwable -> assertThat(throwable.getSuppressed()).satisfiesExactly(requirements);
 	}
 
@@ -376,6 +387,7 @@ private static final Consumer<Throwable> hasSuppressedExceptionsSatisfyingExactl
 	private static class CustomFileNotFoundException extends FileNotFoundException {
 	}
 
+
 	/**
 	 * Custom {@link RuntimeException} that implements {@link #equals(Object)}
 	 * and {@link #hashCode()} for use in assertions that check for equality.