diff --git a/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java b/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java index b943963..e045756 100644 --- a/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java +++ b/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java @@ -46,7 +46,9 @@ import org.springframework.util.ReflectionUtils.MethodCallback; public class RecoverAnnotationRecoveryHandler implements MethodInvocationRecoverer { private SubclassClassifier classifier = new SubclassClassifier(); + private Map methods = new HashMap(); + private Object target; public RecoverAnnotationRecoveryHandler(Object target, Method method) { @@ -60,13 +62,13 @@ public class RecoverAnnotationRecoveryHandler implements MethodInvocationReco if (method == null) { throw new ExhaustedRetryException("Cannot locate recovery method", cause); } - SimpleMetadata meta = methods.get(method); + SimpleMetadata meta = this.methods.get(method); Object[] argsToUse = meta.getArgs(cause, args); boolean methodAccessible = method.isAccessible(); try { ReflectionUtils.makeAccessible(method); @SuppressWarnings("unchecked") - T result = (T) ReflectionUtils.invokeMethod(method, target, argsToUse); + T result = (T) ReflectionUtils.invokeMethod(method, this.target, argsToUse); return result; } finally { @@ -79,8 +81,8 @@ public class RecoverAnnotationRecoveryHandler implements MethodInvocationReco private Method findClosestMatch(Object[] args, Class cause) { int min = Integer.MAX_VALUE; Method result = null; - for (Method method : methods.keySet()) { - SimpleMetadata meta = methods.get(method); + for (Method method : this.methods.keySet()) { + SimpleMetadata meta = this.methods.get(method); Class type = meta.getType(); if (type == null) { type = Throwable.class; @@ -92,8 +94,8 @@ public class RecoverAnnotationRecoveryHandler implements MethodInvocationReco result = method; } else if (distance == min) { - boolean parametersMatch = compareParameters( args, meta.getArgCount(), - method.getParameterTypes() ); + boolean parametersMatch = compareParameters(args, meta.getArgCount(), + method.getParameterTypes()); if (parametersMatch) { result = method; } @@ -114,14 +116,17 @@ public class RecoverAnnotationRecoveryHandler implements MethodInvocationReco return result; } - private boolean compareParameters(Object[] args, int argCount, Class[] parameterTypes) { + private boolean compareParameters(Object[] args, int argCount, + Class[] parameterTypes) { if (argCount == (args.length + 1)) { int startingIndex = 0; - if (parameterTypes.length > 0 && Throwable.class.isAssignableFrom(parameterTypes[0])) { + if (parameterTypes.length > 0 + && Throwable.class.isAssignableFrom(parameterTypes[0])) { startingIndex = 1; } for (int i = startingIndex; i < parameterTypes.length; i++) { - final Object argument = args[i-1]; + final Object argument = i - startingIndex < args.length + ? args[i - startingIndex] : null; if (argument == null) { continue; } @@ -140,48 +145,50 @@ public class RecoverAnnotationRecoveryHandler implements MethodInvocationReco ReflectionUtils.doWithMethods(failingMethod.getDeclaringClass(), new MethodCallback() { @Override - public void doWith(Method method) throws IllegalArgumentException, - IllegalAccessException { + public void doWith(Method method) + throws IllegalArgumentException, IllegalAccessException { Recover recover = AnnotationUtils.findAnnotation(method, Recover.class); - if (recover != null - && method.getReturnType().isAssignableFrom( - failingMethod.getReturnType())) { + if (recover != null && method.getReturnType() + .isAssignableFrom(failingMethod.getReturnType())) { Class[] parameterTypes = method.getParameterTypes(); - if (parameterTypes.length > 0 - && Throwable.class - .isAssignableFrom(parameterTypes[0])) { + if (parameterTypes.length > 0 && Throwable.class + .isAssignableFrom(parameterTypes[0])) { @SuppressWarnings("unchecked") Class type = (Class) parameterTypes[0]; types.put(type, method); - methods.put(method, new SimpleMetadata( - parameterTypes.length, type)); - } else { - classifier.setDefaultValue(method); - methods.put(method, new SimpleMetadata( - parameterTypes.length, null)); + RecoverAnnotationRecoveryHandler.this.methods.put(method, + new SimpleMetadata(parameterTypes.length, type)); + } + else { + RecoverAnnotationRecoveryHandler.this.classifier + .setDefaultValue(method); + RecoverAnnotationRecoveryHandler.this.methods.put(method, + new SimpleMetadata(parameterTypes.length, null)); } } } }); - classifier.setTypeMap(types); + this.classifier.setTypeMap(types); optionallyFilterMethodsBy(failingMethod.getReturnType()); } private void optionallyFilterMethodsBy(Class returnClass) { Map filteredMethods = new HashMap(); - for (Method method : methods.keySet()) { + for (Method method : this.methods.keySet()) { if (method.getReturnType() == returnClass) { - filteredMethods.put(method, methods.get(method)); + filteredMethods.put(method, this.methods.get(method)); } } if (filteredMethods.size() > 0) { - methods = filteredMethods; + this.methods = filteredMethods; } } private static class SimpleMetadata { + private int argCount; + private Class type; public SimpleMetadata(int argCount, Class type) { @@ -191,23 +198,29 @@ public class RecoverAnnotationRecoveryHandler implements MethodInvocationReco } public int getArgCount() { - return argCount; + return this.argCount; } public Class getType() { - return type; + return this.type; } public Object[] getArgs(Throwable t, Object[] args) { Object[] result = new Object[getArgCount()]; int startArgs = 0; - if (type != null) { + if (this.type != null) { result[0] = t; startArgs = 1; } - System.arraycopy(args, 0, result, startArgs, result.length - startArgs); + int length = result.length - startArgs > args.length ? args.length + : result.length - startArgs; + if (length == 0) { + return result; + } + System.arraycopy(args, 0, result, startArgs, length); return result; } + } } diff --git a/src/test/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandlerTests.java b/src/test/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandlerTests.java index 813f108..44d1c0d 100644 --- a/src/test/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandlerTests.java +++ b/src/test/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandlerTests.java @@ -16,15 +16,16 @@ package org.springframework.retry.annotation; -import static org.junit.Assert.assertEquals; +import java.lang.reflect.Method; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; + import org.springframework.retry.ExhaustedRetryException; import org.springframework.util.ReflectionUtils; -import java.lang.reflect.Method; +import static org.junit.Assert.assertEquals; /** * @author Dave Syer @@ -39,10 +40,10 @@ public class RecoverAnnotationRecoveryHandlerTests { @Test public void defaultRecoverMethod() { RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler( - new DefaultRecover(), ReflectionUtils.findMethod(DefaultRecover.class, - "foo", String.class)); - assertEquals(1, - handler.recover(new Object[] { "Dave" }, new RuntimeException("Planned"))); + new DefaultRecover(), + ReflectionUtils.findMethod(DefaultRecover.class, "foo", String.class)); + assertEquals(1, handler.recover(new Object[] { "Dave" }, + new RuntimeException("Planned"))); } @Test @@ -50,8 +51,8 @@ public class RecoverAnnotationRecoveryHandlerTests { RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler( new FewerArgs(), ReflectionUtils.findMethod(FewerArgs.class, "foo", String.class, int.class)); - assertEquals(1, - handler.recover(new Object[] { "Dave" }, new RuntimeException("Planned"))); + assertEquals(1, handler.recover(new Object[] { "Dave" }, + new RuntimeException("Planned"))); } @Test @@ -66,69 +67,69 @@ public class RecoverAnnotationRecoveryHandlerTests { @Test public void noMatch() { RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler( - new SpecificException(), ReflectionUtils.findMethod( - SpecificException.class, "foo", String.class)); - expected.expect(ExhaustedRetryException.class); + new SpecificException(), + ReflectionUtils.findMethod(SpecificException.class, "foo", String.class)); + this.expected.expect(ExhaustedRetryException.class); handler.recover(new Object[] { "Dave" }, new Error("Planned")); } @Test public void specificRecoverMethod() { RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler( - new SpecificRecover(), ReflectionUtils.findMethod(SpecificRecover.class, - "foo", String.class)); - assertEquals(2, - handler.recover(new Object[] { "Dave" }, new RuntimeException("Planned"))); + new SpecificRecover(), + ReflectionUtils.findMethod(SpecificRecover.class, "foo", String.class)); + assertEquals(2, handler.recover(new Object[] { "Dave" }, + new RuntimeException("Planned"))); } @Test - public void inAccessibleRecoverMethods (){ - Method foo = ReflectionUtils.findMethod(InAccessibleRecover.class, - "foo", String.class); + public void inAccessibleRecoverMethods() { + Method foo = ReflectionUtils.findMethod(InAccessibleRecover.class, "foo", + String.class); RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler( new InAccessibleRecover(), foo); - assertEquals(1, - handler.recover(new Object[] { "Dave" }, new RuntimeException("Planned"))); + assertEquals(1, handler.recover(new Object[] { "Dave" }, + new RuntimeException("Planned"))); } @Test public void specificReturnTypeRecoverMethod() { RecoverAnnotationRecoveryHandler fooHandler = new RecoverAnnotationRecoveryHandler( - new InheritanceReturnTypeRecover(), ReflectionUtils.findMethod(InheritanceReturnTypeRecover.class, - "foo", String.class)); - assertEquals(1, - fooHandler.recover(new Object[] { "Aldo" }, new RuntimeException("Planned"))); - assertEquals(2, - fooHandler.recover(new Object[] { "Aldo" }, new IllegalStateException("Planned"))); + new InheritanceReturnTypeRecover(), ReflectionUtils.findMethod( + InheritanceReturnTypeRecover.class, "foo", String.class)); + assertEquals(1, fooHandler.recover(new Object[] { "Aldo" }, + new RuntimeException("Planned"))); + assertEquals(2, fooHandler.recover(new Object[] { "Aldo" }, + new IllegalStateException("Planned"))); } @Test public void parentReturnTypeRecoverMethod() { RecoverAnnotationRecoveryHandler barHandler = new RecoverAnnotationRecoveryHandler( - new InheritanceReturnTypeRecover(), ReflectionUtils.findMethod(InheritanceReturnTypeRecover.class, - "bar", String.class)); - assertEquals(3, - barHandler.recover(new Object[] { "Aldo" }, new RuntimeException("Planned"))); + new InheritanceReturnTypeRecover(), ReflectionUtils.findMethod( + InheritanceReturnTypeRecover.class, "bar", String.class)); + assertEquals(3, barHandler.recover(new Object[] { "Aldo" }, + new RuntimeException("Planned"))); } @Test - public void multipleQualifyingRecoverMethods(){ - Method foo = ReflectionUtils.findMethod(MultipleQualifyingRecovers.class, - "foo", String.class); + public void multipleQualifyingRecoverMethods() { + Method foo = ReflectionUtils.findMethod(MultipleQualifyingRecovers.class, "foo", + String.class); RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler( new MultipleQualifyingRecovers(), foo); - assertEquals(1, - handler.recover(new Object[] { "Randell" }, new RuntimeException("Planned"))); + assertEquals(1, handler.recover(new Object[] { "Randell" }, + new RuntimeException("Planned"))); } @Test - public void multipleQualifyingRecoverMethodsWithNull(){ - Method foo = ReflectionUtils.findMethod(MultipleQualifyingRecovers.class, - "foo", String.class); + public void multipleQualifyingRecoverMethodsWithNull() { + Method foo = ReflectionUtils.findMethod(MultipleQualifyingRecovers.class, "foo", + String.class); RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler( new MultipleQualifyingRecovers(), foo); assertEquals(1, @@ -137,40 +138,52 @@ public class RecoverAnnotationRecoveryHandlerTests { } @Test - public void multipleQualifyingRecoverMethodsReOrdered(){ + public void multipleQualifyingRecoverMethodsWithNoThrowable() { + Method foo = ReflectionUtils.findMethod( + MultipleQualifyingRecoversNoThrowable.class, "foo", String.class); + RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler( + new MultipleQualifyingRecoversNoThrowable(), foo); + assertEquals(1, + handler.recover(new Object[] { null }, new RuntimeException("Planned"))); + + } + + @Test + public void multipleQualifyingRecoverMethodsReOrdered() { Method foo = ReflectionUtils.findMethod(MultipleQualifyingRecoversReOrdered.class, "foo", String.class); RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler( new MultipleQualifyingRecoversReOrdered(), foo); - assertEquals(3, - handler.recover(new Object[] { "Randell" }, new RuntimeException("Planned"))); + assertEquals(3, handler.recover(new Object[] { "Randell" }, + new RuntimeException("Planned"))); } - - @Test - public void multipleQualifyingRecoverMethodsExtendsThrowable(){ - Method foo = ReflectionUtils.findMethod(MultipleQualifyingRecoversExtendsThrowable.class, - "foo", String.class); + + @Test + public void multipleQualifyingRecoverMethodsExtendsThrowable() { + Method foo = ReflectionUtils.findMethod( + MultipleQualifyingRecoversExtendsThrowable.class, "foo", String.class); RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler( new MultipleQualifyingRecoversExtendsThrowable(), foo); - assertEquals(2, - handler.recover(new Object[] { "Kevin" }, new IllegalArgumentException("Planned"))); - assertEquals(3, - handler.recover(new Object[] { "Kevin" }, new UnsupportedOperationException("Planned"))); + assertEquals(2, handler.recover(new Object[] { "Kevin" }, + new IllegalArgumentException("Planned"))); + assertEquals(3, handler.recover(new Object[] { "Kevin" }, + new UnsupportedOperationException("Planned"))); } private static class InAccessibleRecover { @Retryable - private int foo (String n) { + private int foo(String n) { throw new RuntimeException("error trying to foo('" + n + "')"); } @Recover - private int bar(String n){ - return 1 ; + private int bar(String n) { + return 1; } + } protected static class DefaultRecover { @@ -184,9 +197,11 @@ public class RecoverAnnotationRecoveryHandlerTests { public int bar(String name) { return 1; } + } protected static class NoArgs { + private Throwable cause; @Retryable @@ -199,11 +214,13 @@ public class RecoverAnnotationRecoveryHandlerTests { } public Throwable getCause() { - return cause; + return this.cause; } + } protected static class SpecificRecover { + @Retryable public int foo(String name) { return 0; @@ -222,6 +239,7 @@ public class RecoverAnnotationRecoveryHandlerTests { } protected static class FewerArgs { + @Retryable public int foo(String name, int value) { return 0; @@ -235,6 +253,7 @@ public class RecoverAnnotationRecoveryHandlerTests { } protected static class SpecificException { + @Retryable public int foo(String name) { return 0; @@ -276,6 +295,25 @@ public class RecoverAnnotationRecoveryHandlerTests { } + protected static class MultipleQualifyingRecoversNoThrowable { + + @Retryable + public int foo(String name) { + return 0; + } + + @Recover + public int fooRecover(String name, String nullable) { + return 1; + } + + @Recover + public int fooRecover(int other, String nullable) { + return 2; + } + + } + protected static class MultipleQualifyingRecovers { @Retryable @@ -323,8 +361,8 @@ public class RecoverAnnotationRecoveryHandlerTests { } } - - protected static class MultipleQualifyingRecoversExtendsThrowable { + + protected static class MultipleQualifyingRecoversExtendsThrowable { @Retryable public int foo(String name) { @@ -335,8 +373,8 @@ public class RecoverAnnotationRecoveryHandlerTests { public int fooRecover(IllegalArgumentException e, String name) { return 1; } - - @Recover + + @Recover public int barRecover(IllegalArgumentException e, String name) { return 2; }