Add generic check for function return type

Fixes: [#215](https://github.com/spring-projects/spring-retry/issues/215) Signed-off-by: deepesh-verma <deepeshvrm1@gmail.com>
2020-09-04 01:10:46 +05:30
parent c452ec3237
commit 4d2bb2be27
2 changed files with 272 additions and 14 deletions
--- 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<T> 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<? extends Throwable> type = (Class<? extends Throwable>) 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<Class<? extends Throwable>, Method> types) {
+		Class<?>[] parameterTypes = method.getParameterTypes();
+		if (parameterTypes.length > 0 && Throwable.class.isAssignableFrom(parameterTypes[0])) {
+			@SuppressWarnings("unchecked")
+			Class<? extends Throwable> type = (Class<? extends Throwable>) 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());
--- 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<List<String>>(
+				new GenericReturnTypeRecover(),
+				ReflectionUtils.findMethod(GenericReturnTypeRecover.class, "foo", String.class));
+
+		@SuppressWarnings("unchecked")
+		Map<String, String> recoverResponseMap = (Map<String, String>) 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<List<String>>(
+				new GenericReturnTypeRecover(),
+				ReflectionUtils.findMethod(GenericReturnTypeRecover.class, "foo", String.class));
+
+		@SuppressWarnings("unchecked")
+		Map<String, String> recoverResponseMap = (Map<String, String>) 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<List<String>>(
+				new GenericReturnTypeRecover(),
+				ReflectionUtils.findMethod(GenericReturnTypeRecover.class, "bar", String.class));
+
+		@SuppressWarnings("unchecked")
+		Map<String, GenericReturnTypeRecover.One> recoverResponseMap = (Map<String, GenericReturnTypeRecover.One>) 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<Integer>(
+				new GenericInheritanceReturnTypeRecover(),
+				ReflectionUtils.findMethod(GenericInheritanceReturnTypeRecover.class, "foo", String.class));
+		@SuppressWarnings("unchecked")
+		Map<String, Integer> recoverResponseMapRe = (Map<String, Integer>) fooHandler.recover(new Object[] { "Aldo" },
+				new RuntimeException("Planned"));
+		assertEquals(1, recoverResponseMapRe.get("foo").intValue());
+		@SuppressWarnings("unchecked")
+		Map<String, Integer> recoverResponseMapIse = (Map<String, Integer>) 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<Double>(
+				new GenericInheritanceReturnTypeRecover(),
+				ReflectionUtils.findMethod(GenericInheritanceReturnTypeRecover.class, "bar", String.class));
+		@SuppressWarnings("unchecked")
+		Map<String, Number> recoverResponseMapRe = (Map<String, Number>) barHandler.recover(new Object[] { "Aldo" },
+				new RuntimeException("Planned"));
+		assertEquals(0.2, recoverResponseMapRe.get("bar"));
+	}
+
+	@Test
+	public void genericNestedMapIntegerStringReturnTypeRecoverMethod() {
+		RecoverAnnotationRecoveryHandler<?> fooHandler = new RecoverAnnotationRecoveryHandler<Integer>(
+				new NestedGenericInheritanceReturnTypeRecover(),
+				ReflectionUtils.findMethod(NestedGenericInheritanceReturnTypeRecover.class, "foo", String.class));
+		@SuppressWarnings("unchecked")
+		Map<String, Map<String, Map<Integer, String>>> recoverResponseMapRe = (Map<String, Map<String, Map<Integer, String>>>) fooHandler
+				.recover(new Object[] { "Aldo" }, new RuntimeException("Planned"));
+		assertEquals("fooRecoverReValue", recoverResponseMapRe.get("foo").get("foo").get(0));
+		@SuppressWarnings("unchecked")
+		Map<String, Map<String, Map<Integer, String>>> recoverResponseMapIe = (Map<String, Map<String, Map<Integer, String>>>) 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<Double>(
+				new NestedGenericInheritanceReturnTypeRecover(),
+				ReflectionUtils.findMethod(NestedGenericInheritanceReturnTypeRecover.class, "bar", String.class));
+		@SuppressWarnings("unchecked")
+		Map<String, Map<String, Map<Number, String>>> recoverResponseMapRe = (Map<String, Map<String, Map<Number, String>>>) 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<String, String> foo(String name) {
+			return Collections.singletonMap("foo", "fooValue");
+		}
+
+		@Retryable
+		public Map<String, One> bar(String name) {
+			return Collections.singletonMap("bar", new One("barValue"));
+		}
+
+		@Recover
+		public Map<String, String> fooRecoverRe(RuntimeException re, String name) {
+			return Collections.singletonMap("foo", "fooRecoverValue1");
+		}
+
+		@Recover
+		public Map<String, String> fooRecoverIe(IllegalStateException re, String name) {
+			return Collections.singletonMap("foo", "fooRecoverValue2");
+		}
+
+		@Recover
+		public Map<String, One> barRecover(RuntimeException re, String name) {
+			return Collections.singletonMap("bar", new One("barRecoverValue"));
+		}
+
+	}
+
+	protected static class GenericInheritanceReturnTypeRecover {
+
+		@Retryable
+		public Map<String, Integer> foo(String name) {
+			return Collections.singletonMap("foo", 0);
+		}
+
+		@Retryable
+		public Map<String, Number> bar(String name) {
+			return Collections.singletonMap("bar", (Number) 0.0);
+		}
+
+		@Recover
+		public Map<String, Integer> fooRecoverRe(RuntimeException re, String name) {
+			return Collections.singletonMap("foo", 1);
+		}
+
+		@Recover
+		public Map<String, Integer> fooRecoverIe(IllegalStateException re, String name) {
+			return Collections.singletonMap("foo", 2);
+		}
+
+		@Recover
+		public Map<String, Double> barRecoverDouble(RuntimeException re, String name) {
+			return Collections.singletonMap("bar", 0.1);
+		}
+
+		@Recover
+		public Map<String, Number> barRecoverNumber(RuntimeException re, String name) {
+			return Collections.singletonMap("bar", (Number) 0.2);
+		}
+
+	}
+
+	protected static class NestedGenericInheritanceReturnTypeRecover {
+
+		@Retryable
+		public Map<String, Map<String, Map<Integer, String>>> foo(String name) {
+			return Collections.singletonMap("foo",
+					Collections.singletonMap("foo", Collections.singletonMap(0, "fooValue")));
+		}
+
+		@Retryable
+		public Map<String, Map<String, Map<Number, String>>> bar(String name) {
+			return Collections.singletonMap("bar",
+					Collections.singletonMap("bar", Collections.singletonMap((Number) 0.0, "barValue")));
+		}
+
+		@Recover
+		public Map<String, Map<String, Map<Integer, String>>> fooRecoverRe(RuntimeException re, String name) {
+			return Collections.singletonMap("foo",
+					Collections.singletonMap("foo", Collections.singletonMap(0, "fooRecoverReValue")));
+		}
+
+		@Recover
+		public Map<String, Map<String, Map<Integer, String>>> fooRecoverIe(IllegalStateException re, String name) {
+			return Collections.singletonMap("foo",
+					Collections.singletonMap("foo", Collections.singletonMap(0, "fooRecoverIeValue")));
+		}
+
+		@Recover
+		public Map<String, Map<String, Map<Number, String>>> barRecoverNumber(RuntimeException re, String name) {
+			return Collections.singletonMap("bar",
+					Collections.singletonMap("bar", Collections.singletonMap((Number) 0.0, "barRecoverNumberValue")));
+		}
+
+		@Recover
+		public Map<String, Map<String, Map<Double, String>>> barRecoverDouble(RuntimeException re, String name) {
+			return Collections.singletonMap("bar",
+					Collections.singletonMap("bar", Collections.singletonMap(0.0, "barRecoverDoubleValue")));
+		}
+
+	}
+
 	protected static class MultipleQualifyingRecoversNoThrowable {

 		@Retryable