From 5fa302b75f1e303f98e3618e74db8d7a05f8ea13 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 a7ddf0d32..7426b1c19 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 {
 
@@ -62,10 +63,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);
 		TypeDescriptor methodReturnTypeDescriptor = TypeDescriptor.nested(parameter, 0);
 
@@ -88,20 +85,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)) {
 
 			TypeDescriptor targetType = TypeDescriptor.valueOf(expectedReturnType);
@@ -126,8 +111,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 ba6f69577..3d902392c 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 javaslang.control.Option;
@@ -25,6 +26,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;
@@ -33,6 +35,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.core.convert.TypeDescriptor;
@@ -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() {
 
-		Iterable<?> source = Arrays.asList(new Object());
+		Iterable<?> source = asList(new Object());
 
 		Object result = handler.postProcessInvocationResult(source, TypeDescriptor.valueOf(Streamable.class));
 
@@ -362,6 +366,22 @@ public class QueryExecutionResultHandlerUnitTests {
 				it -> assertThat(it.stream().collect(Collectors.toList())).isEqualTo(source));
 	}
 
+	@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);
 	}
@@ -387,6 +407,8 @@ public class QueryExecutionResultHandlerUnitTests {
 		Single<Entity> single();
 
 		Completable completable();
+
+		List<Integer> listOfInteger();
 	}
 
 	static class Entity {}