From a5408a478d015bb5e83de62c3872119b02ad89a7 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Fri, 26 May 2023 11:25:08 +0200 Subject: [PATCH] Introduce `Limit` type to limit repository query results. We now accept Limit as type to express dynamic repository query limits. Closes #2827 Original pull request: #2836 --- .../asciidoc/repositories-paging-sorting.adoc | 38 +++++- .../springframework/data/domain/Limit.java | 121 ++++++++++++++++++ .../springframework/data/domain/Pageable.java | 14 +- .../springframework/data/domain/Unpaged.java | 18 ++- .../data/repository/query/Parameter.java | 13 +- .../repository/query/ParameterAccessor.java | 17 +++ .../data/repository/query/Parameters.java | 34 ++++- .../query/ParametersParameterAccessor.java | 35 ++++- .../data/repository/query/QueryMethod.java | 95 +++++++++----- .../repository/query/parser/PartTree.java | 11 ++ .../data/repository/util/ClassUtils.java | 13 ++ .../data/domain/LimitUnitTests.java | 56 ++++++++ .../ParametersParameterAccessorUnitTests.java | 97 ++++++++++++++ .../repository/query/ParametersUnitTests.java | 13 ++ .../query/QueryMethodUnitTests.java | 73 +++++++++++ .../query/parser/PartTreeUnitTests.java | 3 + 16 files changed, 604 insertions(+), 47 deletions(-) create mode 100644 src/main/java/org/springframework/data/domain/Limit.java create mode 100644 src/test/java/org/springframework/data/domain/LimitUnitTests.java diff --git a/src/main/asciidoc/repositories-paging-sorting.adoc b/src/main/asciidoc/repositories-paging-sorting.adoc index 58b94fe86..9f372c652 100644 --- a/src/main/asciidoc/repositories-paging-sorting.adoc +++ b/src/main/asciidoc/repositories-paging-sorting.adoc @@ -1,12 +1,12 @@ [[repositories.special-parameters]] -=== Paging, Iterating Large Results, Sorting +=== Paging, Iterating Large Results, Sorting & Limiting To handle parameters in your query, define method parameters as already seen in the preceding examples. -Besides that, the infrastructure recognizes certain specific types like `Pageable` and `Sort`, to apply pagination and sorting to your queries dynamically. +Besides that, the infrastructure recognizes certain specific types like `Pageable`, `Sort` and `Limit`, to apply pagination, sorting and limiting to your queries dynamically. The following example demonstrates these features: ifdef::feature-scroll[] -.Using `Pageable`, `Slice`, `ScrollPosition`, and `Sort` in query methods +.Using `Pageable`, `Slice`, `ScrollPosition`, `Sort` and `Limit` in query methods ==== [source,java] ---- @@ -18,13 +18,15 @@ Window findTop10ByLastname(String lastname, ScrollPosition position, Sort List findByLastname(String lastname, Sort sort); +List findByLastname(String lastname, Sort sort, Limit limit); + List findByLastname(String lastname, Pageable pageable); ---- ==== endif::[] ifndef::feature-scroll[] -.Using `Pageable`, `Slice`, and `Sort` in query methods +.Using `Pageable`, `Slice`, `Sort` and `Limit` in query methods ==== [source,java] ---- @@ -34,13 +36,15 @@ Slice findByLastname(String lastname, Pageable pageable); List findByLastname(String lastname, Sort sort); +List findByLastname(String lastname, Sort sort, Limit limit); + List findByLastname(String lastname, Pageable pageable); ---- ==== endif::[] -IMPORTANT: APIs taking `Sort` and `Pageable` expect non-`null` values to be handed into methods. -If you do not want to apply any sorting or pagination, use `Sort.unsorted()` and `Pageable.unpaged()`. +IMPORTANT: APIs taking `Sort`, `Pageable` and `Limit` expect non-`null` values to be handed into methods. +If you do not want to apply any sorting or pagination, use `Sort.unsorted()`, `Pageable.unpaged()` and `Limit.unlimited()`. The first method lets you pass an `org.springframework.data.domain.Pageable` instance to the query method to dynamically add paging to your statically defined query. A `Page` knows about the total number of elements and pages available. @@ -57,6 +61,28 @@ Rather, it restricts the query to look up only the given range of entities. NOTE: To find out how many pages you get for an entire query, you have to trigger an additional count query. By default, this query is derived from the query you actually trigger. +[IMPORTANT] +==== +Special parameters may only be used once within a query method. + +Some of the special parameters described above are mutually exclusive. +Please consider the following list of invalid parameter combinations. + +|=== +| Parameters | Example | Reason + +| `Pageable` & `Sort` +| `findBy...(Pageable page, Sort sort)` +| `Pageable` already defines `Sort` + +| `Pageable` & `Limit` +| `findBy...(Pageable page, Limit limit)` +| `Pageable` already defines a limit. + +|=== + +The `Top` keyword used to limit results can be used to along with `Pageable` whereas `Top` defines the total maximum of results, whereas the Pageable parameter may reduce this this number. +==== + [[repositories.scrolling.guidance]] ==== Which Method is Appropriate? diff --git a/src/main/java/org/springframework/data/domain/Limit.java b/src/main/java/org/springframework/data/domain/Limit.java new file mode 100644 index 000000000..9cd68cf50 --- /dev/null +++ b/src/main/java/org/springframework/data/domain/Limit.java @@ -0,0 +1,121 @@ +/* + * Copyright 2023 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.domain; + +import org.springframework.data.domain.Limit.Limited; +import org.springframework.data.domain.Limit.Unlimited; +import org.springframework.util.ClassUtils; + +/** + * {@link Limit} represents the maximum value up to which an operation should continue processing. It may be used for + * defining the {@link #max() maximum} number of results within a repository finder method or if applicable a template + * operation. + *

