Skip to content

Commit c942b21

Browse files
committed
Generate consistent validation error messages in RetryPolicy
Closes gh-35355
1 parent fce7b3d commit c942b21

File tree

2 files changed

+29
-22
lines changed

2 files changed

+29
-22
lines changed

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

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ static RetryPolicy withDefaults() {
8787
* @see FixedBackOff
8888
*/
8989
static RetryPolicy withMaxAttempts(long maxAttempts) {
90-
Assert.isTrue(maxAttempts > 0, "Max attempts must be greater than zero");
90+
assertMaxAttemptsIsPositive(maxAttempts);
9191
return builder().backOff(new FixedBackOff(Builder.DEFAULT_DELAY, maxAttempts)).build();
9292
}
9393

@@ -100,6 +100,22 @@ static Builder builder() {
100100
}
101101

102102

103+
private static void assertMaxAttemptsIsPositive(long maxAttempts) {
104+
Assert.isTrue(maxAttempts > 0,
105+
() -> "Invalid maxAttempts (%d): must be greater than zero.".formatted(maxAttempts));
106+
}
107+
108+
private static void assertIsPositive(String name, Duration duration) {
109+
Assert.isTrue((!duration.isNegative() && !duration.isZero()),
110+
() -> "Invalid %s (%dms): must be greater than zero.".formatted(name, duration.toMillis()));
111+
}
112+
113+
private static void assertIsNotNegative(String name, Duration duration) {
114+
Assert.isTrue(!duration.isNegative(),
115+
() -> "Invalid %s (%dms): must be greater than or equal to zero.".formatted(name, duration.toMillis()));
116+
}
117+
118+
103119
/**
104120
* Fluent API for configuring a {@link RetryPolicy} with common configuration
105121
* options.
@@ -180,7 +196,7 @@ public Builder backOff(BackOff backOff) {
180196
* @return this {@code Builder} instance for chained method invocations
181197
*/
182198
public Builder maxAttempts(long maxAttempts) {
183-
Assert.isTrue(maxAttempts > 0, "Max attempts must be greater than zero");
199+
assertMaxAttemptsIsPositive(maxAttempts);
184200
this.maxAttempts = maxAttempts;
185201
return this;
186202
}
@@ -201,8 +217,7 @@ public Builder maxAttempts(long maxAttempts) {
201217
* @see #maxDelay(Duration)
202218
*/
203219
public Builder delay(Duration delay) {
204-
Assert.isTrue(!delay.isNegative(),
205-
() -> "Invalid delay (%dms): must be >= 0.".formatted(delay.toMillis()));
220+
assertIsNotNegative("delay", delay);
206221
this.delay = delay;
207222
return this;
208223
}
@@ -227,8 +242,7 @@ public Builder delay(Duration delay) {
227242
* @see #maxDelay(Duration)
228243
*/
229244
public Builder jitter(Duration jitter) {
230-
Assert.isTrue(!jitter.isNegative(),
231-
() -> "Invalid jitter (%dms): must be >= 0.".formatted(jitter.toMillis()));
245+
assertIsNotNegative("jitter", jitter);
232246
this.jitter = jitter;
233247
return this;
234248
}
@@ -243,6 +257,7 @@ public Builder jitter(Duration jitter) {
243257
* <p>The supplied value will override any previously configured value.
244258
* <p>You should not specify this configuration option if you have
245259
* configured a custom {@link #backOff(BackOff) BackOff} strategy.
260+
* @param multiplier the multiplier value; must be greater than or equal to 1
246261
* @return this {@code Builder} instance for chained method invocations
247262
* @see #delay(Duration)
248263
* @see #jitter(Duration)
@@ -264,7 +279,7 @@ public Builder multiplier(double multiplier) {
264279
* <p>The supplied value will override any previously configured value.
265280
* <p>You should not specify this configuration option if you have
266281
* configured a custom {@link #backOff(BackOff) BackOff} strategy.
267-
* @param maxDelay the maximum delay; must be positive
282+
* @param maxDelay the maximum delay; must be greater than zero
268283
* @return this {@code Builder} instance for chained method invocations
269284
* @see #delay(Duration)
270285
* @see #jitter(Duration)
@@ -403,11 +418,6 @@ public RetryPolicy build() {
403418
}
404419
return new DefaultRetryPolicy(this.includes, this.excludes, this.predicate, backOff);
405420
}
406-
407-
private static void assertIsPositive(String name, Duration duration) {
408-
Assert.isTrue((!duration.isNegative() && !duration.isZero()),
409-
() -> "Invalid duration (%dms): %s must be positive.".formatted(duration.toMillis(), name));
410-
}
411421
}
412422

413423
}

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

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,10 @@ void withDefaults() {
6767
void withMaxAttemptsPreconditions() {
6868
assertThatIllegalArgumentException()
6969
.isThrownBy(() -> RetryPolicy.withMaxAttempts(0))
70-
.withMessage("Max attempts must be greater than zero");
70+
.withMessage("Invalid maxAttempts (0): must be greater than zero.");
7171
assertThatIllegalArgumentException()
7272
.isThrownBy(() -> RetryPolicy.withMaxAttempts(-1))
73-
.withMessage("Max attempts must be greater than zero");
73+
.withMessage("Invalid maxAttempts (-1): must be greater than zero.");
7474
}
7575

7676
@Test
@@ -117,10 +117,10 @@ void backOff() {
117117
void maxAttemptsPreconditions() {
118118
assertThatIllegalArgumentException()
119119
.isThrownBy(() -> RetryPolicy.builder().maxAttempts(0))
120-
.withMessage("Max attempts must be greater than zero");
120+
.withMessage("Invalid maxAttempts (0): must be greater than zero.");
121121
assertThatIllegalArgumentException()
122122
.isThrownBy(() -> RetryPolicy.builder().maxAttempts(-1))
123-
.withMessage("Max attempts must be greater than zero");
123+
.withMessage("Invalid maxAttempts (-1): must be greater than zero.");
124124
}
125125

126126
@Test
@@ -141,7 +141,7 @@ void maxAttempts() {
141141
void delayPreconditions() {
142142
assertThatIllegalArgumentException()
143143
.isThrownBy(() -> RetryPolicy.builder().delay(Duration.ofMillis(-1)))
144-
.withMessage("Invalid delay (-1ms): must be >= 0.");
144+
.withMessage("Invalid delay (-1ms): must be greater than or equal to zero.");
145145
}
146146

147147
@Test
@@ -162,7 +162,7 @@ void delay() {
162162
void jitterPreconditions() {
163163
assertThatIllegalArgumentException()
164164
.isThrownBy(() -> RetryPolicy.builder().jitter(Duration.ofMillis(-1)))
165-
.withMessage("Invalid jitter (-1ms): must be >= 0.");
165+
.withMessage("Invalid jitter (-1ms): must be greater than or equal to zero.");
166166
}
167167

168168
@Test
@@ -208,12 +208,9 @@ void multiplier() {
208208

209209
@Test
210210
void maxDelayPreconditions() {
211-
assertThatIllegalArgumentException()
212-
.isThrownBy(() -> RetryPolicy.builder().maxDelay(Duration.ZERO))
213-
.withMessage("Invalid duration (0ms): maxDelay must be positive.");
214211
assertThatIllegalArgumentException()
215212
.isThrownBy(() -> RetryPolicy.builder().maxDelay(Duration.ofMillis(-1)))
216-
.withMessage("Invalid duration (-1ms): maxDelay must be positive.");
213+
.withMessage("Invalid maxDelay (-1ms): must be greater than zero.");
217214
}
218215

219216
@Test

0 commit comments

Comments
 (0)