From 2596c6a641cb66b33bc2063724606e615e36a5dd Mon Sep 17 00:00:00 2001 From: Yanming Zhou Date: Mon, 12 Jun 2023 12:53:43 +0800 Subject: [PATCH] Supports scrolling base on keyset without id id shouldn't be added to sort if sort property already provided. See GH-2996 --- .../query/KeysetScrollSpecification.java | 5 + .../sample/EntityWithoutScrollableId.java | 34 +++++ .../ScrollingWithoutIdIntegrationTests.java | 143 ++++++++++++++++++ .../EntityWithoutScrollableIdRepository.java | 29 ++++ .../test/resources/META-INF/persistence.xml | 1 + .../test/resources/META-INF/persistence2.xml | 2 + 6 files changed, 214 insertions(+) create mode 100644 spring-data-jpa/src/test/java/org/springframework/data/jpa/domain/sample/EntityWithoutScrollableId.java create mode 100644 spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/ScrollingWithoutIdIntegrationTests.java create mode 100644 spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/EntityWithoutScrollableIdRepository.java diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/KeysetScrollSpecification.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/KeysetScrollSpecification.java index ee4979a637..bbf954ca43 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/KeysetScrollSpecification.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/KeysetScrollSpecification.java @@ -38,6 +38,7 @@ * * @author Mark Paluch * @author Christoph Strobl + * @author Yanming Zhou * @since 3.1 */ public record KeysetScrollSpecification (KeysetScrollPosition position, Sort sort, @@ -61,6 +62,10 @@ public static Sort createSort(KeysetScrollPosition position, Sort sort, JpaEntit KeysetScrollDelegate delegate = KeysetScrollDelegate.of(position.getDirection()); + if (sort.isSorted()) { + return delegate.getSortOrders(sort); + } + Sort sortToUse; if (entity.hasCompositeId()) { sortToUse = sort.and(Sort.by(entity.getIdAttributeNames().toArray(new String[0]))); diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/domain/sample/EntityWithoutScrollableId.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/domain/sample/EntityWithoutScrollableId.java new file mode 100644 index 0000000000..2734abe441 --- /dev/null +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/domain/sample/EntityWithoutScrollableId.java @@ -0,0 +1,34 @@ +/* + * Copyright 2019-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.jpa.domain.sample; + +import jakarta.persistence.*; +import lombok.Data; +import org.springframework.data.domain.Persistable; + +import java.util.UUID; + +/** + * @author Yanming Zhou + */ +@Entity +@Data +public class EntityWithoutScrollableId { + + private @Id UUID id = UUID.randomUUID(); + + private @Column(unique = true) int seqNo; +} diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/ScrollingWithoutIdIntegrationTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/ScrollingWithoutIdIntegrationTests.java new file mode 100644 index 0000000000..c051e72c3a --- /dev/null +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/ScrollingWithoutIdIntegrationTests.java @@ -0,0 +1,143 @@ +/* + * Copyright 2019-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.jpa.repository; + +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.domain.KeysetScrollPosition; +import org.springframework.data.domain.ScrollPosition; +import org.springframework.data.domain.Sort; +import org.springframework.data.domain.Window; +import org.springframework.data.jpa.domain.sample.EntityWithoutScrollableId; +import org.springframework.data.jpa.repository.sample.EntityWithoutScrollableIdRepository; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit.jupiter.SpringExtension; +import org.springframework.transaction.annotation.Transactional; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Yanming Zhou + */ +@ExtendWith(SpringExtension.class) +@ContextConfiguration("classpath:config/namespace-application-context.xml") +@Transactional +class ScrollingWithoutIdIntegrationTests { + + private final static int pageSize = 10; + + private final static Integer[] totals = new Integer[] { 0, 5, 10, 15, 20, 25 }; + + @Autowired + EntityWithoutScrollableIdRepository repository; + + void prepare(int total) { + for (int i = 0; i < total; i++) { + EntityWithoutScrollableId entity = new EntityWithoutScrollableId(); + entity.setSeqNo(i); + this.repository.save(entity); + } + } + + @ParameterizedTest + @MethodSource("cartesian") + void scroll(Sort.Direction sortDirection, ScrollPosition.Direction scrollDirection, int total) { + + prepare(total); + + List> contents = new ArrayList<>(); + + String propertyName = "seqNo"; + KeysetScrollPosition position = ScrollPosition.of(Map.of(), scrollDirection); + if (scrollDirection == ScrollPosition.Direction.BACKWARD && position.getDirection() == ScrollPosition.Direction.FORWARD) { + // remove this workaround if https://github.com/spring-projects/spring-data-commons/pull/2841 merged + position = position.backward(); + } + while (true) { + ScrollPosition positionToUse = position; + Window window = this.repository.findBy((root, query, cb) -> null, + q -> q.limit(pageSize).sortBy(Sort.by(sortDirection, propertyName)).scroll(positionToUse)); + if (!window.isEmpty()) { + contents.add(window.getContent()); + } + if (!window.hasNext()) { + break; + } + int indexForNext = position.scrollsForward() ? window.size() - 1 : 0; + position = ScrollPosition.of( + Map.of(propertyName,window.getContent().get(indexForNext).getSeqNo()), + scrollDirection); + // position = window.positionAt(indexForNext); https://github.com/spring-projects/spring-data-jpa/pull/3000 + // position = window.positionForNext(); https://github.com/spring-projects/spring-data-commons/pull/2843 + } + + if (total == 0) { + assertThat(contents).hasSize(0); + return; + } + + boolean divisible = total % pageSize == 0; + + assertThat(contents).hasSize(divisible ? total / pageSize : total / pageSize + 1); + for (int i = 0; i < contents.size() - 1; i++) { + assertThat(contents.get(i)).hasSize(pageSize); + } + if (divisible) { + assertThat(contents.get(contents.size() - 1)).hasSize(pageSize); + } + else { + assertThat(contents.get(contents.size() - 1)).hasSize(total % pageSize); + } + + List first = contents.get(0); + List last = contents.get(contents.size() - 1); + + if (sortDirection == Sort.Direction.ASC) { + if (scrollDirection == ScrollPosition.Direction.FORWARD) { + assertThat(first.get(0).getSeqNo()).isEqualTo(0); + assertThat(last.get(last.size() - 1).getSeqNo()).isEqualTo(total - 1); + } + else { + assertThat(first.get(first.size() - 1).getSeqNo()).isEqualTo(total - 1); + assertThat(last.get(0).getSeqNo()).isEqualTo(0); + } + } + else { + if (scrollDirection == ScrollPosition.Direction.FORWARD) { + assertThat(first.get(0).getSeqNo()).isEqualTo(total - 1); + assertThat(last.get(last.size() - 1).getSeqNo()).isEqualTo(0); + } + else { + assertThat(first.get(first.size() - 1).getSeqNo()).isEqualTo(0); + assertThat(last.get(0).getSeqNo()).isEqualTo(total - 1); + } + } + } + + private static Stream cartesian() { + return Stream.of(Sort.Direction.class.getEnumConstants()) + .flatMap(a -> Stream.of(ScrollPosition.Direction.class.getEnumConstants()) + .flatMap(b -> Stream.of(totals).map(total -> Arguments.of(a, b, total)))); + } +} diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/EntityWithoutScrollableIdRepository.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/EntityWithoutScrollableIdRepository.java new file mode 100644 index 0000000000..8e5ac60ee3 --- /dev/null +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/EntityWithoutScrollableIdRepository.java @@ -0,0 +1,29 @@ +/* + * Copyright 2019-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.jpa.repository.sample; + +import org.springframework.data.jpa.domain.sample.EntityWithoutScrollableId; +import org.springframework.data.jpa.repository.JpaSpecificationExecutor; +import org.springframework.data.repository.CrudRepository; + +import java.util.UUID; + +/** + * @author Yanming Zhou + */ +public interface EntityWithoutScrollableIdRepository extends CrudRepository, JpaSpecificationExecutor { + +} diff --git a/spring-data-jpa/src/test/resources/META-INF/persistence.xml b/spring-data-jpa/src/test/resources/META-INF/persistence.xml index c5e0e90b06..afc5b5f1bc 100644 --- a/spring-data-jpa/src/test/resources/META-INF/persistence.xml +++ b/spring-data-jpa/src/test/resources/META-INF/persistence.xml @@ -18,6 +18,7 @@ org.springframework.data.jpa.domain.sample.ConcreteType2 org.springframework.data.jpa.domain.sample.CustomAbstractPersistable org.springframework.data.jpa.domain.sample.Customer + org.springframework.data.jpa.domain.sample.EntityWithoutScrollableId org.springframework.data.jpa.domain.sample.EntityWithAssignedId org.springframework.data.jpa.domain.sample.EmbeddedIdExampleEmployeePK org.springframework.data.jpa.domain.sample.EmbeddedIdExampleEmployee diff --git a/spring-data-jpa/src/test/resources/META-INF/persistence2.xml b/spring-data-jpa/src/test/resources/META-INF/persistence2.xml index 683457aac8..b49d50d554 100644 --- a/spring-data-jpa/src/test/resources/META-INF/persistence2.xml +++ b/spring-data-jpa/src/test/resources/META-INF/persistence2.xml @@ -10,6 +10,7 @@ org.springframework.data.jpa.domain.sample.AuditableEmbeddable org.springframework.data.jpa.domain.sample.Category org.springframework.data.jpa.domain.sample.CustomAbstractPersistable + org.springframework.data.jpa.domain.sample.EntityWithoutScrollableId org.springframework.data.jpa.domain.sample.EntityWithAssignedId org.springframework.data.jpa.domain.sample.Item org.springframework.data.jpa.domain.sample.ItemSite @@ -30,6 +31,7 @@ org.springframework.data.jpa.domain.sample.AuditableRole org.springframework.data.jpa.domain.sample.Category org.springframework.data.jpa.domain.sample.CustomAbstractPersistable + org.springframework.data.jpa.domain.sample.EntityWithoutScrollableId org.springframework.data.jpa.domain.sample.EntityWithAssignedId org.springframework.data.jpa.domain.sample.Item org.springframework.data.jpa.domain.sample.ItemSite