From 4886b75d0b09260348c5480cde33d386d8d48285 Mon Sep 17 00:00:00 2001 From: Tobias Soloschenko Date: Thu, 19 Sep 2024 17:24:41 +0200 Subject: [PATCH] feat: setter for Log of RetryTemplate (#471) GH-470: Add `RetryTemplate.setLogger()` to avoid reflection in other places Fixes: #470 Issue link: https://github.com/spring-projects/spring-retry/issues/470 Spring Cloud Config does mutation in the `RetryTemplate` for its system loading logger via `RetryTemplateFactory`. * Expose setter for `logger` property to avoid reflection. * Add `RetryTemplateBuilder.withLogger()` for convenience --- .../retry/support/RetryTemplate.java | 15 ++++++++++- .../retry/support/RetryTemplateBuilder.java | 27 +++++++++++++++++++ .../support/RetryTemplateBuilderTests.java | 17 ++++++++++++ .../retry/support/RetryTemplateTests.java | 19 +++++++++++-- 4 files changed, 75 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/springframework/retry/support/RetryTemplate.java b/src/main/java/org/springframework/retry/support/RetryTemplate.java index c02385d..4b529b9 100644 --- a/src/main/java/org/springframework/retry/support/RetryTemplate.java +++ b/src/main/java/org/springframework/retry/support/RetryTemplate.java @@ -78,6 +78,7 @@ import org.springframework.util.Assert; * @author Josh Long * @author Aleksandr Shamukov * @author Emanuele Ivaldi + * @author Tobias Soloschenko */ public class RetryTemplate implements RetryOperations { @@ -87,7 +88,7 @@ public class RetryTemplate implements RetryOperations { */ private static final String GLOBAL_STATE = "state.global"; - protected final Log logger = LogFactory.getLog(getClass()); + protected Log logger = LogFactory.getLog(getClass()); private volatile BackOffPolicy backOffPolicy = new NoBackOffPolicy(); @@ -186,6 +187,18 @@ public class RetryTemplate implements RetryOperations { return this.listeners.length > 0; } + /** + * Setter for {@link Log}. If not applied the following is used: + *

+ * {@code LogFactory.getLog(getClass())} + *

+ * @param logger the logger the retry template uses for logging + * @since 2.0.10 + */ + public void setLogger(Log logger) { + this.logger = logger; + } + /** * Setter for {@link BackOffPolicy}. * @param backOffPolicy the {@link BackOffPolicy} diff --git a/src/main/java/org/springframework/retry/support/RetryTemplateBuilder.java b/src/main/java/org/springframework/retry/support/RetryTemplateBuilder.java index 17f4423..ef24a4d 100644 --- a/src/main/java/org/springframework/retry/support/RetryTemplateBuilder.java +++ b/src/main/java/org/springframework/retry/support/RetryTemplateBuilder.java @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.List; import java.util.function.Predicate; +import org.apache.commons.logging.Log; import org.springframework.classify.BinaryExceptionClassifier; import org.springframework.classify.BinaryExceptionClassifierBuilder; import org.springframework.retry.RetryListener; @@ -79,12 +80,15 @@ import org.springframework.util.Assert; * @author Kim In Hoi * @author Andreas Ahlenstorf * @author Morulai Planinski + * @author Tobias Soloschenko * @since 1.3 */ public class RetryTemplateBuilder { private RetryPolicy baseRetryPolicy; + private Log logger; + private BackOffPolicy backOffPolicy; private List listeners; @@ -288,6 +292,23 @@ public class RetryTemplateBuilder { return this.exponentialBackoff(initialInterval.toMillis(), multiplier, maxInterval.toMillis(), withRandom); } + /** + * Applies a dedicated logger to the {@link RetryTemplate}. If not applied the + * following is used: + *

+ * {@code LogFactory.getLog(getClass())} + *

+ * @param logger the logger which should be used for logging + * @return this + * @since 2.0.10 + */ + public RetryTemplateBuilder withLogger(Log logger) { + Assert.isNull(this.logger, "You have already applied a logger"); + Assert.notNull(logger, "The given logger should not be null"); + this.logger = logger; + return this; + } + /** * Perform each retry after a fixed amount of time. * @param interval fixed interval in milliseconds @@ -584,6 +605,12 @@ public class RetryTemplateBuilder { finalPolicy.setPolicies(new RetryPolicy[] { this.baseRetryPolicy, exceptionRetryPolicy }); retryTemplate.setRetryPolicy(finalPolicy); + // Logger + + if (this.logger != null) { + retryTemplate.setLogger(this.logger); + } + // Backoff policy if (this.backOffPolicy == null) { diff --git a/src/test/java/org/springframework/retry/support/RetryTemplateBuilderTests.java b/src/test/java/org/springframework/retry/support/RetryTemplateBuilderTests.java index eb29242..2fff335 100644 --- a/src/test/java/org/springframework/retry/support/RetryTemplateBuilderTests.java +++ b/src/test/java/org/springframework/retry/support/RetryTemplateBuilderTests.java @@ -24,6 +24,7 @@ import java.util.Collections; import java.util.List; import java.util.function.Predicate; +import org.apache.commons.logging.Log; import org.junit.jupiter.api.Test; import org.springframework.classify.BinaryExceptionClassifier; import org.springframework.retry.RetryListener; @@ -58,6 +59,7 @@ import static org.springframework.retry.util.test.TestUtils.getPropertyValue; * @author Gary Russell * @author Andreas Ahlenstorf * @author Morulai Planinski + * @author Tobias Soloschenko */ public class RetryTemplateBuilderTests { @@ -346,6 +348,21 @@ public class RetryTemplateBuilderTests { .isThrownBy(() -> RetryTemplate.builder().exponentialBackoff(0, 2, 200).build()); } + @Test + public void testBuilderWithLogger() { + Log logMock = mock(Log.class); + RetryTemplate retryTemplate = RetryTemplate.builder().withLogger(logMock).build(); + Log logger = getPropertyValue(retryTemplate, "logger", Log.class); + assertThat(logger).isEqualTo(logMock); + } + + @Test + public void testBuilderWithDefaultLogger() { + RetryTemplate retryTemplate = RetryTemplate.builder().build(); + Log logger = getPropertyValue(retryTemplate, "logger", Log.class); + assertThat(logger).isNotNull(); + } + /* ---------------- Utils -------------- */ private static class PolicyTuple { diff --git a/src/test/java/org/springframework/retry/support/RetryTemplateTests.java b/src/test/java/org/springframework/retry/support/RetryTemplateTests.java index 5030b12..94f024c 100644 --- a/src/test/java/org/springframework/retry/support/RetryTemplateTests.java +++ b/src/test/java/org/springframework/retry/support/RetryTemplateTests.java @@ -20,8 +20,10 @@ import java.util.Collections; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import org.apache.commons.logging.Log; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; import org.springframework.classify.BinaryExceptionClassifier; import org.springframework.retry.RetryCallback; import org.springframework.retry.RetryContext; @@ -45,6 +47,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; /** * @author Rob Harrop @@ -53,6 +56,7 @@ import static org.mockito.Mockito.verify; * @author Henning Pöttker * @author Emanuele Ivaldi * @author Morulai Planinski + * @author Tobias Soloschenko */ public class RetryTemplateTests { @@ -286,7 +290,6 @@ public class RetryTemplateTests { } } - @SuppressWarnings("serial") @Test public void testFailedPolicy() { RetryTemplate retryTemplate = new RetryTemplate(); @@ -325,7 +328,6 @@ public class RetryTemplateTests { tested.setRetryPolicy(new SimpleRetryPolicy(1)); BackOffPolicy bop = mock(BackOffPolicy.class); - @SuppressWarnings("serial") BackOffContext backOffContext = new BackOffContext() { }; tested.setBackOffPolicy(bop); @@ -439,4 +441,17 @@ public class RetryTemplateTests { } + @Test + public void testLoggingAppliedCorrectly() throws Exception { + ArgumentCaptor logOutputCaptor = ArgumentCaptor.forClass(String.class); + RetryTemplate retryTemplate = new RetryTemplate(); + Log logMock = mock(Log.class); + when(logMock.isTraceEnabled()).thenReturn(false); + when(logMock.isDebugEnabled()).thenReturn(true); + retryTemplate.setLogger(logMock); + retryTemplate.execute(new MockRetryCallback()); + verify(logMock).debug(logOutputCaptor.capture()); + assertThat(logOutputCaptor.getValue()).contains("Retry: count=0"); + } + }