DATACMNS-1005 - Polishing.

Simplified type check by using Set as method signature to avoid unnecessary manual array wrapping. Simplification in the test assertions. A bit of Javadoc, corrected imports and author tags.

Original pull request: #200.

Author: odrotbohm

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2008-2015 the original author or authors.
+ * Copyright 2008-2017 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.
@@ -18,7 +18,6 @@
 import static org.springframework.data.repository.util.ClassUtils.*;
 
 import java.lang.reflect.Method;
-import java.util.Arrays;
 import java.util.Set;
 
 import org.springframework.data.domain.Page;
@@ -40,6 +39,7 @@
  * 
  * @author Oliver Gierke
  * @author Thomas Darimont
+ * @author Maciek Opała
  */
 public class QueryMethod {
 
@@ -80,8 +80,7 @@ public QueryMethod(Method method, RepositoryMetadata metadata, ProjectionFactory
 		if (hasParameterOfType(method, Pageable.class)) {
 
 			if (!isStreamQuery()) {
-				final Set<Class<?>> allowedPageableTypes = QueryExecutionConverters.getAllowedPageableTypes();
-				assertReturnTypeAssignable(method, allowedPageableTypes.toArray(new Class<?>[allowedPageableTypes.size()]));
+				assertReturnTypeAssignable(method, QueryExecutionConverters.getAllowedPageableTypes());
 			}
 
 			if (hasParameterOfType(method, Sort.class)) {
@@ -90,7 +89,8 @@ public QueryMethod(Method method, RepositoryMetadata metadata, ProjectionFactory
 			}
 		}
 
-		Assert.notNull(this.parameters, String.format("Parameters extracted from method '%s' must not be null!", method.getName()));
+		Assert.notNull(this.parameters,
+				String.format("Parameters extracted from method '%s' must not be null!", method.getName()));
 
 		if (isPageQuery()) {
 			Assert.isTrue(this.parameters.hasPageableParameter(),
@@ -279,20 +279,21 @@ private static Class<? extends Object> potentiallyUnwrapReturnTypeFor(Method met
 		return method.getReturnType();
 	}
 
-	private static void assertReturnTypeAssignable(Method method, Class<?>... types) {
+	private static void assertReturnTypeAssignable(Method method, Set<Class<?>> types) {
 
 		Assert.notNull(method, "Method must not be null!");
 		Assert.notEmpty(types, "Types must not be null or empty!");
 
 		TypeInformation<?> returnType = ClassTypeInformation.fromReturnTypeOf(method);
-		returnType = QueryExecutionConverters.isSingleValue(returnType.getType()) ? returnType.getComponentType() : returnType;
+		returnType = QueryExecutionConverters.isSingleValue(returnType.getType()) ? returnType.getComponentType()
+				: returnType;
 
 		for (Class<?> type : types) {
 			if (type.isAssignableFrom(returnType.getType())) {
 				return;
 			}
 		}
 
-		throw new IllegalStateException("Method has to have one of the following return types! " + Arrays.toString(types));
+		throw new IllegalStateException("Method has to have one of the following return types! " + types.toString());
 	}
-}
+}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2014-2016 the original author or authors.
+ * Copyright 2014-2017 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.
@@ -20,8 +20,6 @@
 import lombok.AccessLevel;
 import lombok.RequiredArgsConstructor;
 import lombok.Value;
-import org.springframework.data.domain.Page;
-import org.springframework.data.domain.Slice;
 import scala.Function0;
 import scala.Option;
 import scala.runtime.AbstractFunction0;
@@ -41,6 +39,8 @@
 import org.springframework.core.convert.converter.Converter;
 import org.springframework.core.convert.converter.GenericConverter;
 import org.springframework.core.convert.support.ConfigurableConversionService;
+import org.springframework.data.domain.Page;
+import org.springframework.data.domain.Slice;
 import org.springframework.scheduling.annotation.AsyncResult;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
@@ -60,10 +60,13 @@
  * <li>{@code java.util.concurrent.CompletableFuture}</li>
  * <li>{@code org.springframework.util.concurrent.ListenableFuture<}</li>
  * <li>{@code javaslang.control.Option} - as of 1.13</li>
+ * <li>{@code javaslang.collection.Seq}, {@code javaslang.collection.Map}, {@code javaslang.collection.Set} - as of
+ * 1.13</li>
  * </ul>
  * 
  * @author Oliver Gierke
  * @author Mark Paluch
+ * @author Maciek Opała
  * @since 1.8
  */
 public abstract class QueryExecutionConverters {
@@ -152,6 +155,12 @@ public static boolean isSingleValue(Class<?> type) {
 		return false;
 	}
 
+	/**
+	 * Returns the types that are supported on paginating query methods. Will include custom collection types of e.g.
+	 * Javaslang.
+	 * 
+	 * @return
+	 */
 	public static Set<Class<?>> getAllowedPageableTypes() {
 		return Collections.unmodifiableSet(ALLOWED_PAGEABLE_TYPES);
 	}

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

@@ -46,6 +46,7 @@
  * 
  * @author Oliver Gierke
  * @author Thomas Darimont
+ * @author Maciek Opała
  */
 public class QueryMethodUnitTests {
 

src/test/java/org/springframework/data/repository/util/QueryExecutionConvertersUnitTests.java

@@ -15,16 +15,14 @@
  */
 package org.springframework.data.repository.util;
 
-import static org.hamcrest.CoreMatchers.*;
+import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
 import static org.springframework.data.repository.util.QueryExecutionConverters.*;
 
 import javaslang.collection.HashMap;
 import javaslang.collection.HashSet;
 import javaslang.collection.Seq;
 import javaslang.collection.Traversable;
-import org.springframework.data.domain.Page;
-import org.springframework.data.domain.Slice;
 import scala.Option;
 
 import java.lang.reflect.Method;
@@ -36,9 +34,12 @@
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.Future;
 
+import org.hamcrest.Matchers;
 import org.junit.Before;
 import org.junit.Test;
 import org.springframework.core.convert.support.DefaultConversionService;
+import org.springframework.data.domain.Page;
+import org.springframework.data.domain.Slice;
 import org.springframework.util.ReflectionUtils;
 import org.springframework.util.concurrent.ListenableFuture;
 
@@ -49,6 +50,7 @@
  * 
  * @author Oliver Gierke
  * @author Mark Paluch
+ * @author Maciek Opała
  */
 public class QueryExecutionConvertersUnitTests {
 
@@ -225,11 +227,8 @@ public void unwrapsJavaslangCollectionsToJavaOnes() {
 	@Test // DATACMNS-1005
 	public void registersAllowedPageabletypes() {
 
-		final Set<Class<?>> allowedPageableTypes = QueryExecutionConverters.getAllowedPageableTypes();
-		assertThat(allowedPageableTypes, hasItem(Page.class));
-		assertThat(allowedPageableTypes, hasItem(Slice.class));
-		assertThat(allowedPageableTypes, hasItem(List.class));
-		assertThat(allowedPageableTypes, hasItem(Seq.class));
+		Set<Class<?>> allowedPageableTypes = QueryExecutionConverters.getAllowedPageableTypes();
+		assertThat(allowedPageableTypes, Matchers.<Class<?>> hasItems(Page.class, Slice.class, List.class, Seq.class));
 	}
 
 	@SuppressWarnings("unchecked")