From 4d2bb2be27f668e5b6b5b5c6699658dce2ac52da Mon Sep 17 00:00:00 2001 From: deepesh-verma Date: Fri, 4 Sep 2020 01:10:46 +0530 Subject: [PATCH] Add generic check for function return type Fixes: [#215](https://github.com/spring-projects/spring-retry/issues/215) Signed-off-by: deepesh-verma --- .../RecoverAnnotationRecoveryHandler.java | 68 ++++-- ...RecoverAnnotationRecoveryHandlerTests.java | 218 +++++++++++++++++- 2 files changed, 272 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java b/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java index 02b208a..80065af 100644 --- a/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java +++ b/src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java @@ -17,6 +17,8 @@ package org.springframework.retry.annotation; import java.lang.reflect.Method; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Type; import java.util.HashMap; import java.util.Map; @@ -173,27 +175,67 @@ public class RecoverAnnotationRecoveryHandler implements MethodInvocationReco if (recover == null) { recover = findAnnotationOnTarget(target, method); } - if (recover != null && method.getReturnType().isAssignableFrom(failingMethod.getReturnType())) { - Class[] parameterTypes = method.getParameterTypes(); - if (parameterTypes.length > 0 && Throwable.class.isAssignableFrom(parameterTypes[0])) { - @SuppressWarnings("unchecked") - Class type = (Class) parameterTypes[0]; - types.put(type, method); - 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)); + if (recover != null && failingMethod.getGenericReturnType() instanceof ParameterizedType + && method.getGenericReturnType() instanceof ParameterizedType) { + if (isParameterizedTypeAssignable((ParameterizedType) method.getGenericReturnType(), + (ParameterizedType) failingMethod.getGenericReturnType())) { + putToMethodsMap(method, types); } } + else if (recover != null && method.getReturnType().isAssignableFrom(failingMethod.getReturnType())) { + putToMethodsMap(method, types); + } } }); this.classifier.setTypeMap(types); optionallyFilterMethodsBy(failingMethod.getReturnType()); } + /** + * Returns {@code true} if the input methodReturnType is a direct match of the + * failingMethodReturnType. Takes nested generics into consideration as well, while + * deciding a match. + * @param methodReturnType + * @param failingMethodReturnType + * @return + */ + private boolean isParameterizedTypeAssignable(ParameterizedType methodReturnType, + ParameterizedType failingMethodReturnType) { + Type[] methodActualArgs = methodReturnType.getActualTypeArguments(); + Type[] failingMethodActualArgs = failingMethodReturnType.getActualTypeArguments(); + if (methodActualArgs.length != failingMethodActualArgs.length) { + return false; + } + int startingIndex = 0; + for (int i = startingIndex; i < methodActualArgs.length; i++) { + Type methodArgType = methodActualArgs[i]; + Type failingMethodArgType = failingMethodActualArgs[i]; + if (methodArgType instanceof ParameterizedType && failingMethodArgType instanceof ParameterizedType) { + return isParameterizedTypeAssignable((ParameterizedType) methodArgType, + (ParameterizedType) failingMethodArgType); + } + if (methodArgType instanceof Class && failingMethodArgType instanceof Class + && !failingMethodArgType.equals(methodArgType)) { + return false; + } + } + return true; + } + + private void putToMethodsMap(Method method, Map, Method> types) { + Class[] parameterTypes = method.getParameterTypes(); + if (parameterTypes.length > 0 && Throwable.class.isAssignableFrom(parameterTypes[0])) { + @SuppressWarnings("unchecked") + Class type = (Class) parameterTypes[0]; + types.put(type, method); + 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)); + } + } + private Recover findAnnotationOnTarget(Object target, Method method) { try { Method targetMethod = target.getClass().getMethod(method.getName(), method.getParameterTypes()); diff --git a/src/test/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandlerTests.java b/src/test/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandlerTests.java index 9b73827..02c25fd 100644 --- a/src/test/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandlerTests.java +++ b/src/test/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandlerTests.java @@ -18,16 +18,19 @@ package org.springframework.retry.annotation; import java.lang.reflect.Method; import java.util.ArrayList; +import java.util.Collections; import java.util.List; +import java.util.Map; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; import org.springframework.retry.ExhaustedRetryException; +import org.springframework.util.CollectionUtils; import org.springframework.util.ReflectionUtils; -import static org.junit.Assert.assertEquals; +import static org.junit.Assert.*; /** * @author Dave Syer @@ -107,6 +110,106 @@ public class RecoverAnnotationRecoveryHandlerTests { } + @Test + public void genericReturnStringValueTypeParentThrowableRecoverMethod() { + + RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler>( + new GenericReturnTypeRecover(), + ReflectionUtils.findMethod(GenericReturnTypeRecover.class, "foo", String.class)); + + @SuppressWarnings("unchecked") + Map recoverResponseMap = (Map) handler.recover(new Object[] { "Aldo" }, + new RuntimeException("Planned")); + assertFalse(CollectionUtils.isEmpty(recoverResponseMap)); + assertEquals("fooRecoverValue1", recoverResponseMap.get("foo")); + } + + @Test + public void genericReturnStringValueTypeChildThrowableRecoverMethod() { + + RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler>( + new GenericReturnTypeRecover(), + ReflectionUtils.findMethod(GenericReturnTypeRecover.class, "foo", String.class)); + + @SuppressWarnings("unchecked") + Map recoverResponseMap = (Map) handler.recover(new Object[] { "Aldo" }, + new IllegalStateException("Planned")); + assertFalse(CollectionUtils.isEmpty(recoverResponseMap)); + assertEquals("fooRecoverValue2", recoverResponseMap.get("foo")); + } + + @Test + public void genericReturnOneValueTypeRecoverMethod() { + + RecoverAnnotationRecoveryHandler handler = new RecoverAnnotationRecoveryHandler>( + new GenericReturnTypeRecover(), + ReflectionUtils.findMethod(GenericReturnTypeRecover.class, "bar", String.class)); + + @SuppressWarnings("unchecked") + Map recoverResponseMap = (Map) handler + .recover(new Object[] { "Aldo" }, new RuntimeException("Planned")); + assertFalse(CollectionUtils.isEmpty(recoverResponseMap)); + assertNotNull(recoverResponseMap.get("bar")); + assertEquals("barRecoverValue", recoverResponseMap.get("bar").name); + } + + @Test + public void genericSpecifiedReturnTypeRecoverMethod() { + RecoverAnnotationRecoveryHandler fooHandler = new RecoverAnnotationRecoveryHandler( + new GenericInheritanceReturnTypeRecover(), + ReflectionUtils.findMethod(GenericInheritanceReturnTypeRecover.class, "foo", String.class)); + @SuppressWarnings("unchecked") + Map recoverResponseMapRe = (Map) fooHandler.recover(new Object[] { "Aldo" }, + new RuntimeException("Planned")); + assertEquals(1, recoverResponseMapRe.get("foo").intValue()); + @SuppressWarnings("unchecked") + Map recoverResponseMapIse = (Map) fooHandler.recover(new Object[] { "Aldo" }, + new IllegalStateException("Planned")); + assertEquals(2, recoverResponseMapIse.get("foo").intValue()); + } + + /** + * Even if there are @Recover methods with narrower generic return types, the one with + * direct match should get called + */ + @Test + public void genericDirectMatchReturnTypeRecoverMethod() { + RecoverAnnotationRecoveryHandler barHandler = new RecoverAnnotationRecoveryHandler( + new GenericInheritanceReturnTypeRecover(), + ReflectionUtils.findMethod(GenericInheritanceReturnTypeRecover.class, "bar", String.class)); + @SuppressWarnings("unchecked") + Map recoverResponseMapRe = (Map) barHandler.recover(new Object[] { "Aldo" }, + new RuntimeException("Planned")); + assertEquals(0.2, recoverResponseMapRe.get("bar")); + } + + @Test + public void genericNestedMapIntegerStringReturnTypeRecoverMethod() { + RecoverAnnotationRecoveryHandler fooHandler = new RecoverAnnotationRecoveryHandler( + new NestedGenericInheritanceReturnTypeRecover(), + ReflectionUtils.findMethod(NestedGenericInheritanceReturnTypeRecover.class, "foo", String.class)); + @SuppressWarnings("unchecked") + Map>> recoverResponseMapRe = (Map>>) fooHandler + .recover(new Object[] { "Aldo" }, new RuntimeException("Planned")); + assertEquals("fooRecoverReValue", recoverResponseMapRe.get("foo").get("foo").get(0)); + @SuppressWarnings("unchecked") + Map>> recoverResponseMapIe = (Map>>) fooHandler + .recover(new Object[] { "Aldo" }, new IllegalStateException("Planned")); + assertEquals("fooRecoverIeValue", recoverResponseMapIe.get("foo").get("foo").get(0)); + } + + @Test + public void genericNestedMapNumberStringReturnTypeRecoverMethod() { + RecoverAnnotationRecoveryHandler barHandler = new RecoverAnnotationRecoveryHandler( + new NestedGenericInheritanceReturnTypeRecover(), + ReflectionUtils.findMethod(NestedGenericInheritanceReturnTypeRecover.class, "bar", String.class)); + @SuppressWarnings("unchecked") + Map>> recoverResponseMapRe = (Map>>) barHandler + .recover(new Object[] { "Aldo" }, new RuntimeException("Planned")); + assertEquals("barRecoverNumberValue", recoverResponseMapRe.get("bar").get("bar").get(0.0)); + + } + @Test public void multipleQualifyingRecoverMethods() { Method foo = ReflectionUtils.findMethod(MultipleQualifyingRecovers.class, "foo", String.class); @@ -293,6 +396,119 @@ public class RecoverAnnotationRecoveryHandlerTests { } + protected static class GenericReturnTypeRecover { + + private static class One { + + String name; + + public One(String name) { + this.name = name; + } + + } + + @Retryable + public Map foo(String name) { + return Collections.singletonMap("foo", "fooValue"); + } + + @Retryable + public Map bar(String name) { + return Collections.singletonMap("bar", new One("barValue")); + } + + @Recover + public Map fooRecoverRe(RuntimeException re, String name) { + return Collections.singletonMap("foo", "fooRecoverValue1"); + } + + @Recover + public Map fooRecoverIe(IllegalStateException re, String name) { + return Collections.singletonMap("foo", "fooRecoverValue2"); + } + + @Recover + public Map barRecover(RuntimeException re, String name) { + return Collections.singletonMap("bar", new One("barRecoverValue")); + } + + } + + protected static class GenericInheritanceReturnTypeRecover { + + @Retryable + public Map foo(String name) { + return Collections.singletonMap("foo", 0); + } + + @Retryable + public Map bar(String name) { + return Collections.singletonMap("bar", (Number) 0.0); + } + + @Recover + public Map fooRecoverRe(RuntimeException re, String name) { + return Collections.singletonMap("foo", 1); + } + + @Recover + public Map fooRecoverIe(IllegalStateException re, String name) { + return Collections.singletonMap("foo", 2); + } + + @Recover + public Map barRecoverDouble(RuntimeException re, String name) { + return Collections.singletonMap("bar", 0.1); + } + + @Recover + public Map barRecoverNumber(RuntimeException re, String name) { + return Collections.singletonMap("bar", (Number) 0.2); + } + + } + + protected static class NestedGenericInheritanceReturnTypeRecover { + + @Retryable + public Map>> foo(String name) { + return Collections.singletonMap("foo", + Collections.singletonMap("foo", Collections.singletonMap(0, "fooValue"))); + } + + @Retryable + public Map>> bar(String name) { + return Collections.singletonMap("bar", + Collections.singletonMap("bar", Collections.singletonMap((Number) 0.0, "barValue"))); + } + + @Recover + public Map>> fooRecoverRe(RuntimeException re, String name) { + return Collections.singletonMap("foo", + Collections.singletonMap("foo", Collections.singletonMap(0, "fooRecoverReValue"))); + } + + @Recover + public Map>> fooRecoverIe(IllegalStateException re, String name) { + return Collections.singletonMap("foo", + Collections.singletonMap("foo", Collections.singletonMap(0, "fooRecoverIeValue"))); + } + + @Recover + public Map>> barRecoverNumber(RuntimeException re, String name) { + return Collections.singletonMap("bar", + Collections.singletonMap("bar", Collections.singletonMap((Number) 0.0, "barRecoverNumberValue"))); + } + + @Recover + public Map>> barRecoverDouble(RuntimeException re, String name) { + return Collections.singletonMap("bar", + Collections.singletonMap("bar", Collections.singletonMap(0.0, "barRecoverDoubleValue"))); + } + + } + protected static class MultipleQualifyingRecoversNoThrowable { @Retryable