Polishing.

Revise nullability requirements around non-nullable specifications.

Original Pull Request: #3578

Author: mp911de

Diff for: spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/JpaSpecificationExecutor.java

@@ -25,6 +25,9 @@
 import java.util.Optional;
 import java.util.function.Function;
 
+import org.springframework.dao.InvalidDataAccessApiUsageException;
+import org.jspecify.annotations.Nullable;
+
 import org.springframework.dao.InvalidDataAccessApiUsageException;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.Pageable;

Diff for: spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryBySpecification.java

@@ -26,6 +26,8 @@
 import java.util.function.Function;
 import java.util.stream.Stream;
 
+import org.jspecify.annotations.Nullable;
+
 import org.springframework.dao.IncorrectResultSizeDataAccessException;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.PageImpl;
@@ -174,18 +176,18 @@ public Window<R> scroll(ScrollPosition scrollPosition) {
 
 	@Override
 	public Slice<R> slice(Pageable pageable) {
-		return pageable.isUnpaged() ? new PageImpl<>(all(pageable.getSortOr(this.sort))) : readSlice(pageable);
+		return pageable.isUnpaged() ? new PageImpl<>(all(pageable.getSort())) : readSlice(pageable);
 	}
 
 	@Override
 	public Page<R> page(Pageable pageable) {
-		return pageable.isUnpaged() ? new PageImpl<>(all(pageable.getSortOr(this.sort))) : readPage(pageable, spec);
+		return pageable.isUnpaged() ? new PageImpl<>(all(pageable.getSort())) : readPage(pageable, spec);
 	}
 
 	@Override
 	@SuppressWarnings({ "rawtypes", "unchecked" })
 	public Page<R> page(Pageable pageable, Specification<?> countSpec) {
-		return pageable.isUnpaged() ? new PageImpl<>(all(pageable.getSortOr(this.sort)))
+		return pageable.isUnpaged() ? new PageImpl<>(all(pageable.getSort()))
 				: readPage(pageable, (Specification) countSpec);
 	}
 

Diff for: spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java

@@ -513,6 +513,7 @@ public <S extends T, R> R findBy(Specification<T> spec,
 		return doFindBy(spec, getDomainClass(), queryFunction);
 	}
 
+	@SuppressWarnings("unchecked")
 	private <S extends T, R> R doFindBy(Specification<T> spec, Class<T> domainClass,
 			Function<? super SpecificationFluentQuery<S>, R> queryFunction) {
 
@@ -610,6 +611,7 @@ public <S extends T> Page<S> findAll(Example<S> example, Pageable pageable) {
 	}
 
 	@Override
+	@SuppressWarnings("unchecked")
 	public <S extends T, R> R findBy(Example<S> example, Function<FetchableFluentQuery<S>, R> queryFunction) {
 
 		Assert.notNull(example, EXAMPLE_MUST_NOT_BE_NULL);
@@ -632,7 +634,7 @@ public long count() {
 	}
 
 	@Override
-	public long count(@Nullable Specification<T> spec) {
+	public long count(Specification<T> spec) {
 		return executeCountQuery(getCountQuery(spec, getDomainClass()));
 	}
 
@@ -701,7 +703,7 @@ public void flush() {
 	 * @deprecated use {@link #readPage(TypedQuery, Class, Pageable, Specification)} instead
 	 */
 	@Deprecated
-	protected Page<T> readPage(TypedQuery<T> query, Pageable pageable, @Nullable Specification<T> spec) {
+	protected Page<T> readPage(TypedQuery<T> query, Pageable pageable, Specification<T> spec) {
 		return readPage(query, getDomainClass(), pageable, spec);
 	}
 
@@ -711,11 +713,13 @@ protected Page<T> readPage(TypedQuery<T> query, Pageable pageable, @Nullable Spe
 	 *
 	 * @param query must not be {@literal null}.
 	 * @param domainClass must not be {@literal null}.
-	 * @param spec can be {@literal null}.
+	 * @param spec must not be {@literal null}.
 	 * @param pageable can be {@literal null}.
 	 */
 	protected <S extends T> Page<S> readPage(TypedQuery<S> query, Class<S> domainClass, Pageable pageable,
-			@Nullable Specification<S> spec) {
+			Specification<S> spec) {
+
+		Assert.notNull(spec, "Specification must not be null");
 
 		if (pageable.isPaged()) {
 			query.setFirstResult(PageableUtils.getOffsetAsInteger(pageable));
@@ -729,21 +733,21 @@ protected <S extends T> Page<S> readPage(TypedQuery<S> query, Class<S> domainCla
 	/**
 	 * Creates a new {@link TypedQuery} from the given {@link Specification}.
 	 *
-	 * @param spec can be {@literal null}.
+	 * @param spec must not be {@literal null}.
 	 * @param pageable must not be {@literal null}.
 	 */
-	protected TypedQuery<T> getQuery(@Nullable Specification<T> spec, Pageable pageable) {
+	protected TypedQuery<T> getQuery(Specification<T> spec, Pageable pageable) {
 		return getQuery(spec, getDomainClass(), pageable.getSort());
 	}
 
 	/**
 	 * Creates a new {@link TypedQuery} from the given {@link Specification}.
 	 *
-	 * @param spec can be {@literal null}.
+	 * @param spec must not be {@literal null}.
 	 * @param domainClass must not be {@literal null}.
 	 * @param pageable must not be {@literal null}.
 	 */
-	protected <S extends T> TypedQuery<S> getQuery(@Nullable Specification<S> spec, Class<S> domainClass,
+	protected <S extends T> TypedQuery<S> getQuery(Specification<S> spec, Class<S> domainClass,
 			Pageable pageable) {
 		return getQuery(spec, domainClass, pageable.getSort());
 	}
@@ -877,21 +881,23 @@ protected <S> Query getDelete(DeleteSpecification<S> spec, Class<S> domainClass)
 	/**
 	 * Creates a new count query for the given {@link Specification}.
 	 *
-	 * @param spec can be {@literal null}.
+	 * @param spec must not be {@literal null}.
 	 * @deprecated override {@link #getCountQuery(Specification, Class)} instead
 	 */
 	@Deprecated
-	protected TypedQuery<Long> getCountQuery(@Nullable Specification<T> spec) {
+	protected TypedQuery<Long> getCountQuery(Specification<T> spec) {
 		return getCountQuery(spec, getDomainClass());
 	}
 
 	/**
 	 * Creates a new count query for the given {@link Specification}.
 	 *
-	 * @param spec can be {@literal null}.
+	 * @param spec must not be {@literal null}.
 	 * @param domainClass must not be {@literal null}.
 	 */
-	protected <S extends T> TypedQuery<Long> getCountQuery(@Nullable Specification<S> spec, Class<S> domainClass) {
+	protected <S extends T> TypedQuery<Long> getCountQuery(Specification<S> spec, Class<S> domainClass) {
+
+		Assert.notNull(spec, "Specification must not be null");
 
 		CriteriaBuilder builder = entityManager.getCriteriaBuilder();
 		CriteriaQuery<Long> query = builder.createQuery(Long.class);
@@ -1041,7 +1047,7 @@ private Map<String, Object> getHints() {
 	private void applyComment(CrudMethodMetadata metadata, BiConsumer<String, Object> consumer) {
 
 		if (metadata.getComment() != null && provider.getCommentHintKey() != null) {
-			consumer.accept(provider.getCommentHintKey(), provider.getCommentHintValue(this.metadata.getComment()));
+			consumer.accept(provider.getCommentHintKey(), provider.getCommentHintValue(metadata.getComment()));
 		}
 	}
 

Diff for: spring-data-jpa/src/test/java/org/springframework/data/jpa/domain/SpecificationUnitTests.java

@@ -62,79 +62,6 @@ void setUp() {
 		spec = (root, query, cb) -> predicate;
 	}
 
-	@Test // DATAJPA-300, DATAJPA-1170
-	void createsSpecificationsFromNull() {
-
-		Specification<Object> specification = where(null);
-		assertThat(specification).isNotNull();
-		assertThat(specification.toPredicate(root, query, builder)).isNull();
-	}
-
-	@Test // DATAJPA-300, DATAJPA-1170
-	void negatesNullSpecToNull() {
-
-		Specification<Object> specification = not(null);
-
-		assertThat(specification).isNotNull();
-		assertThat(specification.toPredicate(root, query, builder)).isNull();
-	}
-
-	@Test // DATAJPA-300, DATAJPA-1170
-	void andConcatenatesSpecToNullSpec() {
-
-		Specification<Object> specification = where(null);
-		specification = specification.and(spec);
-
-		assertThat(specification).isNotNull();
-		assertThat(specification.toPredicate(root, query, builder)).isEqualTo(predicate);
-	}
-
-	@Test // DATAJPA-300, DATAJPA-1170
-	void andConcatenatesNullSpecToSpec() {
-
-		Specification<Object> specification = spec.and(null);
-
-		assertThat(specification).isNotNull();
-		assertThat(specification.toPredicate(root, query, builder)).isEqualTo(predicate);
-	}
-
-	@Test // DATAJPA-300, DATAJPA-1170
-	void orConcatenatesSpecToNullSpec() {
-
-		Specification<Object> specification = where(null);
-		specification = specification.or(spec);
-
-		assertThat(specification).isNotNull();
-		assertThat(specification.toPredicate(root, query, builder)).isEqualTo(predicate);
-	}
-
-	@Test // DATAJPA-300, DATAJPA-1170
-	void orConcatenatesNullSpecToSpec() {
-
-		Specification<Object> specification = spec.or(null);
-
-		assertThat(specification).isNotNull();
-		assertThat(specification.toPredicate(root, query, builder)).isEqualTo(predicate);
-	}
-
-	@Test // GH-1943
-	void allOfConcatenatesNull() {
-
-		Specification<Object> specification = Specification.allOf(null, spec, null);
-
-		assertThat(specification).isNotNull();
-		assertThat(specification.toPredicate(root, query, builder)).isEqualTo(predicate);
-	}
-
-	@Test // GH-1943
-	void anyOfConcatenatesNull() {
-
-		Specification<Object> specification = Specification.anyOf(null, spec, null);
-
-		assertThat(specification).isNotNull();
-		assertThat(specification.toPredicate(root, query, builder)).isEqualTo(predicate);
-	}
-
 	@Test // GH-1943
 	void emptyAllOfReturnsEmptySpecification() {