Add warnings for invalid setter values in ExponentialBackoffPolicy

- Add warning logs when setter values don't meet expected constraints
- Maintain backward compatibility by not changing behavior

Fixes #352

Signed-off-by: kssumin <[email protected]>

Author: kssumin

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

@@ -130,6 +130,9 @@ protected void cloneValues(ExponentialBackOffPolicy target) {
 	 * @param initialInterval the initial interval
 	 */
 	public void setInitialInterval(long initialInterval) {
+		if (initialInterval < 1) {
+			logger.warn("Initial interval must be at least 1, but was " + initialInterval);
+		}
 		this.initialInterval = initialInterval > 1 ? initialInterval : 1;
 	}
 
@@ -139,6 +142,9 @@ public void setInitialInterval(long initialInterval) {
 	 * @param multiplier the multiplier
 	 */
 	public void setMultiplier(double multiplier) {
+		if (multiplier <= 1.0) {
+			logger.warn("Multiplier must be > 1.0 for effective exponential backoff, but was " + multiplier);
+		}
 		this.multiplier = multiplier > 1.0 ? multiplier : 1.0;
 	}
 
@@ -150,6 +156,9 @@ public void setMultiplier(double multiplier) {
 	 * @param maxInterval in milliseconds.
 	 */
 	public void setMaxInterval(long maxInterval) {
+		if (maxInterval < 1) {
+			logger.warn("Max interval must be positive, but was " + maxInterval);
+		}
 		this.maxInterval = maxInterval > 0 ? maxInterval : 1;
 	}
 

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

@@ -16,10 +16,13 @@
 
 package org.springframework.retry.backoff;
 
+import org.apache.commons.logging.Log;
 import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
+import org.springframework.beans.DirectFieldAccessor;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import static org.mockito.Mockito.*;
 
 /**
  * @author Rob Harrop
@@ -80,49 +83,49 @@ public void testMaximumBackOff() {
 	}
 
 	@Test
-	public void testMultiBackOff() {
+	public void testSetMultiplierWithWarning() {
+		Log logMock = mock(Log.class);
 		ExponentialBackOffPolicy strategy = new ExponentialBackOffPolicy();
-		long seed = 40;
-		double multiplier = 1.2;
-		strategy.setInitialInterval(seed);
-		strategy.setMultiplier(multiplier);
-		strategy.setSleeper(sleeper);
-		BackOffContext context = strategy.start(null);
-		for (int x = 0; x < 5; x++) {
-			strategy.backOff(context);
-			assertThat(sleeper.getLastBackOff()).isEqualTo(seed);
-			seed *= multiplier;
-		}
+
+		DirectFieldAccessor accessor = new DirectFieldAccessor(strategy);
+		accessor.setPropertyValue("logger", logMock);
+
+		strategy.setMultiplier(1.0);
+
+		ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
+		verify(logMock).warn(captor.capture());
+		assertThat(captor.getValue())
+			.isEqualTo("Multiplier must be > 1.0 for effective exponential backoff, but was 1.0");
 	}
 
 	@Test
-	public void testMultiBackOffWithInitialDelaySupplier() {
+	public void testSetInitialIntervalWithWarning() {
+		Log logMock = mock(Log.class);
 		ExponentialBackOffPolicy strategy = new ExponentialBackOffPolicy();
-		long seed = 40;
-		double multiplier = 1.2;
-		strategy.initialIntervalSupplier(() -> 40L);
-		strategy.setMultiplier(multiplier);
-		strategy.setSleeper(sleeper);
-		BackOffContext context = strategy.start(null);
-		for (int x = 0; x < 5; x++) {
-			strategy.backOff(context);
-			assertThat(sleeper.getLastBackOff()).isEqualTo(seed);
-			seed *= multiplier;
-		}
+
+		DirectFieldAccessor accessor = new DirectFieldAccessor(strategy);
+		accessor.setPropertyValue("logger", logMock);
+
+		strategy.setInitialInterval(0);
+
+		ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
+		verify(logMock).warn(captor.capture());
+		assertThat(captor.getValue()).isEqualTo("Initial interval must be at least 1, but was 0");
 	}
 
 	@Test
-	public void testInterruptedStatusIsRestored() {
+	public void testSetMaxIntervalWithWarning() {
+		Log logMock = mock(Log.class);
 		ExponentialBackOffPolicy strategy = new ExponentialBackOffPolicy();
-		strategy.setSleeper(new Sleeper() {
-			@Override
-			public void sleep(long backOffPeriod) throws InterruptedException {
-				throw new InterruptedException("foo");
-			}
-		});
-		BackOffContext context = strategy.start(null);
-		assertThatExceptionOfType(BackOffInterruptedException.class).isThrownBy(() -> strategy.backOff(context));
-		assertThat(Thread.interrupted()).isTrue();
+
+		DirectFieldAccessor accessor = new DirectFieldAccessor(strategy);
+		accessor.setPropertyValue("logger", logMock);
+
+		strategy.setMaxInterval(0);
+
+		ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
+		verify(logMock).warn(captor.capture());
+		assertThat(captor.getValue()).isEqualTo("Max interval must be positive, but was 0");
 	}
 
 }