From c942b21deaab4e6ec6d28f171841a4288f3ac18e Mon Sep 17 00:00:00 2001
From: Sam Brannen <104798+sbrannen@users.noreply.github.com>
Date: Wed, 20 Aug 2025 14:38:36 +0200
Subject: [PATCH] Generate consistent validation error messages in RetryPolicy

Closes gh-35355
---
 .../core/retry/RetryPolicy.java               | 34 ++++++++++++-------
 .../core/retry/RetryPolicyTests.java          | 17 ++++------
 2 files changed, 29 insertions(+), 22 deletions(-)

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 83f2bab2560..16b6ee2473e 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
@@ -87,7 +87,7 @@ public interface RetryPolicy {
 	 * @see FixedBackOff
 	 */
 	static RetryPolicy withMaxAttempts(long maxAttempts) {
-		Assert.isTrue(maxAttempts > 0, "Max attempts must be greater than zero");
+		assertMaxAttemptsIsPositive(maxAttempts);
 		return builder().backOff(new FixedBackOff(Builder.DEFAULT_DELAY, maxAttempts)).build();
 	}
 
@@ -100,6 +100,22 @@ public interface RetryPolicy {
 	}
 
 
+	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 assertIsNotNegative(String name, Duration duration) {
+		Assert.isTrue(!duration.isNegative(),
+				() -> "Invalid %s (%dms): must be greater than or equal to zero.".formatted(name, duration.toMillis()));
+	}
+
+
 	/**
 	 * Fluent API for configuring a {@link RetryPolicy} with common configuration
 	 * options.
@@ -180,7 +196,7 @@ public interface RetryPolicy {
 		 * @return this {@code Builder} instance for chained method invocations
 		 */
 		public Builder maxAttempts(long maxAttempts) {
-			Assert.isTrue(maxAttempts > 0, "Max attempts must be greater than zero");
+			assertMaxAttemptsIsPositive(maxAttempts);
 			this.maxAttempts = maxAttempts;
 			return this;
 		}
@@ -201,8 +217,7 @@ public interface RetryPolicy {
 		 * @see #maxDelay(Duration)
 		 */
 		public Builder delay(Duration delay) {
-			Assert.isTrue(!delay.isNegative(),
-					() -> "Invalid delay (%dms): must be >= 0.".formatted(delay.toMillis()));
+			assertIsNotNegative("delay", delay);
 			this.delay = delay;
 			return this;
 		}
@@ -227,8 +242,7 @@ public interface RetryPolicy {
 		 * @see #maxDelay(Duration)
 		 */
 		public Builder jitter(Duration jitter) {
-			Assert.isTrue(!jitter.isNegative(),
-					() -> "Invalid jitter (%dms): must be >= 0.".formatted(jitter.toMillis()));
+			assertIsNotNegative("jitter", jitter);
 			this.jitter = jitter;
 			return this;
 		}
@@ -243,6 +257,7 @@ public interface RetryPolicy {
 		 * <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 multiplier the multiplier value; must be greater than or equal to 1
 		 * @return this {@code Builder} instance for chained method invocations
 		 * @see #delay(Duration)
 		 * @see #jitter(Duration)
@@ -264,7 +279,7 @@ public interface RetryPolicy {
 		 * <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 maxDelay the maximum delay; must be positive
+		 * @param maxDelay the maximum delay; must be greater than zero
 		 * @return this {@code Builder} instance for chained method invocations
 		 * @see #delay(Duration)
 		 * @see #jitter(Duration)
@@ -403,11 +418,6 @@ public interface RetryPolicy {
 			}
 			return new DefaultRetryPolicy(this.includes, this.excludes, this.predicate, backOff);
 		}
-
-		private static void assertIsPositive(String name, Duration duration) {
-			Assert.isTrue((!duration.isNegative() && !duration.isZero()),
-					() -> "Invalid duration (%dms): %s must be positive.".formatted(duration.toMillis(), name));
-		}
 	}
 
 }
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 62b0a42e3f4..8f86350fc08 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
@@ -67,10 +67,10 @@ class RetryPolicyTests {
 		void withMaxAttemptsPreconditions() {
 			assertThatIllegalArgumentException()
 					.isThrownBy(() -> RetryPolicy.withMaxAttempts(0))
-					.withMessage("Max attempts must be greater than zero");
+					.withMessage("Invalid maxAttempts (0): must be greater than zero.");
 			assertThatIllegalArgumentException()
 					.isThrownBy(() -> RetryPolicy.withMaxAttempts(-1))
-					.withMessage("Max attempts must be greater than zero");
+					.withMessage("Invalid maxAttempts (-1): must be greater than zero.");
 		}
 
 		@Test
@@ -117,10 +117,10 @@ class RetryPolicyTests {
 		void maxAttemptsPreconditions() {
 			assertThatIllegalArgumentException()
 					.isThrownBy(() -> RetryPolicy.builder().maxAttempts(0))
-					.withMessage("Max attempts must be greater than zero");
+					.withMessage("Invalid maxAttempts (0): must be greater than zero.");
 			assertThatIllegalArgumentException()
 					.isThrownBy(() -> RetryPolicy.builder().maxAttempts(-1))
-					.withMessage("Max attempts must be greater than zero");
+					.withMessage("Invalid maxAttempts (-1): must be greater than zero.");
 		}
 
 		@Test
@@ -141,7 +141,7 @@ class RetryPolicyTests {
 		void delayPreconditions() {
 			assertThatIllegalArgumentException()
 					.isThrownBy(() -> RetryPolicy.builder().delay(Duration.ofMillis(-1)))
-					.withMessage("Invalid delay (-1ms): must be >= 0.");
+					.withMessage("Invalid delay (-1ms): must be greater than or equal to zero.");
 		}
 
 		@Test
@@ -162,7 +162,7 @@ class RetryPolicyTests {
 		void jitterPreconditions() {
 			assertThatIllegalArgumentException()
 					.isThrownBy(() -> RetryPolicy.builder().jitter(Duration.ofMillis(-1)))
-					.withMessage("Invalid jitter (-1ms): must be >= 0.");
+					.withMessage("Invalid jitter (-1ms): must be greater than or equal to zero.");
 		}
 
 		@Test
@@ -208,12 +208,9 @@ class RetryPolicyTests {
 
 		@Test
 		void maxDelayPreconditions() {
-			assertThatIllegalArgumentException()
-					.isThrownBy(() -> RetryPolicy.builder().maxDelay(Duration.ZERO))
-					.withMessage("Invalid duration (0ms): maxDelay must be positive.");
 			assertThatIllegalArgumentException()
 					.isThrownBy(() -> RetryPolicy.builder().maxDelay(Duration.ofMillis(-1)))
-					.withMessage("Invalid duration (-1ms): maxDelay must be positive.");
+					.withMessage("Invalid maxDelay (-1ms): must be greater than zero.");
 		}
 
 		@Test