From b194f7794d88a42b9839c7401ef0f5bb2455a807 Mon Sep 17 00:00:00 2001 From: Maciek Opala Date: Tue, 7 Mar 2017 20:08:29 +0100 Subject: [PATCH] DATACMNS-1005 - Paginated query methods now support Javaslang collection types. To eagerly catch invalid declarations, query methods using pagination had a check applied that the method either returns a Page or List. We've introduced support for alternative collection libraries (Javaslang in particular) and thus the check needs to take these newly supported types into account. This is now fixed by externalizing the valid types into QueryExecutionConverters and using the ones returned by that to check against. Original pull request: #200. --- .../data/repository/query/QueryMethod.java | 26 ++++++++++++++++--- .../util/QueryExecutionConverters.java | 15 +++++++++++ .../query/QueryMethodUnitTests.java | 12 +++++++++ .../QueryExecutionConvertersUnitTests.java | 13 ++++++++++ 4 files changed, 63 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/springframework/data/repository/query/QueryMethod.java b/src/main/java/org/springframework/data/repository/query/QueryMethod.java index 6aee5a010..6d93bdd51 100644 --- a/src/main/java/org/springframework/data/repository/query/QueryMethod.java +++ b/src/main/java/org/springframework/data/repository/query/QueryMethod.java @@ -18,7 +18,8 @@ package org.springframework.data.repository.query; import static org.springframework.data.repository.util.ClassUtils.*; import java.lang.reflect.Method; -import java.util.List; +import java.util.Arrays; +import java.util.Set; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; @@ -30,6 +31,7 @@ import org.springframework.data.repository.core.RepositoryMetadata; import org.springframework.data.repository.util.QueryExecutionConverters; import org.springframework.data.util.ClassTypeInformation; import org.springframework.data.util.ReflectionUtils; +import org.springframework.data.util.TypeInformation; import org.springframework.util.Assert; /** @@ -78,7 +80,8 @@ public class QueryMethod { if (hasParameterOfType(method, Pageable.class)) { if (!isStreamQuery()) { - assertReturnTypeAssignable(method, Slice.class, Page.class, List.class); + final Set> allowedPageableTypes = QueryExecutionConverters.getAllowedPageableTypes(); + assertReturnTypeAssignable(method, allowedPageableTypes.toArray(new Class[allowedPageableTypes.size()])); } if (hasParameterOfType(method, Sort.class)) { @@ -275,4 +278,21 @@ public class QueryMethod { return method.getReturnType(); } -} + + private static void assertReturnTypeAssignable(Method method, 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; + + 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)); + } +} \ No newline at end of file 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 232c01262..cfdb43593 100644 --- a/src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java +++ b/src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java @@ -15,11 +15,14 @@ */ package org.springframework.data.repository.util; +import javaslang.collection.Seq; import javaslang.collection.Traversable; import lombok.AccessLevel; import lombok.Getter; 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; @@ -28,6 +31,7 @@ import java.lang.reflect.Method; import java.util.Collection; import java.util.Collections; import java.util.HashSet; +import java.util.List; import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Future; @@ -80,6 +84,7 @@ public abstract class QueryExecutionConverters { private static final Set WRAPPER_TYPES = new HashSet(); private static final Set UNWRAPPER_TYPES = new HashSet(); private static final Set> UNWRAPPERS = new HashSet>(); + private static final Set> ALLOWED_PAGEABLE_TYPES = new HashSet>(); static { @@ -88,6 +93,10 @@ public abstract class QueryExecutionConverters { WRAPPER_TYPES.add(WrapperType.singleValue(ListenableFuture.class)); UNWRAPPER_TYPES.add(WrapperType.singleValue(ListenableFuture.class)); + ALLOWED_PAGEABLE_TYPES.add(Slice.class); + ALLOWED_PAGEABLE_TYPES.add(Page.class); + ALLOWED_PAGEABLE_TYPES.add(List.class); + if (GUAVA_PRESENT) { WRAPPER_TYPES.add(NullableWrapperToGuavaOptionalConverter.getWrapperType()); UNWRAPPER_TYPES.add(NullableWrapperToGuavaOptionalConverter.getWrapperType()); @@ -117,6 +126,8 @@ public abstract class QueryExecutionConverters { WRAPPER_TYPES.add(JavaslangCollections.ToJavaConverter.INSTANCE.getWrapperType()); UNWRAPPERS.add(JavaslangOptionUnwrapper.INSTANCE); + + ALLOWED_PAGEABLE_TYPES.add(Seq.class); } if (ReactiveWrappers.isAvailable()) { @@ -180,6 +191,10 @@ public abstract class QueryExecutionConverters { return false; } + public static Set> getAllowedPageableTypes() { + return Collections.unmodifiableSet(ALLOWED_PAGEABLE_TYPES); + } + /** * Registers converters for wrapper types found on the classpath. * diff --git a/src/test/java/org/springframework/data/repository/query/QueryMethodUnitTests.java b/src/test/java/org/springframework/data/repository/query/QueryMethodUnitTests.java index e27f663cd..98b035d38 100644 --- a/src/test/java/org/springframework/data/repository/query/QueryMethodUnitTests.java +++ b/src/test/java/org/springframework/data/repository/query/QueryMethodUnitTests.java @@ -220,6 +220,15 @@ public class QueryMethodUnitTests { assertThat(new QueryMethod(method, repositoryMetadata, factory).isCollectionQuery(), is(false)); } + @Test // DATACMNS-1005 + public void doesNotRejectSeqForPagination() throws Exception { + + RepositoryMetadata repositoryMetadata = new DefaultRepositoryMetadata(SampleRepository.class); + Method method = SampleRepository.class.getMethod("returnsSeq", Pageable.class); + + assertThat(new QueryMethod(method, repositoryMetadata, factory).isCollectionQuery(), is(true)); + } + interface SampleRepository extends Repository { String pagingMethodWithInvalidReturnType(Pageable pageable); @@ -258,6 +267,9 @@ public class QueryMethodUnitTests { Seq returnsSeq(); + // DATACMNS-1005 + Seq returnsSeq(Pageable pageable); + Future> returnsFutureOfSeq(); Future> returnsFutureOfOption(); 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 e86f28721..6f1de0a15 100644 --- a/src/test/java/org/springframework/data/repository/util/QueryExecutionConvertersUnitTests.java +++ b/src/test/java/org/springframework/data/repository/util/QueryExecutionConvertersUnitTests.java @@ -21,7 +21,10 @@ 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 reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import rx.Completable; @@ -267,6 +270,16 @@ public class QueryExecutionConvertersUnitTests { assertThat(unwrap(javaslangMap("key", "value")), is(instanceOf(Map.class))); } + @Test // DATACMNS-1005 + public void registersAllowedPageabletypes() { + + final Set> allowedPageableTypes = QueryExecutionConverters.getAllowedPageableTypes(); + assertThat(allowedPageableTypes, hasItem(Page.class)); + assertThat(allowedPageableTypes, hasItem(Slice.class)); + assertThat(allowedPageableTypes, hasItem(List.class)); + assertThat(allowedPageableTypes, hasItem(Seq.class)); + } + @SuppressWarnings("unchecked") private static javaslang.control.Option optionNone() {