Polishing.

Refine documentation. Introduce isLimited method to mirror isPresent/isSorted semantics. Introduce Pageable.toLimit() method to deduplicate code.

See #2827
Original pull request: #2836

Author: mp911de

src/main/asciidoc/repositories-paging-sorting.adoc

@@ -64,23 +64,23 @@ 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.
+Some special parameters described above are mutually exclusive.
 Please consider the following list of invalid parameter combinations.
 
 |===
 | Parameters | Example | Reason
 
-| `Pageable` & `Sort`
+| `Pageable` and `Sort`
 | `findBy...(Pageable page, Sort sort)`
 | `Pageable` already defines `Sort`
 
-| `Pageable` & `Limit`
+| `Pageable` and `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.
+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 number.
 ====
 
 [[repositories.scrolling.guidance]]

src/main/asciidoc/repository-query-return-types-reference.adoc

@@ -25,7 +25,7 @@ Some store modules may define their own result wrapper types.
 |`Optional<T>`|A Java 8 or Guava `Optional`. Expects the query method to return one result at most. If no result is found, `Optional.empty()` or `Optional.absent()` is returned. More than one result triggers an `IncorrectResultSizeDataAccessException`.
 |`Option<T>`|Either a Scala or Vavr `Option` type. Semantically the same behavior as Java 8's `Optional`, described earlier.
 |`Stream<T>`|A Java 8 `Stream`.
-|`Streamable<T>`|A convenience extension of `Iterable` that directy exposes methods to stream, map and filter results, concatenate them etc.
+|`Streamable<T>`|A convenience extension of `Iterable` that directly exposes methods to stream, map and filter results, concatenate them etc.
 |Types that implement `Streamable` and take a `Streamable` constructor or factory method argument|Types that expose a constructor or `….of(…)`/`….valueOf(…)` factory method taking a `Streamable` as argument. See <<repositories.collections-and-iterables.streamable-wrapper>> for details.
 |Vavr `Seq`, `List`, `Map`, `Set`|Vavr collection types. See <<repositories.collections-and-iterables.vavr>> for details.
 |`Future<T>`|A `Future`. Expects a method to be annotated with `@Async` and requires Spring's asynchronous method execution capability to be enabled.

src/main/java/org/springframework/data/domain/Limit.java

@@ -28,25 +28,12 @@
  * over using {@literal null} or {@link java.util.Optional#empty()} to indicate the absence of an actual {@link Limit}.
  * </p>
  * {@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.
- * 
+ * value may be valid within a defined context.
+ *
  * @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);
-	}
+public sealed interface Limit permits Limited,Unlimited {
 
 	/**
 	 * @return a {@link Limit} instance that does not define {@link #max()} and answers {@link #isUnlimited()} with
@@ -66,6 +53,23 @@ static Limit of(int max) {
 		return new Limited(max);
 	}
 
+	/**
+	 * @return the max number of potential results.
+	 */
+	int max();
+
+	/**
+	 * @return {@literal true} if limiting (maximum value) should be applied.
+	 */
+	boolean isLimited();
+
+	/**
+	 * @return {@literal true} if no limiting (maximum value) should be applied.
+	 */
+	default boolean isUnlimited() {
+		return !isLimited();
+	}
+
 	final class Limited implements Limit {
 
 		private final int max;
@@ -79,6 +83,11 @@ public int max() {
 			return max;
 		}
 
+		@Override
+		public boolean isLimited() {
+			return true;
+		}
+
 		@Override
 		public boolean equals(Object obj) {
 
@@ -97,7 +106,7 @@ public boolean equals(Object obj) {
 
 		@Override
 		public int hashCode() {
-			return (int) (max ^ (max >>> 32));
+			return max ^ (max >>> 32);
 		}
 	}
 
@@ -110,12 +119,13 @@ final class Unlimited implements Limit {
 		@Override
 		public int max() {
 			throw new IllegalStateException(
-					"Unlimited does not define 'max'. Please check 'isUnlimited' before attempting to read 'max'");
+					"Unlimited does not define 'max'. Please check 'isLimited' before attempting to read 'max'");
 		}
 
 		@Override
-		public boolean isUnlimited() {
-			return true;
+		public boolean isLimited() {
+			return false;
 		}
+
 	}
 }

src/main/java/org/springframework/data/domain/Pageable.java

@@ -173,6 +173,22 @@ default Optional<Pageable> toOptional() {
 		return isUnpaged() ? Optional.empty() : Optional.of(this);
 	}
 
+	/**
+	 * Returns an {@link Limit} from this pageable if the page request {@link #isPaged() is paged} or
+	 * {@link Limit#unlimited()} otherwise.
+	 *
+	 * @return
+	 * @since 3.2
+	 */
+	default Limit toLimit() {
+
+		if (isUnpaged()) {
+			return Limit.unlimited();
+		}
+
+		return Limit.of(getPageSize());
+	}
+
 	/**
 	 * Returns an {@link OffsetScrollPosition} from this pageable if the page request {@link #isPaged() is paged}.
 	 *

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

@@ -60,15 +60,11 @@ public interface ParameterAccessor extends Iterable<Object> {
 	 * 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.
+	 * @return
 	 * @since 3.2
 	 */
 	default Limit getLimit() {
-
-		if (getPageable().isUnpaged()) {
-			return Limit.unlimited();
-		}
-		return Limit.of(getPageable().getPageSize());
+		return getPageable().toLimit();
 	}
 
 	/**

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

@@ -157,7 +157,7 @@ public Limit getLimit() {
 		if (parameters.hasPageableParameter()) {
 
 			Pageable pageable = (Pageable) values[parameters.getPageableIndex()];
-			return pageable.isPaged() ? Limit.of(pageable.getPageSize()) : Limit.unlimited();
+			return pageable.toLimit();
 		}
 
 		return Limit.unlimited();

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

@@ -103,6 +103,68 @@ public QueryMethod(Method method, RepositoryMetadata metadata, ProjectionFactory
 		validate();
 	}
 
+	private 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()) {
+			return false;
+		}
+
+		TypeInformation<?> returnTypeInformation = metadata.getReturnType(method);
+
+		// Check against simple wrapper types first
+		if (metadata.getDomainTypeInformation()
+				.isAssignableFrom(NullableWrapperConverters.unwrapActualType(returnTypeInformation))) {
+			return false;
+		}
+
+		Class<?> returnType = returnTypeInformation.getType();
+
+		if (QueryExecutionConverters.supports(returnType) && !QueryExecutionConverters.isSingleValue(returnType)) {
+			return true;
+		}
+
+		if (QueryExecutionConverters.supports(unwrappedReturnType)) {
+			return !QueryExecutionConverters.isSingleValue(unwrappedReturnType);
+		}
+
+		return TypeInformation.of(unwrappedReturnType).isCollectionLike();
+	}
+
 	/**
 	 * Creates a {@link Parameters} instance.
 	 *
@@ -265,68 +327,6 @@ public String toString() {
 		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()) {
-			return false;
-		}
-
-		TypeInformation<?> returnTypeInformation = metadata.getReturnType(method);
-
-		// Check against simple wrapper types first
-		if (metadata.getDomainTypeInformation()
-				.isAssignableFrom(NullableWrapperConverters.unwrapActualType(returnTypeInformation))) {
-			return false;
-		}
-
-		Class<?> returnType = returnTypeInformation.getType();
-
-		if (QueryExecutionConverters.supports(returnType) && !QueryExecutionConverters.isSingleValue(returnType)) {
-			return true;
-		}
-
-		if (QueryExecutionConverters.supports(unwrappedReturnType)) {
-			return !QueryExecutionConverters.isSingleValue(unwrappedReturnType);
-		}
-
-		return TypeInformation.of(unwrappedReturnType).isCollectionLike();
-	}
-
 	private static Class<? extends Object> potentiallyUnwrapReturnTypeFor(RepositoryMetadata metadata, Method method) {
 
 		TypeInformation<?> returnType = metadata.getReturnType(method);

src/test/java/org/springframework/data/domain/LimitUnitTests.java

@@ -22,6 +22,8 @@
 import org.junit.jupiter.params.provider.ValueSource;
 
 /**
+ * Unit tests for {@link Limit}.
+ *
  * @author Christoph Strobl
  * @soundtrack Rise Against - Tragedy + Time
  */