Skip to content

Commit a05e3a1

Browse files
authored
GH-352: Warning aboiut invalid 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: Kim Sumin <ksoomin25@gmail.com>
1 parent 47a2359 commit a05e3a1

File tree

2 files changed

+62
-0
lines changed

2 files changed

+62
-0
lines changed

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
* @author Artem Bilan
4444
* @author Marius Lichtblau
4545
* @author Anton Aharkau
46+
* @author Kim Sumin
4647
*/
4748
@SuppressWarnings("serial")
4849
public class ExponentialBackOffPolicy implements SleepingBackOffPolicy<ExponentialBackOffPolicy> {
@@ -130,6 +131,9 @@ protected void cloneValues(ExponentialBackOffPolicy target) {
130131
* @param initialInterval the initial interval
131132
*/
132133
public void setInitialInterval(long initialInterval) {
134+
if (initialInterval < 1) {
135+
logger.warn("Initial interval must be at least 1, but was " + initialInterval);
136+
}
133137
this.initialInterval = initialInterval > 1 ? initialInterval : 1;
134138
}
135139

@@ -139,6 +143,9 @@ public void setInitialInterval(long initialInterval) {
139143
* @param multiplier the multiplier
140144
*/
141145
public void setMultiplier(double multiplier) {
146+
if (multiplier <= 1.0) {
147+
logger.warn("Multiplier must be > 1.0 for effective exponential backoff, but was " + multiplier);
148+
}
142149
this.multiplier = multiplier > 1.0 ? multiplier : 1.0;
143150
}
144151

@@ -150,6 +157,9 @@ public void setMultiplier(double multiplier) {
150157
* @param maxInterval in milliseconds.
151158
*/
152159
public void setMaxInterval(long maxInterval) {
160+
if (maxInterval < 1) {
161+
logger.warn("Max interval must be positive, but was " + maxInterval);
162+
}
153163
this.maxInterval = maxInterval > 0 ? maxInterval : 1;
154164
}
155165

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,23 @@
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
2631
* @author Dave Syer
2732
* @author Gary Russell
2833
* @author Marius Lichtblau
2934
* @author Anton Aharkau
35+
* @author Kim Sumin
3036
*/
3137
public class ExponentialBackOffPolicyTests {
3238

@@ -125,4 +131,50 @@ public void sleep(long backOffPeriod) throws InterruptedException {
125131
assertThat(Thread.interrupted()).isTrue();
126132
}
127133

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

0 commit comments

Comments
 (0)