Skip to content

Commit f94bfff

Browse files
authored
GH-427: Fix multiplierExpression & multiplierExpression
Fixes: #427 No randomness with configuration like: ``` @retryable(retryFor = {RuntimeException.class}, maxAttemptsExpression = "${retry.max-attempts}", backoff = @backoff(delayExpression = "${retry.delay}", multiplierExpression = "${retry.multiplier}", randomExpression = "${retry.random}")) ``` The random logic in the `AnnotationAwareRetryOperationsInterceptor` if `multiplierExpression` is for runtime evaluation. * Fix `AnnotationAwareRetryOperationsInterceptor` to check for `if (multiplier > 0 || parsedMultExp != null) {` before evaluating `random` * Fix `@BackOff(randomExpression)` Javadoc to indicate that it is always evaluated on configuration phase.
1 parent 9442435 commit f94bfff

File tree

4 files changed

+89
-4
lines changed

4 files changed

+89
-4
lines changed

src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@
7676
* @author Artem Bilan
7777
* @author Gary Russell
7878
* @author Roman Akentev
79+
* @author Aftab Shaikh
7980
* @since 1.1
8081
*/
8182
public class AnnotationAwareRetryOperationsInterceptor implements IntroductionInterceptor, BeanFactoryAware {
@@ -451,7 +452,8 @@ private BackOffPolicy getBackoffPolicy(Backoff backoff, boolean stateless) {
451452
boolean isRandom = false;
452453
String randomExpression = (String) attrs.get("randomExpression");
453454
Expression parsedRandomExp = null;
454-
if (multiplier > 0) {
455+
456+
if (multiplier > 0 || parsedMultExp != null) {
455457
isRandom = backoff.random();
456458
if (StringUtils.hasText(randomExpression)) {
457459
parsedRandomExp = parse(randomExpression);

src/main/java/org/springframework/retry/annotation/Backoff.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
*
4242
* @author Dave Syer
4343
* @author Gary Russell
44+
* @author Aftab Shaikh
4445
* @since 1.1
4546
*
4647
*/
@@ -129,8 +130,10 @@
129130
* Evaluates to a value. In the exponential case ({@link #multiplier()} > 1.0) set
130131
* this to true to have the backoff delays randomized, so that the maximum delay is
131132
* multiplier times the previous delay and the distribution is uniform between the two
132-
* values. Use {@code #{...}} for one-time evaluation during initialization, omit the
133-
* delimiters for evaluation at runtime.
133+
* values. This expression is always evaluated during initialization. If the
134+
* expression returns true then
135+
* {@link org.springframework.retry.backoff.ExponentialRandomBackOffPolicy} is used
136+
* else {@link org.springframework.retry.backoff.ExponentialBackOffPolicy} is used.
134137
* @return the flag to signal randomization is required (default false)
135138
*/
136139
String randomExpression() default "";

src/main/java/org/springframework/retry/backoff/BackOffPolicyBuilder.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@
6868
* load concurrent access.
6969
*
7070
* @author Tomaz Fernandes
71+
* @author Aftab Shaikh
7172
* @since 1.3.3
7273
*/
7374
public class BackOffPolicyBuilder {
@@ -217,7 +218,7 @@ public BackOffPolicyBuilder randomSupplier(Supplier<Boolean> randomSupplier) {
217218
public BackOffPolicy build() {
218219
if (this.multiplier != null && this.multiplier > 0 || this.multiplierSupplier != null) {
219220
ExponentialBackOffPolicy policy;
220-
if (Boolean.TRUE.equals(this.random)) {
221+
if (isRandom()) {
221222
policy = new ExponentialRandomBackOffPolicy();
222223
}
223224
else {
@@ -279,4 +280,9 @@ else if (this.delay != null) {
279280
return policy;
280281
}
281282

283+
private boolean isRandom() {
284+
return (this.randomSupplier != null && Boolean.TRUE.equals(this.randomSupplier.get()))
285+
|| Boolean.TRUE.equals(this.random);
286+
}
287+
282288
}

src/test/java/org/springframework/retry/backoff/BackOffPolicyBuilderTests.java

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
/**
2727
* @author Tomaz Fernandes
2828
* @author Gary Russell
29+
* @author Aftab Shaikh
2930
* @since 1.3.3
3031
*/
3132
public class BackOffPolicyBuilderTests {
@@ -109,4 +110,77 @@ public void shouldCreateExponentialRandomBackOff() {
109110
assertThat(new DirectFieldAccessor(policy).getPropertyValue("sleeper")).isEqualTo(mockSleeper);
110111
}
111112

113+
@Test
114+
public void shouldCreateExponentialRandomBackOffWhenProvidedRandomSupplier() {
115+
Sleeper mockSleeper = mock(Sleeper.class);
116+
BackOffPolicy backOffPolicy = BackOffPolicyBuilder.newBuilder()
117+
.delay(10000)
118+
.maxDelay(100000)
119+
.multiplier(10)
120+
.randomSupplier(() -> true)
121+
.sleeper(mockSleeper)
122+
.build();
123+
124+
assertThat(ExponentialRandomBackOffPolicy.class.isAssignableFrom(backOffPolicy.getClass())).isTrue();
125+
ExponentialRandomBackOffPolicy policy = (ExponentialRandomBackOffPolicy) backOffPolicy;
126+
assertThat(policy.getInitialInterval()).isEqualTo(10000);
127+
assertThat(policy.getMaxInterval()).isEqualTo(100000);
128+
assertThat(policy.getMultiplier()).isEqualTo(10);
129+
}
130+
131+
@Test
132+
public void shouldCreateExponentialRandomBackOffWithProvidedSuppliers() {
133+
Sleeper mockSleeper = mock(Sleeper.class);
134+
BackOffPolicy backOffPolicy = BackOffPolicyBuilder.newBuilder()
135+
.delaySupplier(() -> 10000L)
136+
.maxDelaySupplier(() -> 100000L)
137+
.multiplierSupplier(() -> 10d)
138+
.randomSupplier(() -> true)
139+
.sleeper(mockSleeper)
140+
.build();
141+
142+
assertThat(ExponentialRandomBackOffPolicy.class.isAssignableFrom(backOffPolicy.getClass())).isTrue();
143+
ExponentialRandomBackOffPolicy policy = (ExponentialRandomBackOffPolicy) backOffPolicy;
144+
assertThat(policy.getInitialInterval()).isEqualTo(10000);
145+
assertThat(policy.getMaxInterval()).isEqualTo(100000);
146+
assertThat(policy.getMultiplier()).isEqualTo(10);
147+
}
148+
149+
@Test
150+
public void shouldCreateExponentialBackOffWhenProvidedRandomSupplier() {
151+
Sleeper mockSleeper = mock(Sleeper.class);
152+
BackOffPolicy backOffPolicy = BackOffPolicyBuilder.newBuilder()
153+
.delay(100)
154+
.maxDelay(1000)
155+
.multiplier(2)
156+
.randomSupplier(() -> false)
157+
.sleeper(mockSleeper)
158+
.build();
159+
assertThat(backOffPolicy instanceof ExponentialRandomBackOffPolicy).isFalse();
160+
assertThat(ExponentialBackOffPolicy.class.isAssignableFrom(backOffPolicy.getClass())).isTrue();
161+
ExponentialBackOffPolicy policy = (ExponentialBackOffPolicy) backOffPolicy;
162+
assertThat(policy.getInitialInterval()).isEqualTo(100);
163+
assertThat(policy.getMaxInterval()).isEqualTo(1000);
164+
assertThat(policy.getMultiplier()).isEqualTo(2);
165+
}
166+
167+
@Test
168+
public void shouldCreateExponentialBackOffWithProvidedSuppliers() {
169+
Sleeper mockSleeper = mock(Sleeper.class);
170+
BackOffPolicy backOffPolicy = BackOffPolicyBuilder.newBuilder()
171+
.delaySupplier(() -> 10000L)
172+
.maxDelaySupplier(() -> 100000L)
173+
.multiplierSupplier(() -> 10d)
174+
.randomSupplier(() -> false)
175+
.sleeper(mockSleeper)
176+
.build();
177+
178+
assertThat(backOffPolicy instanceof ExponentialRandomBackOffPolicy).isFalse();
179+
assertThat(ExponentialBackOffPolicy.class.isAssignableFrom(backOffPolicy.getClass())).isTrue();
180+
ExponentialBackOffPolicy policy = (ExponentialBackOffPolicy) backOffPolicy;
181+
assertThat(policy.getInitialInterval()).isEqualTo(10000);
182+
assertThat(policy.getMaxInterval()).isEqualTo(100000);
183+
assertThat(policy.getMultiplier()).isEqualTo(10);
184+
}
185+
112186
}

0 commit comments

Comments
 (0)