Skip to content

Commit 0ab5470

Browse files
committed
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]>
1 parent 47a2359 commit 0ab5470

File tree

2 files changed

+59
-0
lines changed

2 files changed

+59
-0
lines changed

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,9 @@ protected void cloneValues(ExponentialBackOffPolicy target) {
130130
* @param initialInterval the initial interval
131131
*/
132132
public void setInitialInterval(long initialInterval) {
133+
if (initialInterval < 1) {
134+
logger.warn("Initial interval must be at least 1, but was " + initialInterval);
135+
}
133136
this.initialInterval = initialInterval > 1 ? initialInterval : 1;
134137
}
135138

@@ -139,6 +142,9 @@ public void setInitialInterval(long initialInterval) {
139142
* @param multiplier the multiplier
140143
*/
141144
public void setMultiplier(double multiplier) {
145+
if (multiplier <= 1.0) {
146+
logger.warn("Multiplier must be > 1.0 for effective exponential backoff, but was " + multiplier);
147+
}
142148
this.multiplier = multiplier > 1.0 ? multiplier : 1.0;
143149
}
144150

@@ -150,6 +156,9 @@ public void setMultiplier(double multiplier) {
150156
* @param maxInterval in milliseconds.
151157
*/
152158
public void setMaxInterval(long maxInterval) {
159+
if (maxInterval < 1) {
160+
logger.warn("Max interval must be positive, but was " + maxInterval);
161+
}
153162
this.maxInterval = maxInterval > 0 ? maxInterval : 1;
154163
}
155164

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,15 @@
1616

1717
package org.springframework.retry.backoff;
1818

19+
import org.apache.commons.logging.Log;
1920
import org.junit.jupiter.api.Test;
21+
import org.mockito.ArgumentCaptor;
22+
import org.springframework.beans.DirectFieldAccessor;
2023

2124
import static org.assertj.core.api.Assertions.assertThat;
2225
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
26+
import static org.mockito.Mockito.mock;
27+
import static org.mockito.Mockito.verify;
2328

2429
/**
2530
* @author Rob Harrop
@@ -125,4 +130,49 @@ public void sleep(long backOffPeriod) throws InterruptedException {
125130
assertThat(Thread.interrupted()).isTrue();
126131
}
127132

133+
@Test
134+
public void testSetMultiplierWithWarning() {
135+
Log logMock = mock(Log.class);
136+
ExponentialBackOffPolicy strategy = new ExponentialBackOffPolicy();
137+
138+
DirectFieldAccessor accessor = new DirectFieldAccessor(strategy);
139+
accessor.setPropertyValue("logger", logMock);
140+
141+
strategy.setMultiplier(1.0);
142+
143+
ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
144+
verify(logMock).warn(captor.capture());
145+
assertThat(captor.getValue())
146+
.isEqualTo("Multiplier must be > 1.0 for effective exponential backoff, but was 1.0");
147+
}
148+
149+
@Test
150+
public void testSetInitialIntervalWithWarning() {
151+
Log logMock = mock(Log.class);
152+
ExponentialBackOffPolicy strategy = new ExponentialBackOffPolicy();
153+
154+
DirectFieldAccessor accessor = new DirectFieldAccessor(strategy);
155+
accessor.setPropertyValue("logger", logMock);
156+
157+
strategy.setInitialInterval(0);
158+
159+
ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
160+
verify(logMock).warn(captor.capture());
161+
assertThat(captor.getValue()).isEqualTo("Initial interval must be at least 1, but was 0");
162+
}
163+
164+
@Test
165+
public void testSetMaxIntervalWithWarning() {
166+
Log logMock = mock(Log.class);
167+
ExponentialBackOffPolicy strategy = new ExponentialBackOffPolicy();
168+
169+
DirectFieldAccessor accessor = new DirectFieldAccessor(strategy);
170+
accessor.setPropertyValue("logger", logMock);
171+
172+
strategy.setMaxInterval(0);
173+
174+
ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
175+
verify(logMock).warn(captor.capture());
176+
assertThat(captor.getValue()).isEqualTo("Max interval must be positive, but was 0");
177+
}
128178
}

0 commit comments

Comments
 (0)