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 12a28e4e9..6aee5a010 100644 --- a/src/main/java/org/springframework/data/repository/query/QueryMethod.java +++ b/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(); } diff --git a/src/main/java/org/springframework/data/repository/util/JavaslangCollections.java b/src/main/java/org/springframework/data/repository/util/JavaslangCollections.java index 3d1fbf847..6ee4b4221 100644 --- a/src/main/java/org/springframework/data/repository/util/JavaslangCollections.java +++ b/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); } /* 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 88a07f24f..232c01262 100644 --- a/src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java +++ b/src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java @@ -16,6 +16,10 @@ package org.springframework.data.repository.util; import javaslang.collection.Traversable; +import lombok.AccessLevel; +import lombok.Getter; +import lombok.RequiredArgsConstructor; +import lombok.Value; import scala.Function0; import scala.Option; import scala.runtime.AbstractFunction0; @@ -28,6 +32,7 @@ import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Future; import java.util.function.Supplier; +import java.util.stream.Collectors; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.TypeDescriptor; @@ -72,16 +77,16 @@ public abstract class QueryExecutionConverters { private static final boolean JAVASLANG_PRESENT = ClassUtils.isPresent("javaslang.control.Option", QueryExecutionConverters.class.getClassLoader()); - private static final Set> WRAPPER_TYPES = new HashSet>(); - private static final Set> UNWRAPPER_TYPES = new HashSet>(); + private static final Set WRAPPER_TYPES = new HashSet(); + private static final Set UNWRAPPER_TYPES = new HashSet(); private static final Set> UNWRAPPERS = new HashSet>(); static { - WRAPPER_TYPES.add(Future.class); - UNWRAPPER_TYPES.add(Future.class); - WRAPPER_TYPES.add(ListenableFuture.class); - UNWRAPPER_TYPES.add(ListenableFuture.class); + WRAPPER_TYPES.add(WrapperType.singleValue(Future.class)); + UNWRAPPER_TYPES.add(WrapperType.singleValue(Future.class)); + WRAPPER_TYPES.add(WrapperType.singleValue(ListenableFuture.class)); + UNWRAPPER_TYPES.add(WrapperType.singleValue(ListenableFuture.class)); if (GUAVA_PRESENT) { WRAPPER_TYPES.add(NullableWrapperToGuavaOptionalConverter.getWrapperType()); @@ -115,9 +120,12 @@ public abstract class QueryExecutionConverters { } if (ReactiveWrappers.isAvailable()) { - WRAPPER_TYPES.addAll(ReactiveWrappers.getNoValueTypes()); - WRAPPER_TYPES.addAll(ReactiveWrappers.getSingleValueTypes()); - WRAPPER_TYPES.addAll(ReactiveWrappers.getMultiValueTypes()); + WRAPPER_TYPES + .addAll(ReactiveWrappers.getNoValueTypes().stream().map(WrapperType::noValue).collect(Collectors.toList())); + WRAPPER_TYPES.addAll( + ReactiveWrappers.getSingleValueTypes().stream().map(WrapperType::singleValue).collect(Collectors.toList())); + WRAPPER_TYPES.addAll( + ReactiveWrappers.getMultiValueTypes().stream().map(WrapperType::multiValue).collect(Collectors.toList())); } } @@ -133,8 +141,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; } } @@ -152,8 +160,8 @@ public abstract class QueryExecutionConverters { Assert.notNull(type, "Type must not be null!"); - for (Class candidate : UNWRAPPER_TYPES) { - if (candidate.isAssignableFrom(type)) { + for (WrapperType candidate : UNWRAPPER_TYPES) { + if (candidate.getType().isAssignableFrom(type)) { return true; } } @@ -161,6 +169,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. * @@ -311,8 +330,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); } } @@ -341,8 +360,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); } } @@ -397,8 +416,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); } } @@ -423,8 +442,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); } } @@ -440,8 +459,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"); } /** @@ -450,11 +469,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); } /* @@ -586,4 +605,32 @@ public abstract class QueryExecutionConverters { return source; } } + + @Value + @RequiredArgsConstructor(access = AccessLevel.PRIVATE) + public static class WrapperType { + + enum Cardinality { + NONE, SINGLE, MULTI; + } + + Class type; + @Getter(AccessLevel.NONE) Cardinality cardinality; + + public static WrapperType singleValue(Class type) { + return new WrapperType(type, Cardinality.SINGLE); + } + + public static WrapperType multiValue(Class type) { + return new WrapperType(type, Cardinality.MULTI); + } + + public static WrapperType noValue(Class type) { + return new WrapperType(type, Cardinality.NONE); + } + + boolean isSingleValue() { + return cardinality.equals(Cardinality.SINGLE); + } + } } 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 6ab178168..e27f663cd 100644 --- a/src/test/java/org/springframework/data/repository/query/QueryMethodUnitTests.java +++ b/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; @@ -181,6 +184,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 { String pagingMethodWithInvalidReturnType(Pageable pageable); @@ -216,6 +255,12 @@ public class QueryMethodUnitTests { // DATACMNS-716 Future> returnsFutureForEntityCollection(); + + Seq returnsSeq(); + + Future> returnsFutureOfSeq(); + + Future> returnsFutureOfOption(); } class User {