From 5986b4b7afc84105bdd45c26e9a59c708b975506 Mon Sep 17 00:00:00 2001
From: Oliver Gierke <ogierke@pivotal.io>
Date: Mon, 16 Apr 2018 16:00:43 +0200
Subject: [PATCH] DATACMNS-1299 - QueryExecutionsConverters now don't unwrap
 custom Iterable implementations.

Instead of a simple check for assignability from Iterable, we now properly use TypeInformation.isCollectionLike(), which checks for Iterable equality or assignability of collections or arrays as well as an explicit check for Slice as that is needed to properly unwrap Page instances and Slices themselves. That prevents custom domain types implementing Iterable from being unwrapped into their element types.
---
 .../util/QueryExecutionConverters.java        |  3 ++-
 .../AbstractRepositoryMetadataUnitTests.java  | 19 +++++++++++++++++++
 .../QueryExecutionConvertersUnitTests.java    | 18 ++++++++++++++++++
 3 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java b/src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java
index 971556944..866b21471 100644
--- a/src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java
+++ b/src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java
@@ -300,7 +300,8 @@ public abstract class QueryExecutionConverters {
 
 		Class<?> rawType = type.getType();
 
-		boolean needToUnwrap = Iterable.class.isAssignableFrom(rawType) //
+		boolean needToUnwrap = type.isCollectionLike() //
+				|| Slice.class.isAssignableFrom(rawType) //
 				|| rawType.isArray() //
 				|| supports(rawType) //
 				|| Stream.class.isAssignableFrom(rawType);
diff --git a/src/test/java/org/springframework/data/repository/core/support/AbstractRepositoryMetadataUnitTests.java b/src/test/java/org/springframework/data/repository/core/support/AbstractRepositoryMetadataUnitTests.java
index afcf85504..3d250b97b 100755
--- a/src/test/java/org/springframework/data/repository/core/support/AbstractRepositoryMetadataUnitTests.java
+++ b/src/test/java/org/springframework/data/repository/core/support/AbstractRepositoryMetadataUnitTests.java
@@ -101,6 +101,16 @@ public class AbstractRepositoryMetadataUnitTests {
 		assertThat(metadata.getReturnedDomainClass(method)).isEqualTo(User.class);
 	}
 
+	@Test // DATACMNS-1299
+	public void doesNotUnwrapCustomTypeImplementingIterable() throws Exception {
+
+		RepositoryMetadata metadata = AbstractRepositoryMetadata.getMetadata(ContainerRepository.class);
+
+		Method method = ContainerRepository.class.getMethod("someMethod");
+
+		assertThat(metadata.getReturnedDomainClass(method)).isEqualTo(Container.class);
+	}
+
 	interface UserRepository extends Repository<User, Long> {
 
 		User findSingle();
@@ -150,4 +160,13 @@ public class AbstractRepositoryMetadataUnitTests {
 		}
 	}
 
+	// DATACMNS-1299
+
+	class Element {}
+
+	abstract class Container implements Iterable<Element> {}
+
+	interface ContainerRepository extends Repository<Container, Long> {
+		Container someMethod();
+	}
 }
diff --git a/src/test/java/org/springframework/data/repository/util/QueryExecutionConvertersUnitTests.java b/src/test/java/org/springframework/data/repository/util/QueryExecutionConvertersUnitTests.java
index a96c41513..b3bc69624 100755
--- a/src/test/java/org/springframework/data/repository/util/QueryExecutionConvertersUnitTests.java
+++ b/src/test/java/org/springframework/data/repository/util/QueryExecutionConvertersUnitTests.java
@@ -29,6 +29,7 @@ import rx.Observable;
 import rx.Single;
 import scala.Option;
 
+import java.lang.reflect.Method;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
@@ -43,6 +44,8 @@ import org.reactivestreams.Publisher;
 import org.springframework.core.convert.support.DefaultConversionService;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.Slice;
+import org.springframework.data.util.ClassTypeInformation;
+import org.springframework.data.util.TypeInformation;
 import org.springframework.util.concurrent.ListenableFuture;
 
 import com.google.common.base.Optional;
@@ -347,4 +350,19 @@ public class QueryExecutionConvertersUnitTests {
 		assertThat(conversionService.convert(source, io.vavr.collection.Set.class)) //
 				.isInstanceOf(io.vavr.collection.Set.class);
 	}
+
+	@Test // DATACMNS-1299
+	public void unwrapsPages() throws Exception {
+
+		Method method = Sample.class.getMethod("pages");
+		TypeInformation<Object> returnType = ClassTypeInformation.fromReturnTypeOf(method);
+
+		assertThat(QueryExecutionConverters.unwrapWrapperTypes(returnType))
+				.isEqualTo(ClassTypeInformation.from(String.class));
+	}
+
+	interface Sample {
+
+		Page<String> pages();
+	}
 }