diff --git a/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java b/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java index 74c69d9..454655b 100644 --- a/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java +++ b/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java @@ -78,35 +78,27 @@ public class RecoverAnnotationRecoveryHandler implements MethodInvocationReco } SimpleMetadata meta = this.methods.get(method); Object[] argsToUse = meta.getArgs(cause, args); - boolean methodAccessible = method.isAccessible(); - try { - ReflectionUtils.makeAccessible(method); - RetryContext context = RetrySynchronizationManager.getContext(); - Object proxy = null; - if (context != null) { - proxy = context.getAttribute("___proxy___"); - if (proxy != null) { - Method proxyMethod = findMethodOnProxy(method, proxy); - if (proxyMethod == null) { - proxy = null; - } - else { - method = proxyMethod; - } + ReflectionUtils.makeAccessible(method); + RetryContext context = RetrySynchronizationManager.getContext(); + Object proxy = null; + if (context != null) { + proxy = context.getAttribute("___proxy___"); + if (proxy != null) { + Method proxyMethod = findMethodOnProxy(method, proxy); + if (proxyMethod == null) { + proxy = null; + } + else { + method = proxyMethod; } } - if (proxy == null) { - proxy = this.target; - } - @SuppressWarnings("unchecked") - T result = (T) ReflectionUtils.invokeMethod(method, proxy, argsToUse); - return result; } - finally { - if (methodAccessible != method.isAccessible()) { - method.setAccessible(methodAccessible); - } + if (proxy == null) { + proxy = this.target; } + @SuppressWarnings("unchecked") + T result = (T) ReflectionUtils.invokeMethod(method, proxy, argsToUse); + return result; } private Method findMethodOnProxy(Method method, Object proxy) { @@ -121,7 +113,7 @@ public class RecoverAnnotationRecoveryHandler implements MethodInvocationReco private Method findClosestMatch(Object[] args, Class cause) { Method result = null; - if (StringUtils.isEmpty(this.recoverMethodName)) { + if (!StringUtils.hasText(this.recoverMethodName)) { int min = Integer.MAX_VALUE; for (Map.Entry entry : this.methods.entrySet()) { Method method = entry.getKey(); diff --git a/src/main/java/org/springframework/retry/policy/SimpleRetryPolicy.java b/src/main/java/org/springframework/retry/policy/SimpleRetryPolicy.java index 5470916..3916cc2 100644 --- a/src/main/java/org/springframework/retry/policy/SimpleRetryPolicy.java +++ b/src/main/java/org/springframework/retry/policy/SimpleRetryPolicy.java @@ -38,7 +38,7 @@ import org.springframework.util.ClassUtils; * * * will execute the callback at least once, and as many as 3 times. - * + *

* Since version 1.3 it is not necessary to use this class. The same behaviour can be * achieved by constructing a {@link CompositeRetryPolicy} with * {@link MaxAttemptsRetryPolicy} and {@link BinaryExceptionClassifierRetryPolicy} inside, @@ -57,6 +57,7 @@ import org.springframework.util.ClassUtils; * @author Rob Harrop * @author Gary Russell * @author Aleksandr Shamukov + * @author Artem Bilan */ @SuppressWarnings("serial") public class SimpleRetryPolicy implements RetryPolicy { @@ -70,7 +71,7 @@ public class SimpleRetryPolicy implements RetryPolicy { private Supplier maxAttemptsSupplier; - private BinaryExceptionClassifier retryableClassifier = new BinaryExceptionClassifier(false); + private BinaryExceptionClassifier retryableClassifier; private BinaryExceptionClassifier recoverableClassifier = new BinaryExceptionClassifier(Collections.emptyMap(), true, true); @@ -164,6 +165,7 @@ public class SimpleRetryPolicy implements RetryPolicy { * @param noRecovery the throwables. * @since 3.0 */ + @SuppressWarnings("unchecked") public void setNotRecoverable(Class... noRecovery) { Map, Boolean> map = new HashMap<>(); for (Class clazz : noRecovery) { @@ -258,7 +260,7 @@ public class SimpleRetryPolicy implements RetryPolicy { /** * Delegates to an exception classifier. - * @param ex + * @param ex the exception to classify. * @return true if this exception or its ancestors have been registered as retryable. */ private boolean retryForException(Throwable ex) { diff --git a/src/test/java/org/springframework/retry/annotation/CircuitBreakerTests.java b/src/test/java/org/springframework/retry/annotation/CircuitBreakerTests.java index 6117d34..b6236ce 100644 --- a/src/test/java/org/springframework/retry/annotation/CircuitBreakerTests.java +++ b/src/test/java/org/springframework/retry/annotation/CircuitBreakerTests.java @@ -40,6 +40,7 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; /** * @author Dave Syer * @author Gary Russell + * @author Artem Bilan * */ public class CircuitBreakerTests { @@ -176,8 +177,7 @@ public class CircuitBreakerTests { @Override @CircuitBreaker(maxAttemptsExpression = "#{2 * ${foo:4}}", openTimeoutExpression = "#{${bar:19}000}", - resetTimeoutExpression = "#{${baz:20}000}", - exceptionExpression = "#{#root instanceof RuntimeExpression}") + resetTimeoutExpression = "#{${baz:20}000}", exceptionExpression = "#root instanceof RuntimeExpression") public void expressionService() { this.count++; } diff --git a/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java b/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java index de1aac8..2be01c4 100644 --- a/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java +++ b/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java @@ -584,14 +584,14 @@ public class EnableRetryTests { private int count = 0; - @Retryable(RuntimeException.class) + @Retryable(retryFor = RuntimeException.class) public void service() { if (this.count++ < 2) { throw new RuntimeException("Planned"); } } - @Retryable(RuntimeException.class) + @Retryable(retryFor = RuntimeException.class) public void other() { if (this.count++ < 3) { throw new RuntimeException("Other"); @@ -655,7 +655,7 @@ public class EnableRetryTests { } - @Retryable(RuntimeException.class) + @Retryable(retryFor = RuntimeException.class) protected static class RetryableService { private int count = 0; diff --git a/src/test/java/org/springframework/retry/listener/RetryListenerSupportTests.java b/src/test/java/org/springframework/retry/listener/RetryListenerSupportTests.java deleted file mode 100644 index a2f2cbd..0000000 --- a/src/test/java/org/springframework/retry/listener/RetryListenerSupportTests.java +++ /dev/null @@ -1,50 +0,0 @@ -/* - * Copyright 2006-2022 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.retry.listener; - -import org.junit.jupiter.api.Test; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatNoException; - -/** - * @author Dave Syer - * @author Gary Russell - * @author Henning Pöttker - */ -@SuppressWarnings("deprecation") -public class RetryListenerSupportTests { - - @Test - public void testClose() { - RetryListenerSupport support = new RetryListenerSupport(); - assertThatNoException().isThrownBy(() -> support.close(null, null, null)); - } - - @Test - public void testOnError() { - RetryListenerSupport support = new RetryListenerSupport(); - assertThatNoException().isThrownBy(() -> support.onError(null, null, null)); - } - - @Test - public void testOpen() { - RetryListenerSupport support = new RetryListenerSupport(); - assertThat(support.open(null, null)).isTrue(); - } - -} diff --git a/src/test/java/org/springframework/retry/listener/RetryListenerTests.java b/src/test/java/org/springframework/retry/listener/RetryListenerTests.java index b195792..7d9f8a2 100644 --- a/src/test/java/org/springframework/retry/listener/RetryListenerTests.java +++ b/src/test/java/org/springframework/retry/listener/RetryListenerTests.java @@ -38,6 +38,7 @@ import static org.assertj.core.api.Assertions.assertThatNoException; * @author Stéphane Nicoll * @author Gary Russell * @author Henning Pöttker + * @author Artem Bilan */ public class RetryListenerTests { @@ -47,6 +48,27 @@ public class RetryListenerTests { List list = new ArrayList<>(); + @Test + public void testClose() { + RetryListener retryListener = new RetryListener() { + }; + assertThatNoException().isThrownBy(() -> retryListener.close(null, null, null)); + } + + @Test + public void noExceptionOnError() { + RetryListener retryListener = new RetryListener() { + }; + assertThatNoException().isThrownBy(() -> retryListener.onError(null, null, null)); + } + + @Test + public void testOpen() { + RetryListener retryListener = new RetryListener() { + }; + assertThat(retryListener.open(null, null)).isTrue(); + } + @Test public void testOpenDefaultImplementation() { var retryListener = new RetryListener() { diff --git a/src/test/java/org/springframework/retry/policy/SerializedMapRetryContextCache.java b/src/test/java/org/springframework/retry/policy/SerializedMapRetryContextCache.java index 0252acb..2baf62b 100644 --- a/src/test/java/org/springframework/retry/policy/SerializedMapRetryContextCache.java +++ b/src/test/java/org/springframework/retry/policy/SerializedMapRetryContextCache.java @@ -15,6 +15,9 @@ */ package org.springframework.retry.policy; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.ObjectInputStream; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -36,7 +39,15 @@ public class SerializedMapRetryContextCache implements RetryContextCache { @Override public RetryContext get(Object key) { byte[] bytes = map.get(key); - return (RetryContext) SerializationUtils.deserialize(bytes); + try (ObjectInputStream ois = new ObjectInputStream(new ByteArrayInputStream(bytes))) { + return (RetryContext) ois.readObject(); + } + catch (IOException ex) { + throw new IllegalArgumentException("Failed to deserialize object", ex); + } + catch (ClassNotFoundException ex) { + throw new IllegalStateException("Failed to deserialize object type", ex); + } } @Override diff --git a/src/test/java/org/springframework/retry/stats/CircuitBreakerStatisticsTests.java b/src/test/java/org/springframework/retry/stats/CircuitBreakerStatisticsTests.java index be4d354..99fd077 100644 --- a/src/test/java/org/springframework/retry/stats/CircuitBreakerStatisticsTests.java +++ b/src/test/java/org/springframework/retry/stats/CircuitBreakerStatisticsTests.java @@ -39,6 +39,7 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; /** * @author Dave Syer * @author Gary Russell + * @author Artem Bilan * */ public class CircuitBreakerStatisticsTests { @@ -83,9 +84,8 @@ public class CircuitBreakerStatisticsTests { assertThat(result).isEqualTo(RECOVERED); result = this.retryTemplate.execute(this.callback, this.recovery, this.state); assertThat(result).isEqualTo(RECOVERED); - assertThat(stats.getRecoveryCount()).describedAs("There should be two recoveries", null).isEqualTo(2); - assertThat(stats.getErrorCount()) - .describedAs("There should only be one error because the circuit is now open", null) + assertThat(stats.getRecoveryCount()).describedAs("There should be two recoveries").isEqualTo(2); + assertThat(stats.getErrorCount()).describedAs("There should only be one error because the circuit is now open") .isEqualTo(1); assertThat(stats.getAttribute(CircuitBreakerRetryPolicy.CIRCUIT_OPEN)).isEqualTo(Boolean.TRUE); // Both recoveries are through a short circuit because we used NeverRetryPolicy @@ -94,7 +94,7 @@ public class CircuitBreakerStatisticsTests { } @Test - public void testFailedRecoveryCountsAsAbort() throws Throwable { + public void testFailedRecoveryCountsAsAbort() { this.retryTemplate.setRetryPolicy(new CircuitBreakerRetryPolicy(new NeverRetryPolicy())); this.recovery = context -> { throw new ExhaustedRetryException("Planned exhausted"); @@ -123,8 +123,7 @@ public class CircuitBreakerStatisticsTests { } MutableRetryStatistics stats = (MutableRetryStatistics) repository.findOne("test"); assertThat(stats.getAbortCount()).describedAs("There should be two aborts").isEqualTo(2); - assertThat(stats.getErrorCount()) - .describedAs("There should only be one error because the circuit is now open", null) + assertThat(stats.getErrorCount()).describedAs("There should only be one error because the circuit is now open") .isEqualTo(1); assertThat(stats.getAttribute(CircuitBreakerRetryPolicy.CIRCUIT_OPEN)).isEqualTo(true); resetAndAssert(this.cache, stats);