From 08858fe133c645c619f0c4e20641af63a3c448b4 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 20 Aug 2024 14:47:34 +0200 Subject: [PATCH] Polishing. Revise nullability requirements around non-nullable specifications. Original Pull Request: #3578 --- .../repository/JpaSpecificationExecutor.java | 3 + .../FetchableFluentQueryBySpecification.java | 8 +- .../support/SimpleJpaRepository.java | 32 ++++---- .../jpa/domain/SpecificationUnitTests.java | 73 ------------------- 4 files changed, 27 insertions(+), 89 deletions(-) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/JpaSpecificationExecutor.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/JpaSpecificationExecutor.java index da03816ec..f4fac7951 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/JpaSpecificationExecutor.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/JpaSpecificationExecutor.java @@ -25,6 +25,9 @@ import java.util.List; 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 --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryBySpecification.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryBySpecification.java index 5d87904ec..68b4eb258 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryBySpecification.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryBySpecification.java @@ -26,6 +26,8 @@ import java.util.List; 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 @@ class FetchableFluentQueryBySpecification extends FluentQuerySupport @Override public Slice 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 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 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 --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java index 2f18aeca9..f006c4cd9 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java @@ -513,6 +513,7 @@ public class SimpleJpaRepository implements JpaRepositoryImplementation R doFindBy(Specification spec, Class domainClass, Function, R> queryFunction) { @@ -610,6 +611,7 @@ public class SimpleJpaRepository implements JpaRepositoryImplementation R findBy(Example example, Function, R> queryFunction) { Assert.notNull(example, EXAMPLE_MUST_NOT_BE_NULL); @@ -632,7 +634,7 @@ public class SimpleJpaRepository implements JpaRepositoryImplementation spec) { + public long count(Specification spec) { return executeCountQuery(getCountQuery(spec, getDomainClass())); } @@ -701,7 +703,7 @@ public class SimpleJpaRepository implements JpaRepositoryImplementation readPage(TypedQuery query, Pageable pageable, @Nullable Specification spec) { + protected Page readPage(TypedQuery query, Pageable pageable, Specification spec) { return readPage(query, getDomainClass(), pageable, spec); } @@ -711,11 +713,13 @@ public class SimpleJpaRepository implements JpaRepositoryImplementation Page readPage(TypedQuery query, Class domainClass, Pageable pageable, - @Nullable Specification spec) { + Specification spec) { + + Assert.notNull(spec, "Specification must not be null"); if (pageable.isPaged()) { query.setFirstResult(PageableUtils.getOffsetAsInteger(pageable)); @@ -729,21 +733,21 @@ public class SimpleJpaRepository implements JpaRepositoryImplementation getQuery(@Nullable Specification spec, Pageable pageable) { + protected TypedQuery getQuery(Specification 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 TypedQuery getQuery(@Nullable Specification spec, Class domainClass, + protected TypedQuery getQuery(Specification spec, Class domainClass, Pageable pageable) { return getQuery(spec, domainClass, pageable.getSort()); } @@ -877,21 +881,23 @@ public class SimpleJpaRepository implements JpaRepositoryImplementation getCountQuery(@Nullable Specification spec) { + protected TypedQuery getCountQuery(Specification 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 TypedQuery getCountQuery(@Nullable Specification spec, Class domainClass) { + protected TypedQuery getCountQuery(Specification spec, Class domainClass) { + + Assert.notNull(spec, "Specification must not be null"); CriteriaBuilder builder = entityManager.getCriteriaBuilder(); CriteriaQuery query = builder.createQuery(Long.class); @@ -1041,7 +1047,7 @@ public class SimpleJpaRepository implements JpaRepositoryImplementation 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 --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/domain/SpecificationUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/domain/SpecificationUnitTests.java index d45dd113d..83998fb19 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/domain/SpecificationUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/domain/SpecificationUnitTests.java @@ -62,79 +62,6 @@ class SpecificationUnitTests { spec = (root, query, cb) -> predicate; } - @Test // DATAJPA-300, DATAJPA-1170 - void createsSpecificationsFromNull() { - - Specification specification = where(null); - assertThat(specification).isNotNull(); - assertThat(specification.toPredicate(root, query, builder)).isNull(); - } - - @Test // DATAJPA-300, DATAJPA-1170 - void negatesNullSpecToNull() { - - Specification specification = not(null); - - assertThat(specification).isNotNull(); - assertThat(specification.toPredicate(root, query, builder)).isNull(); - } - - @Test // DATAJPA-300, DATAJPA-1170 - void andConcatenatesSpecToNullSpec() { - - Specification 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 specification = spec.and(null); - - assertThat(specification).isNotNull(); - assertThat(specification.toPredicate(root, query, builder)).isEqualTo(predicate); - } - - @Test // DATAJPA-300, DATAJPA-1170 - void orConcatenatesSpecToNullSpec() { - - Specification 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 specification = spec.or(null); - - assertThat(specification).isNotNull(); - assertThat(specification.toPredicate(root, query, builder)).isEqualTo(predicate); - } - - @Test // GH-1943 - void allOfConcatenatesNull() { - - Specification specification = Specification.allOf(null, spec, null); - - assertThat(specification).isNotNull(); - assertThat(specification.toPredicate(root, query, builder)).isEqualTo(predicate); - } - - @Test // GH-1943 - void anyOfConcatenatesNull() { - - Specification specification = Specification.anyOf(null, spec, null); - - assertThat(specification).isNotNull(); - assertThat(specification.toPredicate(root, query, builder)).isEqualTo(predicate); - } - @Test // GH-1943 void emptyAllOfReturnsEmptySpecification() {