DATACMNS-940 - Fix query method query result type detection for custom collections.

The nullable wrapper types now expose whether they're a single value type or a multi value type. This is necessary as Javaslang's Option implements Iterable (something we previously checked for to detect a collection query) and the custom collections potentially being usable within other wrapper types (e.g. Future<Seq<User>>).

src/main/java/org/springframework/data/repository/query/QueryMethod.java

@@ -173,8 +173,22 @@ public class QueryMethod {
 	 */
 	public boolean isCollectionQuery() {
 
-		return !(isPageQuery() || isSliceQuery())
-				&& org.springframework.util.ClassUtils.isAssignable(Iterable.class, unwrappedReturnType)
+		if (isPageQuery() || isSliceQuery()) {
+			return false;
+		}
+
+		Class<?> returnType = method.getReturnType();
+
+		if (QueryExecutionConverters.supports(returnType) && !QueryExecutionConverters.isSingleValue(returnType)) {
+			return true;
+		}
+
+		if (QueryExecutionConverters.supports(unwrappedReturnType)
+				&& QueryExecutionConverters.isSingleValue(unwrappedReturnType)) {
+			return false;
+		}
+
+		return org.springframework.util.ClassUtils.isAssignable(Iterable.class, unwrappedReturnType)
 				|| unwrappedReturnType.isArray();
 	}
 

src/main/java/org/springframework/data/repository/util/JavaslangCollections.java

@@ -28,6 +28,7 @@ import java.util.Set;
 import org.springframework.core.convert.TypeDescriptor;
 import org.springframework.core.convert.converter.ConditionalGenericConverter;
 import org.springframework.core.convert.converter.Converter;
+import org.springframework.data.repository.util.QueryExecutionConverters.WrapperType;
 import org.springframework.util.ReflectionUtils;
 
 /**
@@ -42,8 +43,8 @@ class JavaslangCollections {
 
 		INSTANCE;
 
-		public Class<?> getWrapperType() {
-			return javaslang.collection.Traversable.class;
+		public WrapperType getWrapperType() {
+			return WrapperType.multiValue(javaslang.collection.Traversable.class);
 		}
 
 		/* 

src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java

@@ -16,6 +16,9 @@
 package org.springframework.data.repository.util;
 
 import javaslang.collection.Traversable;
+import lombok.AccessLevel;
+import lombok.RequiredArgsConstructor;
+import lombok.Value;
 import scala.Function0;
 import scala.Option;
 import scala.runtime.AbstractFunction0;
@@ -70,13 +73,13 @@ public abstract class QueryExecutionConverters {
 	private static final boolean JAVASLANG_PRESENT = ClassUtils.isPresent("javaslang.control.Option",
 			QueryExecutionConverters.class.getClassLoader());
 
-	private static final Set<Class<?>> WRAPPER_TYPES = new HashSet<Class<?>>();
+	private static final Set<WrapperType> WRAPPER_TYPES = new HashSet<WrapperType>();
 	private static final Set<Converter<Object, Object>> UNWRAPPERS = new HashSet<Converter<Object, Object>>();
 
 	static {
 
-		WRAPPER_TYPES.add(Future.class);
-		WRAPPER_TYPES.add(ListenableFuture.class);
+		WRAPPER_TYPES.add(WrapperType.singleValue(Future.class));
+		WRAPPER_TYPES.add(WrapperType.singleValue(ListenableFuture.class));
 
 		if (GUAVA_PRESENT) {
 			WRAPPER_TYPES.add(NullableWrapperToGuavaOptionalConverter.getWrapperType());
@@ -118,8 +121,8 @@ public abstract class QueryExecutionConverters {
 
 		Assert.notNull(type, "Type must not be null!");
 
-		for (Class<?> candidate : WRAPPER_TYPES) {
-			if (candidate.isAssignableFrom(type)) {
+		for (WrapperType candidate : WRAPPER_TYPES) {
+			if (candidate.getType().isAssignableFrom(type)) {
 				return true;
 			}
 		}
@@ -127,6 +130,17 @@ public abstract class QueryExecutionConverters {
 		return false;
 	}
 
+	public static boolean isSingleValue(Class<?> type) {
+
+		for (WrapperType candidate : WRAPPER_TYPES) {
+			if (candidate.getType().isAssignableFrom(type)) {
+				return candidate.isSingleValue();
+			}
+		}
+
+		return false;
+	}
+
 	/**
 	 * Registers converters for wrapper types found on the classpath.
 	 * 
@@ -277,8 +291,8 @@ public abstract class QueryExecutionConverters {
 			return Optional.of(source);
 		}
 
-		public static Class<?> getWrapperType() {
-			return Optional.class;
+		public static WrapperType getWrapperType() {
+			return WrapperType.singleValue(Optional.class);
 		}
 	}
 
@@ -307,8 +321,8 @@ public abstract class QueryExecutionConverters {
 			return java.util.Optional.of(source);
 		}
 
-		public static Class<?> getWrapperType() {
-			return java.util.Optional.class;
+		public static WrapperType getWrapperType() {
+			return WrapperType.singleValue(java.util.Optional.class);
 		}
 	}
 
@@ -363,8 +377,8 @@ public abstract class QueryExecutionConverters {
 			return source instanceof CompletableFuture ? source : CompletableFuture.completedFuture(source);
 		}
 
-		public static Class<?> getWrapperType() {
-			return CompletableFuture.class;
+		public static WrapperType getWrapperType() {
+			return WrapperType.singleValue(CompletableFuture.class);
 		}
 	}
 
@@ -389,8 +403,8 @@ public abstract class QueryExecutionConverters {
 			return Option.apply(source);
 		}
 
-		public static Class<?> getWrapperType() {
-			return Option.class;
+		public static WrapperType getWrapperType() {
+			return WrapperType.singleValue(Option.class);
 		}
 	}
 
@@ -406,8 +420,8 @@ public abstract class QueryExecutionConverters {
 		private static final Method NONE_METHOD;
 
 		static {
-			OF_METHOD = ReflectionUtils.findMethod(getWrapperType(), "of", Object.class);
-			NONE_METHOD = ReflectionUtils.findMethod(getWrapperType(), "none");
+			OF_METHOD = ReflectionUtils.findMethod(javaslang.control.Option.class, "of", Object.class);
+			NONE_METHOD = ReflectionUtils.findMethod(javaslang.control.Option.class, "none");
 		}
 
 		/**
@@ -416,11 +430,11 @@ public abstract class QueryExecutionConverters {
 		 * @param conversionService must not be {@literal null}.
 		 */
 		public NullableWrapperToJavaslangOptionConverter(ConversionService conversionService) {
-			super(conversionService, createEmptyOption(), getWrapperType());
+			super(conversionService, createEmptyOption(), javaslang.control.Option.class);
 		}
 
-		public static Class<?> getWrapperType() {
-			return javaslang.control.Option.class;
+		public static WrapperType getWrapperType() {
+			return WrapperType.singleValue(javaslang.control.Option.class);
 		}
 
 		/* 
@@ -552,4 +566,20 @@ public abstract class QueryExecutionConverters {
 			return source;
 		}
 	}
+
+	@Value
+	@RequiredArgsConstructor(access = AccessLevel.PRIVATE)
+	public static class WrapperType {
+
+		Class<?> type;
+		boolean singleValue;
+
+		public static WrapperType singleValue(Class<?> type) {
+			return new WrapperType(type, true);
+		}
+
+		public static WrapperType multiValue(Class<?> type) {
+			return new WrapperType(type, false);
+		}
+	}
 }

src/test/java/org/springframework/data/repository/query/QueryMethodUnitTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2008-2015 the original author or authors.
+ * Copyright 2008-2016 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -19,6 +19,9 @@ import static org.hamcrest.CoreMatchers.*;
 import static org.junit.Assert.*;
 import static org.junit.Assume.*;
 
+import javaslang.collection.Seq;
+import javaslang.control.Option;
+
 import java.io.Serializable;
 import java.lang.reflect.Method;
 import java.util.List;
@@ -226,6 +229,42 @@ public class QueryMethodUnitTests {
 		assertThat(new QueryMethod(method, repositoryMetadata, factory).isCollectionQuery(), is(true));
 	}
 
+	/**
+	 * @see DATACMNS-940
+	 */
+	@Test
+	public void detectsCustomCollectionReturnType() throws Exception {
+
+		RepositoryMetadata repositoryMetadata = new DefaultRepositoryMetadata(SampleRepository.class);
+		Method method = SampleRepository.class.getMethod("returnsSeq");
+
+		assertThat(new QueryMethod(method, repositoryMetadata, factory).isCollectionQuery(), is(true));
+	}
+
+	/**
+	 * @see DATACMNS-940
+	 */
+	@Test
+	public void detectsWrapperWithinWrapper() throws Exception {
+
+		RepositoryMetadata repositoryMetadata = new DefaultRepositoryMetadata(SampleRepository.class);
+		Method method = SampleRepository.class.getMethod("returnsFutureOfSeq");
+
+		assertThat(new QueryMethod(method, repositoryMetadata, factory).isCollectionQuery(), is(true));
+	}
+
+	/**
+	 * @see DATACMNS-940
+	 */
+	@Test
+	public void detectsSinglValueWrapperWithinWrapper() throws Exception {
+
+		RepositoryMetadata repositoryMetadata = new DefaultRepositoryMetadata(SampleRepository.class);
+		Method method = SampleRepository.class.getMethod("returnsFutureOfOption");
+
+		assertThat(new QueryMethod(method, repositoryMetadata, factory).isCollectionQuery(), is(false));
+	}
+
 	interface SampleRepository extends Repository<User, Serializable> {
 
 		String pagingMethodWithInvalidReturnType(Pageable pageable);
@@ -269,6 +308,12 @@ public class QueryMethodUnitTests {
 		 * @see DATACMNS-716
 		 */
 		Future<List<User>> returnsFutureForEntityCollection();
+
+		Seq<User> returnsSeq();
+
+		Future<Seq<User>> returnsFutureOfSeq();
+
+		Future<Option<User>> returnsFutureOfOption();
 	}
 
 	class User {