+ * A {@link Limit#isUnlimited()} is used to indicate that there is no {@link Limit} defined, which should be favoured + * over using {@literal null} or {@link java.util.Optional#empty()} to indicate the absence of an actual {@link Limit}. + *

+ * {@link Limit} itself does not make assumptions about the actual {@link #max()} value sign. This means that a negative + * value may be valid within a defined context. Implementations should override {@link #isUnlimited()} to fit their + * needs and enforce restrictions if needed. + * + * @author Christoph Strobl + * @since 3.2 + */ +public sealed interface Limit permits Limited, Unlimited { + + /** + * @return the max number of potential results. + */ + int max(); + + /** + * @return {@literal true} if no limiting (maximum value) should be applied. + */ + default boolean isUnlimited() { + return Unlimited.INSTANCE.equals(this); + } + + /** + * @return a {@link Limit} instance that does not define {@link #max()} and answers {@link #isUnlimited()} with + * {@literal true}. + */ + static Limit unlimited() { + return Unlimited.INSTANCE; + } + + /** + * Create a new {@link Limit} from the given {@literal max} value. + * + * @param max the maximum value. + * @return new instance of {@link Limit}. + */ + static Limit of(int max) { + return new Limited(max); + } + + final class Limited implements Limit { + + private final int max; + + Limited(int max) { + this.max = max; + } + + @Override + public int max() { + return max; + } + + @Override + public boolean equals(Object obj) { + + if (obj == null) { + return false; + } + if (!ClassUtils.isAssignable(Limit.class, obj.getClass())) { + return false; + } + Limit that = (Limit) obj; + if (this.isUnlimited() && that.isUnlimited()) { + return true; + } + return max() == that.max(); + } + + @Override + public int hashCode() { + return (int) (max ^ (max >>> 32)); + } + } + + final class Unlimited implements Limit { + + static final Limit INSTANCE = new Unlimited(); + + Unlimited() {} + + @Override + public int max() { + throw new IllegalStateException( + "Unlimited does not define 'max'. Please check 'isUnlimited' before attempting to read 'max'"); + } + + @Override + public boolean isUnlimited() { + return true; + } + } +} diff --git a/src/main/java/org/springframework/data/domain/Pageable.java b/src/main/java/org/springframework/data/domain/Pageable.java index ff830f92f..08f1dca73 100644 --- a/src/main/java/org/springframework/data/domain/Pageable.java +++ b/src/main/java/org/springframework/data/domain/Pageable.java @@ -24,6 +24,7 @@ import org.springframework.util.Assert; * * @author Oliver Gierke * @author Mark Paluch + * @author Christoph Strobl */ public interface Pageable { @@ -33,7 +34,18 @@ public interface Pageable { * @return */ static Pageable unpaged() { - return Unpaged.INSTANCE; + return unpaged(Sort.unsorted()); + } + + /** + * Returns a {@link Pageable} instance representing no pagination setup having a defined result {@link Sort order}. + * + * @param sort must not be {@literal null}, use {@link Sort#unsorted()} if needed. + * @return never {@literal null}. + * @since 3.2 + */ + static Pageable unpaged(Sort sort) { + return Unpaged.sorted(sort); } /** diff --git a/src/main/java/org/springframework/data/domain/Unpaged.java b/src/main/java/org/springframework/data/domain/Unpaged.java index b66707c73..49607a0ef 100644 --- a/src/main/java/org/springframework/data/domain/Unpaged.java +++ b/src/main/java/org/springframework/data/domain/Unpaged.java @@ -19,10 +19,21 @@ package org.springframework.data.domain; * {@link Pageable} implementation to represent the absence of pagination information. * * @author Oliver Gierke + * @author Christoph Strobl */ -enum Unpaged implements Pageable { +final class Unpaged implements Pageable { - INSTANCE; + private static final Pageable UNSORTED = new Unpaged(Sort.unsorted()); + + private final Sort sort; + + Unpaged(Sort sort) { + this.sort = sort; + } + + static Pageable sorted(Sort sort) { + return sort.isSorted() ? new Unpaged(sort) : UNSORTED; + } @Override public boolean isPaged() { @@ -46,7 +57,7 @@ enum Unpaged implements Pageable { @Override public Sort getSort() { - return Sort.unsorted(); + return sort; } @Override @@ -78,5 +89,4 @@ enum Unpaged implements Pageable { throw new UnsupportedOperationException(); } - } diff --git a/src/main/java/org/springframework/data/repository/query/Parameter.java b/src/main/java/org/springframework/data/repository/query/Parameter.java index c751db67c..385fca0b5 100644 --- a/src/main/java/org/springframework/data/repository/query/Parameter.java +++ b/src/main/java/org/springframework/data/repository/query/Parameter.java @@ -25,6 +25,7 @@ import java.util.Optional; import org.springframework.core.MethodParameter; import org.springframework.core.ResolvableType; +import org.springframework.data.domain.Limit; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.ScrollPosition; import org.springframework.data.domain.Sort; @@ -58,7 +59,7 @@ public class Parameter { static { - List> types = new ArrayList<>(Arrays.asList(ScrollPosition.class, Pageable.class, Sort.class)); + List> types = new ArrayList<>(Arrays.asList(ScrollPosition.class, Pageable.class, Sort.class, Limit.class)); // consider Kotlin Coroutines Continuation a special parameter. That parameter is synthetic and should not get // bound to any query. @@ -221,6 +222,16 @@ public class Parameter { return Sort.class.isAssignableFrom(getType()); } + /** + * Returns whether the {@link Parameter} is a {@link Limit} parameter. + * + * @return + * @since 3.2 + */ + boolean isLimit() { + return Limit.class.isAssignableFrom(getType()); + } + /** * Returns whether the given {@link MethodParameter} is a dynamic projection parameter, which means it carries a * dynamic type parameter which is identical to the type parameter of the actually returned type. diff --git a/src/main/java/org/springframework/data/repository/query/ParameterAccessor.java b/src/main/java/org/springframework/data/repository/query/ParameterAccessor.java index 369c781a5..c009dda68 100644 --- a/src/main/java/org/springframework/data/repository/query/ParameterAccessor.java +++ b/src/main/java/org/springframework/data/repository/query/ParameterAccessor.java @@ -17,6 +17,7 @@ package org.springframework.data.repository.query; import java.util.Iterator; +import org.springframework.data.domain.Limit; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.ScrollPosition; import org.springframework.data.domain.Sort; @@ -54,6 +55,22 @@ public interface ParameterAccessor extends Iterable { */ Sort getSort(); + /** + * Returns the {@link Limit} instance to be used for query creation. If no {@link java.lang.reflect.Parameter} + * assignable to {@link Limit} can be found {@link Limit} will be created out of {@link Pageable#getPageSize()} if + * present. + * + * @return {@link Limit#unlimited()} by default. + * @since 3.2 + */ + default Limit getLimit() { + + if (getPageable().isUnpaged()) { + return Limit.unlimited(); + } + return Limit.of(getPageable().getPageSize()); + } + /** * Returns the dynamic projection type to be used when executing the query or {@literal null} if none is defined. * diff --git a/src/main/java/org/springframework/data/repository/query/Parameters.java b/src/main/java/org/springframework/data/repository/query/Parameters.java index 523f12799..f98e2a217 100644 --- a/src/main/java/org/springframework/data/repository/query/Parameters.java +++ b/src/main/java/org/springframework/data/repository/query/Parameters.java @@ -27,6 +27,7 @@ import java.util.function.Function; import org.springframework.core.DefaultParameterNameDiscoverer; import org.springframework.core.MethodParameter; import org.springframework.core.ParameterNameDiscoverer; +import org.springframework.data.domain.Limit; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.ScrollPosition; import org.springframework.data.domain.Sort; @@ -43,7 +44,7 @@ import org.springframework.util.Assert; */ public abstract class Parameters, T extends Parameter> implements Streamable { - public static final List> TYPES = Arrays.asList(ScrollPosition.class, Pageable.class, Sort.class); + public static final List> TYPES = Arrays.asList(ScrollPosition.class, Pageable.class, Sort.class, Limit.class); private static final String PARAM_ON_SPECIAL = format("You must not use @%s on a parameter typed %s or %s", Param.class.getSimpleName(), Pageable.class.getSimpleName(), Sort.class.getSimpleName()); @@ -56,6 +57,7 @@ public abstract class Parameters, T extends Parameter private final int scrollPositionIndex; private final int pageableIndex; private final int sortIndex; + private final int limitIndex; private final List parameters; private final Lazy bindable; @@ -96,6 +98,7 @@ public abstract class Parameters, T extends Parameter int scrollPositionIndex = -1; int pageableIndex = -1; int sortIndex = -1; + int limitIndex = -1; for (int i = 0; i < parameterCount; i++) { @@ -126,12 +129,17 @@ public abstract class Parameters, T extends Parameter sortIndex = i; } + if (Limit.class.isAssignableFrom(parameter.getType())) { + limitIndex = i; + } + parameters.add(parameter); } this.scrollPositionIndex = scrollPositionIndex; this.pageableIndex = pageableIndex; this.sortIndex = sortIndex; + this.limitIndex = limitIndex; this.bindable = Lazy.of(this::getBindable); assertEitherAllParamAnnotatedOrNone(); @@ -149,6 +157,7 @@ public abstract class Parameters, T extends Parameter int scrollPositionIndexTemp = -1; int pageableIndexTemp = -1; int sortIndexTemp = -1; + int limitIndexTmp = -1; int dynamicProjectionTemp = -1; for (int i = 0; i < originals.size(); i++) { @@ -159,12 +168,14 @@ public abstract class Parameters, T extends Parameter scrollPositionIndexTemp = original.isScrollPosition() ? i : -1; pageableIndexTemp = original.isPageable() ? i : -1; sortIndexTemp = original.isSort() ? i : -1; + limitIndexTmp = original.isLimit() ? i : -1; dynamicProjectionTemp = original.isDynamicProjectionParameter() ? i : -1; } this.scrollPositionIndex = scrollPositionIndexTemp; this.pageableIndex = pageableIndexTemp; this.sortIndex = sortIndexTemp; + this.limitIndex = limitIndexTmp; this.dynamicProjectionIndex = dynamicProjectionTemp; this.bindable = Lazy.of(() -> (S) this); } @@ -255,6 +266,27 @@ public abstract class Parameters, T extends Parameter return sortIndex != -1; } + /** + * Returns whether the method the {@link Parameters} was created for contains a {@link Limit} argument. + * + * @return + * @since 3.2 + */ + public boolean hasLimitParameter() { + return getLimitIndex() != -1; + } + + /** + * Returns the index of the {@link Limit} {@link Method} parameter if available. Will return {@literal -1} if there is + * no {@link Limit} argument in the {@link Method}'s parameter list. + * + * @return + * @since 3.2 + */ + public int getLimitIndex() { + return limitIndex; + } + /** * Returns the index of the parameter that represents the dynamic projection type. Will return {@literal -1} if no * such parameter exists. diff --git a/src/main/java/org/springframework/data/repository/query/ParametersParameterAccessor.java b/src/main/java/org/springframework/data/repository/query/ParametersParameterAccessor.java index 045ab16b1..15f05b099 100644 --- a/src/main/java/org/springframework/data/repository/query/ParametersParameterAccessor.java +++ b/src/main/java/org/springframework/data/repository/query/ParametersParameterAccessor.java @@ -17,6 +17,8 @@ package org.springframework.data.repository.query; import java.util.Iterator; +import org.springframework.data.domain.Limit; +import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.ScrollPosition; import org.springframework.data.domain.Sort; @@ -111,13 +113,20 @@ public class ParametersParameterAccessor implements ParameterAccessor { @Override public Pageable getPageable() { - if (!parameters.hasPageableParameter()) { - return Pageable.unpaged(); + if (parameters.hasPageableParameter()) { + + Pageable pageable = (Pageable) values[parameters.getPageableIndex()]; + return pageable == null ? Pageable.unpaged() : pageable; } - Pageable pageable = (Pageable) values[parameters.getPageableIndex()]; + Limit limit = parameters.hasLimitParameter() ? getLimit() : Limit.unlimited(); + Sort sort = parameters.hasSortParameter() ? getSort() : Sort.unsorted(); - return pageable == null ? Pageable.unpaged() : pageable; + if (limit.isUnlimited()) { + return Pageable.unpaged(sort); + } + + return PageRequest.of(0, limit.max(), sort); } @Override @@ -136,6 +145,24 @@ public class ParametersParameterAccessor implements ParameterAccessor { return Sort.unsorted(); } + @Override + public Limit getLimit() { + + if (parameters.hasLimitParameter()) { + + Limit limit = (Limit) values[parameters.getLimitIndex()]; + return limit == null ? Limit.unlimited() : limit; + } + + if (parameters.hasPageableParameter()) { + + Pageable pageable = (Pageable) values[parameters.getPageableIndex()]; + return pageable.isPaged() ? Limit.of(pageable.getPageSize()) : Limit.unlimited(); + } + + return Limit.unlimited(); + } + /** * Returns the dynamic projection type if available, {@literal null} otherwise. * 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 05fc4439f..822be10a7 100644 --- a/src/main/java/org/springframework/data/repository/query/QueryMethod.java +++ b/src/main/java/org/springframework/data/repository/query/QueryMethod.java @@ -20,8 +20,10 @@ import static org.springframework.data.repository.util.ClassUtils.*; import java.lang.reflect.Method; import java.util.Collections; import java.util.Set; +import java.util.function.Predicate; import java.util.stream.Stream; +import org.springframework.data.domain.Limit; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.ScrollPosition; @@ -85,36 +87,6 @@ public class QueryMethod { this.metadata = metadata; this.parameters = createParameters(method); - if (hasParameterOfType(method, Pageable.class)) { - - if (!isStreamQuery()) { - assertReturnTypeAssignable(method, QueryExecutionConverters.getAllowedPageableTypes()); - } - - if (hasParameterOfType(method, Sort.class)) { - throw new IllegalStateException(String.format("Method must not have Pageable *and* Sort parameters. " - + "Use sorting capabilities on Pageable instead; Offending method: %s", method)); - } - } - - if (hasParameterOfType(method, ScrollPosition.class)) { - assertReturnTypeAssignable(method, Collections.singleton(Window.class)); - } - - Assert.notNull(this.parameters, - () -> String.format("Parameters extracted from method '%s' must not be null", method.getName())); - - if (isPageQuery()) { - Assert.isTrue(this.parameters.hasPageableParameter(), - String.format("Paging query needs to have a Pageable parameter; Offending method: %s", method)); - } - - if (isScrollQuery()) { - - Assert.isTrue(this.parameters.hasScrollPositionParameter() || this.parameters.hasPageableParameter(), - String.format("Scroll query needs to have a ScrollPosition parameter; Offending method: %s", method)); - } - this.domainClass = Lazy.of(() -> { Class repositoryDomainClass = metadata.getDomainType(); @@ -127,6 +99,8 @@ public class QueryMethod { this.resultProcessor = new ResultProcessor(this, factory); this.isCollectionQuery = Lazy.of(this::calculateIsCollectionQuery); + + validate(); } /** @@ -291,6 +265,41 @@ public class QueryMethod { return method.toString(); } + public void validate() { + + QueryMethodValidator.validate(method); + + if (hasParameterOfType(method, Pageable.class)) { + + if (!isStreamQuery()) { + assertReturnTypeAssignable(method, QueryExecutionConverters.getAllowedPageableTypes()); + } + + if (hasParameterOfType(method, Sort.class)) { + throw new IllegalStateException(String.format("Method must not have Pageable *and* Sort parameters. " + + "Use sorting capabilities on Pageable instead; Offending method: %s", method)); + } + } + + if (hasParameterOfType(method, ScrollPosition.class)) { + assertReturnTypeAssignable(method, Collections.singleton(Window.class)); + } + + Assert.notNull(this.parameters, + () -> String.format("Parameters extracted from method '%s' must not be null", method.getName())); + + if (isPageQuery()) { + Assert.isTrue(this.parameters.hasPageableParameter(), + String.format("Paging query needs to have a Pageable parameter; Offending method: %s", method)); + } + + if (isScrollQuery()) { + + Assert.isTrue(this.parameters.hasScrollPositionParameter() || this.parameters.hasPageableParameter(), + String.format("Scroll query needs to have a ScrollPosition parameter; Offending method: %s", method)); + } + } + private boolean calculateIsCollectionQuery() { if (isPageQuery() || isSliceQuery() || isScrollQuery()) { @@ -363,4 +372,30 @@ public class QueryMethod { throw new IllegalStateException("Method has to have one of the following return types " + types); } + + static class QueryMethodValidator { + + static void validate(Method method) { + + if (!pageableCannotHaveSortOrLimit.test(method)) { + + throw new IllegalStateException( + "Method method using Pageable parameter must not define Limit nor Sort. Offending method: %s" + .formatted(method)); + } + } + + static Predicate pageableCannotHaveSortOrLimit = (method) -> { + + if (!hasParameterAssignableToType(method, Pageable.class)) { + return true; + } + + if (hasParameterAssignableToType(method, Sort.class) || hasParameterAssignableToType(method, Limit.class)) { + return false; + } + + return true; + }; + } } diff --git a/src/main/java/org/springframework/data/repository/query/parser/PartTree.java b/src/main/java/org/springframework/data/repository/query/parser/PartTree.java index ad4a66904..647b8f109 100644 --- a/src/main/java/org/springframework/data/repository/query/parser/PartTree.java +++ b/src/main/java/org/springframework/data/repository/query/parser/PartTree.java @@ -23,6 +23,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; +import org.springframework.data.domain.Limit; import org.springframework.data.domain.Sort; import org.springframework.data.repository.query.parser.Part.Type; import org.springframework.data.repository.query.parser.PartTree.OrPart; @@ -169,6 +170,16 @@ public class PartTree implements Streamable { return subject.getMaxResults().orElse(null); } + /** + * Return the number of maximal results to return or {@link Limit#unlimited()} if not restricted. + * + * @return {@literal null} if not restricted. + * @since 3.2 + */ + public Limit getResultLimit() { + return subject.getMaxResults().map(Limit::of).orElse(Limit.unlimited()); + } + /** * Returns an {@link Iterable} of all parts contained in the {@link PartTree}. * diff --git a/src/main/java/org/springframework/data/repository/util/ClassUtils.java b/src/main/java/org/springframework/data/repository/util/ClassUtils.java index c26a2fbee..faa46aedd 100644 --- a/src/main/java/org/springframework/data/repository/util/ClassUtils.java +++ b/src/main/java/org/springframework/data/repository/util/ClassUtils.java @@ -19,6 +19,8 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.Arrays; import java.util.Collection; +import java.util.List; +import java.util.Set; import java.util.function.Consumer; import org.springframework.data.repository.Repository; @@ -176,6 +178,17 @@ public abstract class ClassUtils { return Arrays.asList(method.getParameterTypes()).contains(type); } + /** + * Returns whether the given {@link Method} has a parameter that is assignable to the given type. + * + * @param method + * @param type + * @return + */ + public static boolean hasParameterAssignableToType(Method method, Class type) { + return List.of(method.getParameterTypes()).stream().anyMatch(type::isAssignableFrom); + } + /** * Helper method to extract the original exception that can possibly occur during a reflection call. * diff --git a/src/test/java/org/springframework/data/domain/LimitUnitTests.java b/src/test/java/org/springframework/data/domain/LimitUnitTests.java new file mode 100644 index 000000000..5d4164cfa --- /dev/null +++ b/src/test/java/org/springframework/data/domain/LimitUnitTests.java @@ -0,0 +1,56 @@ +/* + * Copyright 2023 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.domain; + +import static org.assertj.core.api.Assertions.*; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +/** + * @author Christoph Strobl + * @soundtrack Rise Against - Tragedy + Time + */ +class LimitUnitTests { + + @Test // GH-2827 + void unlimitedReusesInstance() { + assertThat(Limit.unlimited()).isSameAs(Limit.unlimited()); + } + + @Test // GH-2827 + void unlimitedReturnsTrueOnIsLimited() { + assertThat(Limit.unlimited().isUnlimited()).isTrue(); + } + + @ParameterizedTest // GH-2827 + @ValueSource(ints = { Integer.MIN_VALUE, -100, 0, 100, Integer.MAX_VALUE }) + void limitOfCapturesMaxValue(int value) { + assertThat(Limit.of(value)).extracting(Limit::max).isEqualTo(value); + } + + @ParameterizedTest // GH-2827 + @ValueSource(ints = { Integer.MIN_VALUE, -100, 0, 100, Integer.MAX_VALUE }) + void isUnlimitedReturnsFalseIfLimited(int value) { + assertThat(Limit.of(value).isUnlimited()).isFalse(); + } + + @Test // GH-2827 + void unlimitedErrorsOnMax() { + assertThatExceptionOfType(IllegalStateException.class).isThrownBy(() -> Limit.unlimited().max()); + } +} diff --git a/src/test/java/org/springframework/data/repository/query/ParametersParameterAccessorUnitTests.java b/src/test/java/org/springframework/data/repository/query/ParametersParameterAccessorUnitTests.java index ec82d1397..223aa0f35 100755 --- a/src/test/java/org/springframework/data/repository/query/ParametersParameterAccessorUnitTests.java +++ b/src/test/java/org/springframework/data/repository/query/ParametersParameterAccessorUnitTests.java @@ -19,9 +19,12 @@ import static org.assertj.core.api.Assertions.*; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.springframework.data.domain.Limit; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.ScrollPosition; +import org.springframework.data.domain.Sort; +import org.springframework.data.domain.Sort.Order; /** * Unit tests for {@link ParametersParameterAccessor}. @@ -101,6 +104,94 @@ class ParametersParameterAccessorUnitTests { assertThat(accessor.getBindableValue(0)).isEqualTo("Foo"); } + @Test // GH-2728 + void handlesLimitAsAParameterType() throws NoSuchMethodException { + + var method = Sample.class.getMethod("method", Limit.class, String.class); + var accessor = new ParametersParameterAccessor(new DefaultParameters(method), + new Object[] { Limit.of(100), "spring" }); + + assertThat(accessor).hasSize(1); + assertThat(accessor.getBindableValue(0)).isEqualTo("spring"); + } + + @Test // GH-2728 + void returnsLimitIfAvailable() throws NoSuchMethodException { + + var method = Sample.class.getMethod("method", Limit.class, String.class); + var accessor = new ParametersParameterAccessor(new DefaultParameters(method), + new Object[] { Limit.of(100), "spring" }); + + assertThat(accessor.getLimit()).extracting(Limit::max).isEqualTo(100); + } + + @Test // GH-2728 + void readsLimitFromPageableIfAvailable() throws NoSuchMethodException { + + var method = Sample.class.getMethod("method", Pageable.class, String.class); + var accessor = new ParametersParameterAccessor(new DefaultParameters(method), + new Object[] { Pageable.ofSize(100), "spring" }); + + assertThat(accessor.getLimit()).extracting(Limit::max).isEqualTo(100); + } + + @Test // GH-2728 + void returnsUnlimitedIfNoLimitingAvailable() throws NoSuchMethodException { + + var method = Sample.class.getMethod("method", Sort.class, String.class); + var accessor = new ParametersParameterAccessor(new DefaultParameters(method), + new Object[] { Pageable.ofSize(100), "spring" }); + + assertThat(accessor.getLimit().isUnlimited()).isTrue(); + } + + @Test // GH-2728 + void appliesLimitToPageableIfAvailable() throws NoSuchMethodException { + + var method = Sample.class.getMethod("method", Limit.class, String.class); + var accessor = new ParametersParameterAccessor(new DefaultParameters(method), + new Object[] { Limit.of(100), "spring" }); + + Pageable pageable = accessor.getPageable(); + assertThat(pageable).extracting(Pageable::getPageSize).isEqualTo(100); + assertThat(pageable.getSort().isUnsorted()).isTrue(); + } + + @Test // GH-2728 + void appliesLimitToPageableIfRequested() throws NoSuchMethodException { + + var method = Sample.class.getMethod("method", Limit.class, String.class); + var accessor = new ParametersParameterAccessor(new DefaultParameters(method), + new Object[] { Limit.of(100), "spring" }); + + assertThat(accessor).hasSize(1); + assertThat(accessor.getBindableValue(0)).isEqualTo("spring"); + } + + @Test // GH-2728 + void appliesSortToPageableIfAvailable() throws NoSuchMethodException { + + var method = Sample.class.getMethod("method", Sort.class, String.class); + var accessor = new ParametersParameterAccessor(new DefaultParameters(method), + new Object[] { Sort.by("one", "two"), "spring" }); + + Pageable pageable = accessor.getPageable(); + assertThat(pageable.isPaged()).isFalse(); + assertThat(pageable.getSort()).containsExactly(Order.by("one"), Order.by("two")); + } + + @Test // GH-2728 + void appliesSortAndLimitToPageableIfAvailable() throws NoSuchMethodException { + + var method = Sample.class.getMethod("method", Sort.class, Limit.class, String.class); + var accessor = new ParametersParameterAccessor(new DefaultParameters(method), + new Object[] { Sort.by("one", "two"), Limit.of(42), "spring" }); + + Pageable pageable = accessor.getPageable(); + assertThat(pageable).extracting(Pageable::getPageSize).isEqualTo(42); + assertThat(pageable.getSort()).containsExactly(Order.by("one"), Order.by("two")); + } + interface Sample { void method(String string, int integer); @@ -110,5 +201,11 @@ class ParametersParameterAccessorUnitTests { void method(ScrollPosition scrollPosition, String string); void methodWithPageRequest(PageRequest pageRequest, String string); + + void method(Limit limit, String string); + + void method(Sort sort, Limit limit, String string); + + void method(Sort sort, String string); } } diff --git a/src/test/java/org/springframework/data/repository/query/ParametersUnitTests.java b/src/test/java/org/springframework/data/repository/query/ParametersUnitTests.java index d73466484..0c62d9196 100755 --- a/src/test/java/org/springframework/data/repository/query/ParametersUnitTests.java +++ b/src/test/java/org/springframework/data/repository/query/ParametersUnitTests.java @@ -20,11 +20,13 @@ import static org.assertj.core.api.Assertions.*; import io.reactivex.rxjava3.core.Single; import java.lang.reflect.Method; +import java.util.List; import java.util.Optional; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.reactivestreams.Publisher; +import org.springframework.data.domain.Limit; import org.springframework.data.domain.OffsetScrollPosition; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; @@ -191,6 +193,15 @@ class ParametersUnitTests { assertThat(parameters.hasScrollPositionParameter()).isTrue(); } + @Test // GH-2827 + void acceptsLimitParameter() throws Exception { + + var parameters = getParametersFor("withResultLimit", String.class, Limit.class); + + assertThat(parameters.hasLimitParameter()).isTrue(); + assertThat(parameters.getLimitIndex()).isOne(); + } + private Parameters getParametersFor(String methodName, Class... parameterTypes) throws SecurityException, NoSuchMethodException { @@ -232,6 +243,8 @@ class ParametersUnitTests { Page customPageable(SomePageable pageable); Window customScrollPosition(OffsetScrollPosition request); + + List withResultLimit(String criteria, Limit limit); } interface SomePageable extends Pageable {} 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 ec1d0ed34..53fe91a5d 100755 --- a/src/test/java/org/springframework/data/repository/query/QueryMethodUnitTests.java +++ b/src/test/java/org/springframework/data/repository/query/QueryMethodUnitTests.java @@ -34,10 +34,13 @@ import org.eclipse.collections.api.list.ImmutableList; import org.junit.jupiter.api.DynamicTest; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestFactory; +import org.springframework.data.domain.Limit; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.ScrollPosition; import org.springframework.data.domain.Slice; +import org.springframework.data.domain.Sort; +import org.springframework.data.domain.Window; import org.springframework.data.domain.Window; import org.springframework.data.projection.ProjectionFactory; import org.springframework.data.projection.SpelAwareProxyProjectionFactory; @@ -307,6 +310,62 @@ class QueryMethodUnitTests { assertThat(queryMethod.isCollectionQuery()).isTrue(); } + @Test // GH-2827 + void rejectsPageAndSort() throws NoSuchMethodException { + + var method = SampleRepository.class.getMethod("pageableAndSort", Pageable.class, Sort.class); + + assertThatExceptionOfType(IllegalStateException.class).isThrownBy(() -> new QueryMethod(method, metadata, factory)); + } + + @Test // GH-2827 + void rejectsPageAndLimit() throws NoSuchMethodException { + + var method = SampleRepository.class.getMethod("pageableAndLimit", Pageable.class, Limit.class); + + assertThatExceptionOfType(IllegalStateException.class).isThrownBy(() -> new QueryMethod(method, metadata, factory)); + } + + @Test // GH-2827 + void allowsSortAndLimit() throws NoSuchMethodException { + + var method = SampleRepository.class.getMethod("sortAndLimit", Sort.class, Limit.class); + + new QueryMethod(method, metadata, factory); + } + + @Test // GH-2827 + void allowsScrollPositionAndLimit() throws NoSuchMethodException { + + var method = SampleRepository.class.getMethod("scrollPositionAndLimit", ScrollPosition.class, Limit.class); + + new QueryMethod(method, metadata, factory); + } + + @Test // GH-2827 + void allowFindTopAndLimit() throws NoSuchMethodException { + + var method = SampleRepository.class.getMethod("findTop5By", Limit.class); + + new QueryMethod(method, metadata, factory); + } + + @Test // GH-2827 + void allowsFindTopAndPageable() throws NoSuchMethodException { + + var method = SampleRepository.class.getMethod("findTop5By", Pageable.class); + + new QueryMethod(method, metadata, factory); + } + + @Test // GH-2827 + void acceptsScrollAndSortMaybe() throws NoSuchMethodException { + + var method = SampleRepository.class.getMethod("scrollPositionAndSort", ScrollPosition.class, Sort.class); + + new QueryMethod(method, metadata, factory); + } + @TestFactory // GH-2869 Stream doesNotConsiderQueryMethodReturningAggregateImplementingStreamableACollectionQuery() throws Exception { @@ -387,6 +446,20 @@ class QueryMethodUnitTests { Page cursorWindowMethodWithInvalidReturnType(ScrollPosition cursorRequest); Window cursorWindowWithoutScrollPosition(); + + List pageableAndSort(Pageable pageable, Sort sort); + + List pageableAndLimit(Pageable pageable, Limit limit); + + Window scrollPositionAndLimit(ScrollPosition pageable, Limit limit); + + Window scrollPositionAndSort(ScrollPosition pageable, Sort sort); + + List sortAndLimit(Sort sort, Limit limit); + + List findTop5By(Limit limit); + + List findTop5By(Pageable page); } class User { diff --git a/src/test/java/org/springframework/data/repository/query/parser/PartTreeUnitTests.java b/src/test/java/org/springframework/data/repository/query/parser/PartTreeUnitTests.java index f828ec81c..00a3acd10 100755 --- a/src/test/java/org/springframework/data/repository/query/parser/PartTreeUnitTests.java +++ b/src/test/java/org/springframework/data/repository/query/parser/PartTreeUnitTests.java @@ -27,6 +27,8 @@ import java.util.Date; import java.util.List; import org.junit.jupiter.api.Test; + +import org.springframework.data.domain.Limit; import org.springframework.data.domain.Sort; import org.springframework.data.mapping.PropertyPath; import org.springframework.data.mapping.PropertyReferenceException; @@ -666,6 +668,7 @@ class PartTreeUnitTests { assertThat(tree.isLimiting()).isEqualTo(limiting); assertThat(tree.getMaxResults()).isEqualTo(maxResults); + assertThat(tree.getResultLimit()).isEqualTo(maxResults != null ? Limit.of(maxResults) : Limit.unlimited()); assertThat(tree.isDistinct()).isEqualTo(distinct); }