Skip to content

Commit 112bd60

Browse files
Opaloodrotbohm
authored andcommitted
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.
1 parent da9d7d0 commit 112bd60

File tree

4 files changed

+63
-3
lines changed

4 files changed

+63
-3
lines changed

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

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@
1818
import static org.springframework.data.repository.util.ClassUtils.*;
1919

2020
import java.lang.reflect.Method;
21-
import java.util.List;
21+
import java.util.Arrays;
22+
import java.util.Set;
2223

2324
import org.springframework.data.domain.Page;
2425
import org.springframework.data.domain.Pageable;
@@ -30,6 +31,7 @@
3031
import org.springframework.data.repository.util.QueryExecutionConverters;
3132
import org.springframework.data.util.ClassTypeInformation;
3233
import org.springframework.data.util.ReflectionUtils;
34+
import org.springframework.data.util.TypeInformation;
3335
import org.springframework.util.Assert;
3436

3537
/**
@@ -78,7 +80,8 @@ public QueryMethod(Method method, RepositoryMetadata metadata, ProjectionFactory
7880
if (hasParameterOfType(method, Pageable.class)) {
7981

8082
if (!isStreamQuery()) {
81-
assertReturnTypeAssignable(method, Slice.class, Page.class, List.class);
83+
final Set<Class<?>> allowedPageableTypes = QueryExecutionConverters.getAllowedPageableTypes();
84+
assertReturnTypeAssignable(method, allowedPageableTypes.toArray(new Class<?>[allowedPageableTypes.size()]));
8285
}
8386

8487
if (hasParameterOfType(method, Sort.class)) {
@@ -275,4 +278,21 @@ private static Class<? extends Object> potentiallyUnwrapReturnTypeFor(Method met
275278

276279
return method.getReturnType();
277280
}
278-
}
281+
282+
private static void assertReturnTypeAssignable(Method method, Class<?>... types) {
283+
284+
Assert.notNull(method, "Method must not be null!");
285+
Assert.notEmpty(types, "Types must not be null or empty!");
286+
287+
TypeInformation<?> returnType = ClassTypeInformation.fromReturnTypeOf(method);
288+
returnType = QueryExecutionConverters.isSingleValue(returnType.getType()) ? returnType.getComponentType() : returnType;
289+
290+
for (Class<?> type : types) {
291+
if (type.isAssignableFrom(returnType.getType())) {
292+
return;
293+
}
294+
}
295+
296+
throw new IllegalStateException("Method has to have one of the following return types! " + Arrays.toString(types));
297+
}
298+
}

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,13 @@
1515
*/
1616
package org.springframework.data.repository.util;
1717

