diff --git a/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java b/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java index 0b9ee75..a06b960 100644 --- a/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java +++ b/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java @@ -217,16 +217,13 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn RetryTemplate template = createTemplate(retryable.listeners()); template.setRetryPolicy(getRetryPolicy(retryable)); template.setBackOffPolicy(getBackoffPolicy(retryable.backoff())); - template.setNoRecoveryForNotRetryable(retryable.rethrow()); return RetryInterceptorBuilder.stateless().retryOperations(template).label(retryable.label()) - .recoverer(getRecoverer(target, method, retryable.rethrow())).build(); + .recoverer(getRecoverer(target, method)).build(); } private MethodInterceptor getStatefulInterceptor(Object target, Method method, Retryable retryable) { - boolean rethrow = retryable.rethrow(); RetryTemplate template = createTemplate(retryable.listeners()); template.setRetryContextCache(this.retryContextCache); - template.setNoRecoveryForNotRetryable(rethrow); CircuitBreaker circuit = AnnotatedElementUtils.findMergedAnnotation(method, CircuitBreaker.class); if (circuit == null) { @@ -244,7 +241,7 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn label = method.toGenericString(); } return RetryInterceptorBuilder.circuitBreaker().keyGenerator(new FixedKeyGenerator("circuit")) - .retryOperations(template).recoverer(getRecoverer(target, method, rethrow)).label(label).build(); + .retryOperations(template).recoverer(getRecoverer(target, method)).label(label).build(); } RetryPolicy policy = getRetryPolicy(retryable); template.setRetryPolicy(policy); @@ -252,7 +249,7 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn String label = retryable.label(); return RetryInterceptorBuilder.stateful().keyGenerator(this.methodArgumentsKeyGenerator) .newMethodArgumentsIdentifier(this.newMethodArgumentsIdentifier).retryOperations(template).label(label) - .recoverer(getRecoverer(target, method, rethrow)).build(); + .recoverer(getRecoverer(target, method)).build(); } private long getOpenTimeout(CircuitBreaker circuit) { @@ -296,7 +293,7 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn return listeners; } - private MethodInvocationRecoverer getRecoverer(Object target, Method method, boolean rethrow) { + private MethodInvocationRecoverer getRecoverer(Object target, Method method) { if (target instanceof MethodInvocationRecoverer) { return (MethodInvocationRecoverer) target; } @@ -313,9 +310,7 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn if (!foundRecoverable.get()) { return null; } - RecoverAnnotationRecoveryHandler recoveryHandler = new RecoverAnnotationRecoveryHandler(target, method); - recoveryHandler.setThrowLastExceptionWhenNoRecoverMethod(rethrow); - return recoveryHandler; + return new RecoverAnnotationRecoveryHandler(target, method); } private RetryPolicy getRetryPolicy(Annotation retryable) { diff --git a/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java b/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java index e4a8f16..dcbc7be 100644 --- a/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java +++ b/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2022 the original author or authors. + * Copyright 2013-2019 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. @@ -64,23 +64,16 @@ public class RecoverAnnotationRecoveryHandler implements MethodInvocationReco private String recoverMethodName; - private boolean throwLastExceptionWhenNoRecoverMethod; - public RecoverAnnotationRecoveryHandler(Object target, Method method) { this.target = target; init(target, method); } - public void setThrowLastExceptionWhenNoRecoverMethod(boolean throwLastExceptionWhenNoRecoverMethod) { - this.throwLastExceptionWhenNoRecoverMethod = throwLastExceptionWhenNoRecoverMethod; - } - @Override public T recover(Object[] args, Throwable cause) { Method method = findClosestMatch(args, cause.getClass()); if (method == null) { - throw throwLastExceptionWhenNoRecoverMethod && cause instanceof RuntimeException ? (RuntimeException) cause - : new ExhaustedRetryException("Cannot locate recovery method", cause); + throw new ExhaustedRetryException("Cannot locate recovery method", cause); } SimpleMetadata meta = this.methods.get(method); Object[] argsToUse = meta.getArgs(cause, args); diff --git a/src/main/java/org/springframework/retry/annotation/Retryable.java b/src/main/java/org/springframework/retry/annotation/Retryable.java index 543ea82..62c1d8f 100644 --- a/src/main/java/org/springframework/retry/annotation/Retryable.java +++ b/src/main/java/org/springframework/retry/annotation/Retryable.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2022 the original author or authors. + * Copyright 2014-2019 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. @@ -130,12 +130,4 @@ public @interface Retryable { */ String[] listeners() default {}; - /** - * When true raw exceptions are thrown without being wrapped and no recovery is - * performed for not-retryable exceptions. - * @return true if to rethrow raw exceptions, default false - * @since 1.3.3 - */ - boolean rethrow() default false; - } diff --git a/src/main/java/org/springframework/retry/policy/SimpleRetryPolicy.java b/src/main/java/org/springframework/retry/policy/SimpleRetryPolicy.java index b5b09bc..678c86f 100644 --- a/src/main/java/org/springframework/retry/policy/SimpleRetryPolicy.java +++ b/src/main/java/org/springframework/retry/policy/SimpleRetryPolicy.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2022 the original author or authors. + * Copyright 2006-2019 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. @@ -199,16 +199,6 @@ public class SimpleRetryPolicy implements RetryPolicy { return new SimpleRetryContext(parent); } - /** - * Delegates to an exception classifier. - * @param ex - * @return true if this exception or its ancestors have been registered as retryable. - * @since 1.3.3 - */ - public boolean retryForException(Throwable ex) { - return this.retryableClassifier.classify(ex); - } - private static class SimpleRetryContext extends RetryContextSupport { public SimpleRetryContext(RetryContext parent) { @@ -217,6 +207,15 @@ public class SimpleRetryPolicy implements RetryPolicy { } + /** + * Delegates to an exception classifier. + * @param ex + * @return true if this exception or its ancestors have been registered as retryable. + */ + private boolean retryForException(Throwable ex) { + return this.retryableClassifier.classify(ex); + } + @Override public String toString() { return ClassUtils.getShortName(getClass()) + "[maxAttempts=" + this.maxAttempts + "]"; diff --git a/src/main/java/org/springframework/retry/support/RetryTemplate.java b/src/main/java/org/springframework/retry/support/RetryTemplate.java index 0726221..8bf4bf4 100644 --- a/src/main/java/org/springframework/retry/support/RetryTemplate.java +++ b/src/main/java/org/springframework/retry/support/RetryTemplate.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2022 the original author or authors. + * Copyright 2006-2020 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. @@ -96,8 +96,6 @@ public class RetryTemplate implements RetryOperations { private boolean throwLastExceptionOnExhausted; - private boolean noRecoveryForNotRetryable; - /** * Main entry point to configure RetryTemplate using fluent API. See * {@link RetryTemplateBuilder} for usage examples and details. @@ -126,14 +124,6 @@ public class RetryTemplate implements RetryOperations { this.throwLastExceptionOnExhausted = throwLastExceptionOnExhausted; } - /** - * @param noRecoveryForNotRetryable the noRecoveryForNotRetryable to set - * @since 1.3.3 - */ - public void setNoRecoveryForNotRetryable(boolean noRecoveryForNotRetryable) { - this.noRecoveryForNotRetryable = noRecoveryForNotRetryable; - } - /** * Public setter for the {@link RetryContextCache}. * @param retryContextCache the {@link RetryContextCache} to set. @@ -376,6 +366,7 @@ public class RetryTemplate implements RetryOperations { } throw RetryTemplate.wrapIfNecessary(e); } + } /* @@ -393,7 +384,7 @@ public class RetryTemplate implements RetryOperations { } exhausted = true; - return handleRetryExhausted(recoveryCallback, context, state, retryPolicy); + return handleRetryExhausted(recoveryCallback, context, state); } catch (Throwable e) { @@ -471,6 +462,7 @@ public class RetryTemplate implements RetryOperations { * was encountered */ protected RetryContext open(RetryPolicy retryPolicy, RetryState state) { + if (state == null) { return doOpenInternal(retryPolicy); } @@ -504,6 +496,7 @@ public class RetryTemplate implements RetryOperations { context.removeAttribute(RetryContext.EXHAUSTED); context.removeAttribute(RetryContext.RECOVERED); return context; + } private RetryContext doOpenInternal(RetryPolicy retryPolicy, RetryState state) { @@ -536,16 +529,12 @@ public class RetryTemplate implements RetryOperations { * @return T the payload to return * @throws Throwable if there is an error */ - protected T handleRetryExhausted(RecoveryCallback recoveryCallback, RetryContext context, RetryState state, - RetryPolicy retryPolicy) throws Throwable { + protected T handleRetryExhausted(RecoveryCallback recoveryCallback, RetryContext context, RetryState state) + throws Throwable { context.setAttribute(RetryContext.EXHAUSTED, true); if (state != null && !context.hasAttribute(GLOBAL_STATE)) { this.retryContextCache.remove(state.getKey()); } - if (this.noRecoveryForNotRetryable && retryPolicy instanceof SimpleRetryPolicy - && !((SimpleRetryPolicy) retryPolicy).retryForException(context.getLastThrowable())) { - throw context.getLastThrowable(); - } if (recoveryCallback != null) { T recovered = recoveryCallback.recover(context); context.setAttribute(RetryContext.RECOVERED, true); @@ -559,7 +548,7 @@ public class RetryTemplate implements RetryOperations { } protected void rethrow(RetryContext context, String message) throws E { - if (this.throwLastExceptionOnExhausted || this.noRecoveryForNotRetryable) { + if (this.throwLastExceptionOnExhausted) { @SuppressWarnings("unchecked") E rethrow = (E) context.getLastThrowable(); throw rethrow; @@ -583,6 +572,7 @@ public class RetryTemplate implements RetryOperations { } private boolean doOpenInterceptors(RetryCallback callback, RetryContext context) { + boolean result = true; for (RetryListener listener : this.listeners) { @@ -590,6 +580,7 @@ public class RetryTemplate implements RetryOperations { } return result; + } private void doCloseInterceptors(RetryCallback callback, RetryContext context, diff --git a/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java b/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java index b427bb5..7bff6ae 100644 --- a/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java +++ b/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2022 the original author or authors. + * Copyright 2014-2021 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. @@ -255,22 +255,6 @@ public class EnableRetryTests { context.close(); } - @Test - public void rethrow() { - AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(TestConfiguration.class); - RethrowService service = context.getBean(RethrowService.class); - for (int i = 0; i < 3; i++) { - try { - service.service(); - } - catch (RuntimeException e) { - assertEquals("Planned", e.getMessage()); - } - } - assertEquals(3, service.getCount()); - context.close(); - } - private Object target(Object target) { if (!AopUtils.isAopProxy(target)) { return target; @@ -450,11 +434,6 @@ public class EnableRetryTests { return new ExcludesOnlyService(); } - @Bean - public RethrowService rethrowService() { - return new RethrowService(); - } - @Bean public MethodInterceptor retryInterceptor() { return RetryInterceptorBuilder.stateless().maxAttempts(5).build(); @@ -717,23 +696,6 @@ public class EnableRetryTests { } - private static class RethrowService { - - private int count = 0; - - @Retryable(include = IllegalArgumentException.class, rethrow = true) - public void service() { - if (this.count++ < 2) { - throw new RuntimeException("Planned"); - } - } - - public int getCount() { - return this.count; - } - - } - public static class ExceptionChecker { public boolean shouldRetry(Throwable t) { diff --git a/src/test/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandlerTests.java b/src/test/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandlerTests.java index 0e8f369..ee4cf28 100644 --- a/src/test/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandlerTests.java +++ b/src/test/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandlerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2022 the original author or authors. + * Copyright 2013-2019 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. @@ -74,16 +74,7 @@ public class RecoverAnnotationRecoveryHandlerTests { RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler( new SpecificException(), ReflectionUtils.findMethod(SpecificException.class, "foo", String.class)); this.expected.expect(ExhaustedRetryException.class); - handler.recover(new Object[] { "Dave" }, new RuntimeException("Planned")); - } - - @Test - public void noMatchWithRethrow() { - RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler( - new SpecificException(), ReflectionUtils.findMethod(SpecificException.class, "foo", String.class)); - handler.setThrowLastExceptionWhenNoRecoverMethod(true); - this.expected.expect(IllegalArgumentException.class); - handler.recover(new Object[] { "Dave" }, new IllegalArgumentException("Planned")); + handler.recover(new Object[] { "Dave" }, new Error("Planned")); } @Test @@ -99,6 +90,7 @@ public class RecoverAnnotationRecoveryHandlerTests { RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler( new InAccessibleRecover(), foo); assertEquals(1, handler.recover(new Object[] { "Dave" }, new RuntimeException("Planned"))); + } @Test @@ -122,6 +114,7 @@ public class RecoverAnnotationRecoveryHandlerTests { @Test public void genericReturnStringValueTypeParentThrowableRecoverMethod() { + RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler>( new GenericReturnTypeRecover(), ReflectionUtils.findMethod(GenericReturnTypeRecover.class, "foo", String.class)); @@ -135,6 +128,7 @@ public class RecoverAnnotationRecoveryHandlerTests { @Test public void genericReturnStringValueTypeChildThrowableRecoverMethod() { + RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler>( new GenericReturnTypeRecover(), ReflectionUtils.findMethod(GenericReturnTypeRecover.class, "foo", String.class)); @@ -148,6 +142,7 @@ public class RecoverAnnotationRecoveryHandlerTests { @Test public void genericReturnOneValueTypeRecoverMethod() { + RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler>( new GenericReturnTypeRecover(), ReflectionUtils.findMethod(GenericReturnTypeRecover.class, "bar", String.class)); @@ -214,6 +209,7 @@ public class RecoverAnnotationRecoveryHandlerTests { Map>> recoverResponseMapRe = (Map>>) barHandler .recover(new Object[] { "Aldo" }, new RuntimeException("Planned")); assertEquals("barRecoverNumberValue", recoverResponseMapRe.get("bar").get("bar").get(0.0)); + } @Test @@ -222,6 +218,7 @@ public class RecoverAnnotationRecoveryHandlerTests { RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler( new MultipleQualifyingRecovers(), foo); assertEquals(1, handler.recover(new Object[] { "Randell" }, new RuntimeException("Planned"))); + } @Test @@ -230,6 +227,7 @@ public class RecoverAnnotationRecoveryHandlerTests { RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler( new MultipleQualifyingRecovers(), foo); assertEquals(1, handler.recover(new Object[] { null }, new RuntimeException("Planned"))); + } @Test @@ -238,6 +236,7 @@ public class RecoverAnnotationRecoveryHandlerTests { RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler( new MultipleQualifyingRecoversNoThrowable(), foo); assertEquals(1, handler.recover(new Object[] { null }, new RuntimeException("Planned"))); + } @Test @@ -246,6 +245,7 @@ public class RecoverAnnotationRecoveryHandlerTests { RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler( new MultipleQualifyingRecoversReOrdered(), foo); assertEquals(3, handler.recover(new Object[] { "Randell" }, new RuntimeException("Planned"))); + } @Test @@ -255,6 +255,7 @@ public class RecoverAnnotationRecoveryHandlerTests { new MultipleQualifyingRecoversExtendsThrowable(), foo); assertEquals(2, handler.recover(new Object[] { "Kevin" }, new IllegalArgumentException("Planned"))); assertEquals(3, handler.recover(new Object[] { "Kevin" }, new UnsupportedOperationException("Planned"))); + } @Test @@ -370,7 +371,7 @@ public class RecoverAnnotationRecoveryHandlerTests { } @Recover - public int bar(IllegalStateException e, String name) { + public int bar(RuntimeException e, String name) { return 1; } diff --git a/src/test/java/org/springframework/retry/policy/SimpleRetryPolicyTests.java b/src/test/java/org/springframework/retry/policy/SimpleRetryPolicyTests.java index aa7cff8..d56697d 100644 --- a/src/test/java/org/springframework/retry/policy/SimpleRetryPolicyTests.java +++ b/src/test/java/org/springframework/retry/policy/SimpleRetryPolicyTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2022 the original author or authors. + * Copyright 2006-2013 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. @@ -137,24 +137,4 @@ public class SimpleRetryPolicyTests { assertSame(context, child.getParent()); } - @Test - public void testRetryForException() { - Map, Boolean> map = new HashMap, Boolean>(); - map.put(RuntimeException.class, true); - SimpleRetryPolicy policy = new SimpleRetryPolicy(3, map, true); - RetryContext context = policy.open(null); - assertNotNull(context); - assertTrue(policy.retryForException(new RuntimeException())); - } - - @Test - public void testNoRetryForException() { - Map, Boolean> map = new HashMap, Boolean>(); - map.put(IllegalArgumentException.class, true); - SimpleRetryPolicy policy = new SimpleRetryPolicy(3, map, true); - RetryContext context = policy.open(null); - assertNotNull(context); - assertFalse(policy.retryForException(new RuntimeException())); - } - } diff --git a/src/test/java/org/springframework/retry/support/RetryTemplateTests.java b/src/test/java/org/springframework/retry/support/RetryTemplateTests.java index 5edc68d..8395939 100644 --- a/src/test/java/org/springframework/retry/support/RetryTemplateTests.java +++ b/src/test/java/org/springframework/retry/support/RetryTemplateTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2022 the original author or authors. + * Copyright 2006-2007 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. @@ -346,6 +346,7 @@ public class RetryTemplateTests { */ @Test public void testNoBackOffForRethrownException() throws Throwable { + RetryTemplate tested = new RetryTemplate(); tested.setRetryPolicy(new SimpleRetryPolicy(1)); @@ -383,54 +384,6 @@ public class RetryTemplateTests { verify(bop); } - @Test - public void testRethrowForNotRetryable() throws Throwable { - SimpleRetryPolicy policy = new SimpleRetryPolicy(1, - Collections., Boolean>singletonMap(IllegalArgumentException.class, true)); - RetryTemplate retryTemplate = new RetryTemplate(); - retryTemplate.setRetryPolicy(policy); - retryTemplate.setNoRecoveryForNotRetryable(true); - try { - retryTemplate.execute(new RetryCallback() { - @Override - public Object doWithRetry(RetryContext context) throws Exception { - throw new RuntimeException("Realllly bad!"); - } - }, new RecoveryCallback() { - @Override - public Object recover(RetryContext context) throws Exception { - return new Object(); - } - }); - fail("Expected RuntimeException"); - } - catch (RuntimeException e) { - assertEquals("Realllly bad!", e.getMessage()); - } - } - - @Test - public void testRethrowForRetryable() throws Throwable { - SimpleRetryPolicy policy = new SimpleRetryPolicy(1, - Collections., Boolean>singletonMap(RuntimeException.class, true)); - RetryTemplate retryTemplate = new RetryTemplate(); - retryTemplate.setRetryPolicy(policy); - retryTemplate.setNoRecoveryForNotRetryable(true); - final Object value = new Object(); - Object result = retryTemplate.execute(new RetryCallback() { - @Override - public Object doWithRetry(RetryContext context) throws Exception { - throw new RuntimeException("Will be recovered"); - } - }, new RecoveryCallback() { - @Override - public Object recover(RetryContext context) throws Exception { - return value; - } - }); - assertEquals(value, result); - } - private static class MockRetryCallback implements RetryCallback { private int attempts;