Skip to content

Commit b194f77

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 a3afa84 commit b194f77

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,11 +15,14 @@
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.Getter;
2122
import lombok.RequiredArgsConstructor;
2223
import lombok.Value;
24+
import org.springframework.data.domain.Page;
25+
import org.springframework.data.domain.Slice;
2326
import scala.Function0;
2427
import scala.Option;
2528
import scala.runtime.AbstractFunction0;
@@ -28,6 +31,7 @@
2831
import java.util.Collection;
2932
import java.util.Collections;
3033
import java.util.HashSet;
34+
import java.util.List;
3135
import java.util.Set;
3236
import java.util.concurrent.CompletableFuture;
3337
import java.util.concurrent.Future;
@@ -80,6 +84,7 @@ public abstract class QueryExecutionConverters {
8084
private static final Set<WrapperType> WRAPPER_TYPES = new HashSet<WrapperType>();
8185
private static final Set<WrapperType> UNWRAPPER_TYPES = new HashSet<WrapperType>();
8286
private static final Set<Converter<Object, Object>> UNWRAPPERS = new HashSet<Converter<Object, Object>>();
87+
private static final Set<Class<?>> ALLOWED_PAGEABLE_TYPES = new HashSet<Class<?>>();
8388

8489
static {
8590

@@ -88,6 +93,10 @@ public abstract class QueryExecutionConverters {
8893
WRAPPER_TYPES.add(WrapperType.singleValue(ListenableFuture.class));
8994
UNWRAPPER_TYPES.add(WrapperType.singleValue(ListenableFuture.class));
9095

96+
ALLOWED_PAGEABLE_TYPES.add(Slice.class);
97+
ALLOWED_PAGEABLE_TYPES.add(Page.class);
98+
ALLOWED_PAGEABLE_TYPES.add(List.class);
99+
91100
if (GUAVA_PRESENT) {
92101
WRAPPER_TYPES.add(NullableWrapperToGuavaOptionalConverter.getWrapperType());
93102
UNWRAPPER_TYPES.add(NullableWrapperToGuavaOptionalConverter.getWrapperType());
@@ -117,6 +126,8 @@ public abstract class QueryExecutionConverters {
117126
WRAPPER_TYPES.add(JavaslangCollections.ToJavaConverter.INSTANCE.getWrapperType());
118127

119128
UNWRAPPERS.add(JavaslangOptionUnwrapper.INSTANCE);
129+
130+
ALLOWED_PAGEABLE_TYPES.add(Seq.class);
120131
}
121132

122133
if (ReactiveWrappers.isAvailable()) {
@@ -180,6 +191,10 @@ public static boolean isSingleValue(Class<?> type) {
180191
return false;
181192
}
182193

194+
public static Set<Class<?>> getAllowedPageableTypes() {
195+
return Collections.unmodifiableSet(ALLOWED_PAGEABLE_TYPES);
196+
}
197+
183198
/**
184199
* Registers converters for wrapper types found on the classpath.
185200
*

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

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

223+
@Test // DATACMNS-1005
224+
public void doesNotRejectSeqForPagination() throws Exception {
225+
226+
RepositoryMetadata repositoryMetadata = new DefaultRepositoryMetadata(SampleRepository.class);
227+
Method method = SampleRepository.class.getMethod("returnsSeq", Pageable.class);
228+
229+
assertThat(new QueryMethod(method, repositoryMetadata, factory).isCollectionQuery(), is(true));
230+
}
231+
223232
interface SampleRepository extends Repository<User, Serializable> {
224233

225234
String pagingMethodWithInvalidReturnType(Pageable pageable);
@@ -258,6 +267,9 @@ interface SampleRepository extends Repository<User, Serializable> {
258267

259268
Seq<User> returnsSeq();
260269

270+
// DATACMNS-1005
271+
Seq<User> returnsSeq(Pageable pageable);
272+
261273
Future<Seq<User>> returnsFutureOfSeq();
262274

263275
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 reactor.core.publisher.Flux;
2629
import reactor.core.publisher.Mono;
2730
import rx.Completable;
@@ -267,6 +270,16 @@ public void unwrapsJavaslangCollectionsToJavaOnes() {
267270
assertThat(unwrap(javaslangMap("key", "value")), is(instanceOf(Map.class)));
268271
}
269272

273+
@Test // DATACMNS-1005
274+
public void registersAllowedPageabletypes() {
275+
276+
final Set<Class<?>> allowedPageableTypes = QueryExecutionConverters.getAllowedPageableTypes();
277+
assertThat(allowedPageableTypes, hasItem(Page.class));
278+
assertThat(allowedPageableTypes, hasItem(Slice.class));
279+
assertThat(allowedPageableTypes, hasItem(List.class));
280+
assertThat(allowedPageableTypes, hasItem(Seq.class));
281+
}
282+
270283
@SuppressWarnings("unchecked")
271284
private static javaslang.control.Option<Object> optionNone() {
272285

0 commit comments

Comments
 (0)