18+
import javaslang.collection.Seq;
1819
import javaslang.collection.Traversable;
1920
import lombok.AccessLevel;
2021
import lombok.RequiredArgsConstructor;
2122
import lombok.Value;
23+
import org.springframework.data.domain.Page;
24+
import org.springframework.data.domain.Slice;
2225
import scala.Function0;
2326
import scala.Option;
2427
import scala.runtime.AbstractFunction0;
@@ -27,6 +30,7 @@
2730
import java.util.Collection;
2831
import java.util.Collections;
2932
import java.util.HashSet;
33+
import java.util.List;
3034
import java.util.Set;
3135
import java.util.concurrent.CompletableFuture;
3236
import java.util.concurrent.Future;
@@ -75,12 +79,17 @@ public abstract class QueryExecutionConverters {
7579

7680
private static final Set<WrapperType> WRAPPER_TYPES = new HashSet<WrapperType>();
7781
private static final Set<Converter<Object, Object>> UNWRAPPERS = new HashSet<Converter<Object, Object>>();
82+
private static final Set<Class<?>> ALLOWED_PAGEABLE_TYPES = new HashSet<Class<?>>();
7883

7984
static {
8085

8186
WRAPPER_TYPES.add(WrapperType.singleValue(Future.class));
8287
WRAPPER_TYPES.add(WrapperType.singleValue(ListenableFuture.class));
8388

89+
ALLOWED_PAGEABLE_TYPES.add(Slice.class);
90+
ALLOWED_PAGEABLE_TYPES.add(Page.class);
91+
ALLOWED_PAGEABLE_TYPES.add(List.class);
92+
8493
if (GUAVA_PRESENT) {
8594
WRAPPER_TYPES.add(NullableWrapperToGuavaOptionalConverter.getWrapperType());
8695
UNWRAPPERS.add(GuavaOptionalUnwrapper.INSTANCE);
@@ -106,6 +115,8 @@ public abstract class QueryExecutionConverters {
106115
WRAPPER_TYPES.add(JavaslangCollections.ToJavaConverter.INSTANCE.getWrapperType());
107116

108117
UNWRAPPERS.add(JavaslangOptionUnwrapper.INSTANCE);
118+
119+
ALLOWED_PAGEABLE_TYPES.add(Seq.class);
109120
}
110121
}
111122

@@ -141,6 +152,10 @@ public static boolean isSingleValue(Class<?> type) {
141152
return false;
142153
}
143154

155+
public static Set<Class<?>> getAllowedPageableTypes() {
156+
return Collections.unmodifiableSet(ALLOWED_PAGEABLE_TYPES);
157+
}
158+
144159
/**
145160
* Registers converters for wrapper types found on the classpath.
146161
*

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,15 @@ public void detectsSinglValueWrapperWithinWrapper() throws Exception {
211211
assertThat(new QueryMethod(method, repositoryMetadata, factory).isCollectionQuery(), is(false));
212212
}
213213

214+
@Test // DATACMNS-1005
215+
public void doesNotRejectSeqForPagination() throws Exception {
216+
217+
RepositoryMetadata repositoryMetadata = new DefaultRepositoryMetadata(SampleRepository.class);
218+
Method method = SampleRepository.class.getMethod("returnsSeq", Pageable.class);
219+
220+
assertThat(new QueryMethod(method, repositoryMetadata, factory).isCollectionQuery(), is(true));
221+
}
222+
214223
interface SampleRepository extends Repository<User, Serializable> {
215224

216225
String pagingMethodWithInvalidReturnType(Pageable pageable);
@@ -249,6 +258,9 @@ interface SampleRepository extends Repository<User, Serializable> {
249258

250259
Seq<User> returnsSeq();
251260

261+
// DATACMNS-1005
262+
Seq<User> returnsSeq(Pageable pageable);
263+
252264
Future<Seq<User>> returnsFutureOfSeq();
253265

254266
Future<Option<User>> returnsFutureOfOption();

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@
2121

2222
import javaslang.collection.HashMap;
2323
import javaslang.collection.HashSet;
24+
import javaslang.collection.Seq;
2425
import javaslang.collection.Traversable;
26+
import org.springframework.data.domain.Page;
27+
import org.springframework.data.domain.Slice;
2528
import scala.Option;
2629

2730
import java.lang.reflect.Method;
@@ -219,6 +222,16 @@ public void unwrapsJavaslangCollectionsToJavaOnes() {
219222
assertThat(unwrap(javaslangMap("key", "value")), is(instanceOf(Map.class)));
220223
}
221224

225+
@Test // DATACMNS-1005
226+
public void registersAllowedPageabletypes() {
227+
228+
final Set<Class<?>> allowedPageableTypes = QueryExecutionConverters.getAllowedPageableTypes();
229+
assertThat(allowedPageableTypes, hasItem(Page.class));
230+
assertThat(allowedPageableTypes, hasItem(Slice.class));
231+
assertThat(allowedPageableTypes, hasItem(List.class));
232+
assertThat(allowedPageableTypes, hasItem(Seq.class));
233+
}
234+
222235
@SuppressWarnings("unchecked")
223236
private static javaslang.control.Option<Object> optionNone() {
224237

0 commit comments

Comments
 (0)