GH-427: Add fix for the issue with multiplierExpression and randomExp…

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

@@ -76,6 +76,7 @@
  * @author Artem Bilan
  * @author Gary Russell
  * @author Roman Akentev
+ * @author Aftab Shaikh
  * @since 1.1
  */
 public class AnnotationAwareRetryOperationsInterceptor implements IntroductionInterceptor, BeanFactoryAware {
@@ -451,7 +452,8 @@ private BackOffPolicy getBackoffPolicy(Backoff backoff, boolean stateless) {
 		boolean isRandom = false;
 		String randomExpression = (String) attrs.get("randomExpression");
 		Expression parsedRandomExp = null;
-		if (multiplier > 0) {
+
+		if (multiplier > 0 || parsedMultExp != null) {
 			isRandom = backoff.random();
 			if (StringUtils.hasText(randomExpression)) {
 				parsedRandomExp = parse(randomExpression);

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

@@ -41,6 +41,7 @@
  *
  * @author Dave Syer
  * @author Gary Russell
+ * @author Aftab Shaikh
  * @since 1.1
  *
  */
@@ -129,8 +130,10 @@
 	 * Evaluates to a value. In the exponential case ({@link #multiplier()} > 1.0) set
 	 * this to true to have the backoff delays randomized, so that the maximum delay is
 	 * multiplier times the previous delay and the distribution is uniform between the two
-	 * values. Use {@code #{...}} for one-time evaluation during initialization, omit the
-	 * delimiters for evaluation at runtime.
+	 * values. This expression is always evaluated during initialization. If the
+	 * expression returns true then
+	 * {@link org.springframework.retry.backoff.ExponentialRandomBackOffPolicy} is used
+	 * else {@link org.springframework.retry.backoff.ExponentialBackOffPolicy} is used.
 	 * @return the flag to signal randomization is required (default false)
 	 */
 	String randomExpression() default "";

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

@@ -68,6 +68,7 @@
  * load concurrent access.
  *
  * @author Tomaz Fernandes
+ * @author Aftab Shaikh
  * @since 1.3.3
  */
 public class BackOffPolicyBuilder {
@@ -217,7 +218,7 @@ public BackOffPolicyBuilder randomSupplier(Supplier<Boolean> randomSupplier) {
 	public BackOffPolicy build() {
 		if (this.multiplier != null && this.multiplier > 0 || this.multiplierSupplier != null) {
 			ExponentialBackOffPolicy policy;
-			if (Boolean.TRUE.equals(this.random)) {
+			if (isRandom()) {
 				policy = new ExponentialRandomBackOffPolicy();
 			}
 			else {
@@ -279,4 +280,9 @@ else if (this.delay != null) {
 		return policy;
 	}
 
+	private boolean isRandom() {
+		return (this.randomSupplier != null && Boolean.TRUE.equals(this.randomSupplier.get()))
+				|| Boolean.TRUE.equals(this.random);
+	}
+
 }

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

@@ -26,6 +26,7 @@
 /**
  * @author Tomaz Fernandes
  * @author Gary Russell
+ * @author Aftab Shaikh
  * @since 1.3.3
  */
 public class BackOffPolicyBuilderTests {
@@ -109,4 +110,77 @@ public void shouldCreateExponentialRandomBackOff() {
 		assertThat(new DirectFieldAccessor(policy).getPropertyValue("sleeper")).isEqualTo(mockSleeper);
 	}
 
+	@Test
+	public void shouldCreateExponentialRandomBackOffWhenProvidedRandomSupplier() {
+		Sleeper mockSleeper = mock(Sleeper.class);
+		BackOffPolicy backOffPolicy = BackOffPolicyBuilder.newBuilder()
+			.delay(10000)
+			.maxDelay(100000)
+			.multiplier(10)
+			.randomSupplier(() -> true)
+			.sleeper(mockSleeper)
+			.build();
+
+		assertThat(ExponentialRandomBackOffPolicy.class.isAssignableFrom(backOffPolicy.getClass())).isTrue();
+		ExponentialRandomBackOffPolicy policy = (ExponentialRandomBackOffPolicy) backOffPolicy;
+		assertThat(policy.getInitialInterval()).isEqualTo(10000);
+		assertThat(policy.getMaxInterval()).isEqualTo(100000);
+		assertThat(policy.getMultiplier()).isEqualTo(10);
+	}
+
+	@Test
+	public void shouldCreateExponentialRandomBackOffWithProvidedSuppliers() {
+		Sleeper mockSleeper = mock(Sleeper.class);
+		BackOffPolicy backOffPolicy = BackOffPolicyBuilder.newBuilder()
+			.delaySupplier(() -> 10000L)
+			.maxDelaySupplier(() -> 100000L)
+			.multiplierSupplier(() -> 10d)
+			.randomSupplier(() -> true)
+			.sleeper(mockSleeper)
+			.build();
+
+		assertThat(ExponentialRandomBackOffPolicy.class.isAssignableFrom(backOffPolicy.getClass())).isTrue();
+		ExponentialRandomBackOffPolicy policy = (ExponentialRandomBackOffPolicy) backOffPolicy;
+		assertThat(policy.getInitialInterval()).isEqualTo(10000);
+		assertThat(policy.getMaxInterval()).isEqualTo(100000);
+		assertThat(policy.getMultiplier()).isEqualTo(10);
+	}
+
+	@Test
+	public void shouldCreateExponentialBackOffWhenProvidedRandomSupplier() {
+		Sleeper mockSleeper = mock(Sleeper.class);
+		BackOffPolicy backOffPolicy = BackOffPolicyBuilder.newBuilder()
+			.delay(100)
+			.maxDelay(1000)
+			.multiplier(2)
+			.randomSupplier(() -> false)
+			.sleeper(mockSleeper)
+			.build();
+		assertThat(backOffPolicy instanceof ExponentialRandomBackOffPolicy).isFalse();
+		assertThat(ExponentialBackOffPolicy.class.isAssignableFrom(backOffPolicy.getClass())).isTrue();
+		ExponentialBackOffPolicy policy = (ExponentialBackOffPolicy) backOffPolicy;
+		assertThat(policy.getInitialInterval()).isEqualTo(100);
+		assertThat(policy.getMaxInterval()).isEqualTo(1000);
+		assertThat(policy.getMultiplier()).isEqualTo(2);
+	}
+
+	@Test
+	public void shouldCreateExponentialBackOffWithProvidedSuppliers() {
+		Sleeper mockSleeper = mock(Sleeper.class);
+		BackOffPolicy backOffPolicy = BackOffPolicyBuilder.newBuilder()
+			.delaySupplier(() -> 10000L)
+			.maxDelaySupplier(() -> 100000L)
+			.multiplierSupplier(() -> 10d)
+			.randomSupplier(() -> false)
+			.sleeper(mockSleeper)
+			.build();
+
+		assertThat(backOffPolicy instanceof ExponentialRandomBackOffPolicy).isFalse();
+		assertThat(ExponentialBackOffPolicy.class.isAssignableFrom(backOffPolicy.getClass())).isTrue();
+		ExponentialBackOffPolicy policy = (ExponentialBackOffPolicy) backOffPolicy;
+		assertThat(policy.getInitialInterval()).isEqualTo(10000);
+		assertThat(policy.getMaxInterval()).isEqualTo(100000);
+		assertThat(policy.getMultiplier()).isEqualTo(10);
+	}
+
 }