From 6dc0ced366d34f5acb7926f8785b3df1ae084b1e 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 8282ef081..56853f613 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.Option;
@@ -27,6 +28,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.Iterator;
@@ -36,6 +38,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;
@@ -47,6 +50,7 @@ import org.springframework.data.util.Streamable;
  *
  * @author Oliver Gierke
  * @author Mark Paluch
+ * @author Jens Schauder
  */
 public class QueryExecutionResultHandlerUnitTests {
 
@@ -356,7 +360,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"));
 
@@ -384,6 +388,22 @@ public class QueryExecutionResultHandlerUnitTests {
 		});
 	}
 
+	@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);
 	}
@@ -421,6 +441,8 @@ public class QueryExecutionResultHandlerUnitTests {
 
 		// DATACMNS-1430
 		CustomStreamableWrapper<String> customStreamable();
+
+		List<Integer> listOfInteger();
 	}
 
 	static class Entity {}