From 7bc27ca934a813fe5514700b5fd35e401e1ffbce Mon Sep 17 00:00:00 2001
From: Jens Schauder <jschauder@pivotal.io>
Date: Wed, 20 Feb 2019 10:59:07 +0100
Subject: [PATCH] DATACMNS-1482 - Properly convert collections when collection
 type matches but element type doesn't.

Various fast returns and the use of Class instead of TypeDescriptor led to e.g. List<BigDecimal> not getting properly converted to List<Integer> leading to unexpected ClassCastExceptions when the collection elements where accessed.
---
 .../support/QueryExecutionResultHandler.java  | 21 +++-------------
 .../QueryExecutionResultHandlerUnitTests.java | 24 ++++++++++++++++++-
 2 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/src/main/java/org/springframework/data/repository/core/support/QueryExecutionResultHandler.java b/src/main/java/org/springframework/data/repository/core/support/QueryExecutionResultHandler.java
index eb1366054..5e2667bfd 100644
--- a/src/main/java/org/springframework/data/repository/core/support/QueryExecutionResultHandler.java
+++ b/src/main/java/org/springframework/data/repository/core/support/QueryExecutionResultHandler.java
@@ -34,6 +34,7 @@ import org.springframework.lang.Nullable;
  *
  * @author Oliver Gierke
  * @author Mark Paluch
+ * @author Jens Schauder
  */
 class QueryExecutionResultHandler {
 
@@ -63,10 +64,6 @@ class QueryExecutionResultHandler {
 	@Nullable
 	public Object postProcessInvocationResult(@Nullable Object result, Method method) {
 
-		if (method.getReturnType().isInstance(result)) {
-			return result;
-		}
-
 		MethodParameter parameter = new MethodParameter(method, -1);
 
 		return postProcessInvocationResult(result, 0, parameter);
@@ -91,20 +88,8 @@ class QueryExecutionResultHandler {
 
 		Class<?> expectedReturnType = returnTypeDescriptor.getType();
 
-		// Early return if the raw value matches
-
-		if (result != null && expectedReturnType.isInstance(result)) {
-			return result;
-		}
-
 		result = unwrapOptional(result);
 
-		// Early return if the unrwapped value matches
-
-		if (result != null && expectedReturnType.isInstance(result)) {
-			return result;
-		}
-
 		if (QueryExecutionConverters.supports(expectedReturnType)) {
 
 			// For a wrapper type, try nested resolution first
@@ -132,8 +117,8 @@ class QueryExecutionResultHandler {
 				return ReactiveWrapperConverters.toWrapper(result, expectedReturnType);
 			}
 
-			return conversionService.canConvert(result.getClass(), expectedReturnType)
-					? conversionService.convert(result, expectedReturnType)
+			return conversionService.canConvert(TypeDescriptor.forObject(result), returnTypeDescriptor)
+					? conversionService.convert(result, returnTypeDescriptor)
 					: result;
 		}
 
diff --git a/src/test/java/org/springframework/data/repository/core/support/QueryExecutionResultHandlerUnitTests.java b/src/test/java/org/springframework/data/repository/core/support/QueryExecutionResultHandlerUnitTests.java
index dc3db616c..f8953da3d 100755
--- a/src/test/java/org/springframework/data/repository/core/support/QueryExecutionResultHandlerUnitTests.java
+++ b/src/test/java/org/springframework/data/repository/core/support/QueryExecutionResultHandlerUnitTests.java
@@ -15,6 +15,7 @@
  */
 package org.springframework.data.repository.core.support;
 
+import static java.util.Arrays.*;
 import static org.assertj.core.api.Assertions.*;
 
 import io.vavr.control.Try;
@@ -26,6 +27,7 @@ import rx.Observable;
 import rx.Single;
 
 import java.lang.reflect.Method;
+import java.math.BigDecimal;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
@@ -34,6 +36,7 @@ import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 
+import org.assertj.core.api.SoftAssertions;
 import org.junit.Test;
 import org.reactivestreams.Publisher;
 import org.springframework.dao.InvalidDataAccessApiUsageException;
@@ -45,6 +48,7 @@ import org.springframework.data.util.Streamable;
  *
  * @author Oliver Gierke
  * @author Mark Paluch
+ * @author Jens Schauder
  */
 public class QueryExecutionResultHandlerUnitTests {
 
@@ -354,7 +358,7 @@ public class QueryExecutionResultHandlerUnitTests {
 	@SuppressWarnings("unchecked")
 	public void convertsIterableIntoStreamable() throws Exception {
 
-		Iterable<?> source = Arrays.asList(new Object());
+		Iterable<?> source = asList(new Object());
 
 		Object result = handler.postProcessInvocationResult(source, getMethod("streamable"));
 
@@ -372,6 +376,22 @@ public class QueryExecutionResultHandlerUnitTests {
 		assertThat(result).isInstanceOfSatisfying(Option.class, it -> assertThat(it.get()).isEqualTo(entity));
 	}
 
+	@Test // DATACMNS-1482
+	public void nestedConversion() throws Exception {
+
+		Object result = handler.postProcessInvocationResult(asList(BigDecimal.ZERO, BigDecimal.ONE),
+				getMethod("listOfInteger"));
+
+		assertThat(result).isInstanceOf(List.class);
+
+		List list = (List) result;
+		SoftAssertions.assertSoftly(s -> {
+			// for making the test failure more obvious:
+			(list).forEach(v -> s.assertThat(v).isInstanceOf(Integer.class));
+			s.assertThat(list).containsExactly(0, 1);
+		});
+	}
+
 	private static Method getMethod(String methodName) throws Exception {
 		return Sample.class.getMethod(methodName);
 	}
@@ -406,6 +426,8 @@ public class QueryExecutionResultHandlerUnitTests {
 
 		// DATACMNS-938
 		Try<Option<Entity>> tryOfOption();
+
+		List<Integer> listOfInteger();
 	}
 
 	static class Entity {}