From c5d4be67796fbdbfe2e3ffc3bbca77c62e468281 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 10 Apr 2024 09:28:07 +0200 Subject: [PATCH 1/2] Prepare issue branch. --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 02d3672472..a6204f9ea6 100644 --- a/pom.xml +++ b/pom.xml @@ -24,7 +24,7 @@ org.springframework.data spring-data-neo4j - 7.3.0-SNAPSHOT + 7.3.0-GH-2890-SNAPSHOT Spring Data Neo4j Next generation Object-Graph-Mapping for Spring Data. @@ -113,7 +113,7 @@ ${skipTests} - 3.3.0-SNAPSHOT + 3.3.x-3070-SNAPSHOT From 257c9e28a6558a12e6ff90b5c2983423c37e1c7e Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 10 Apr 2024 09:45:24 +0200 Subject: [PATCH 2/2] Adopt to changes in Spring Data Commons. OffsetScrollPosition is now 0-based instead of 1-based. We differentiate between ScrollPosition.offset() as initial position and ScrollPosition.offset(0) pointing to the first returned element. Closes #2890 --- .../neo4j/repository/query/CypherQueryCreator.java | 4 +++- .../query/FetchableFluentQueryByExample.java | 2 +- .../neo4j/repository/query/FluentQuerySupport.java | 2 +- .../data/neo4j/repository/query/Neo4jQuerySupport.java | 2 +- .../repository/query/QueryFragmentsAndParameters.java | 2 +- .../repository/query/ReactiveFluentQueryByExample.java | 2 +- .../imperative/QuerydslNeo4jPredicateExecutorIT.java | 10 +++++----- .../neo4j/integration/imperative/RepositoryIT.java | 2 +- .../ReactiveQuerydslNeo4jPredicateExecutorIT.java | 8 ++++---- 9 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/springframework/data/neo4j/repository/query/CypherQueryCreator.java b/src/main/java/org/springframework/data/neo4j/repository/query/CypherQueryCreator.java index 924e5eec7d..d90b0f0bf9 100644 --- a/src/main/java/org/springframework/data/neo4j/repository/query/CypherQueryCreator.java +++ b/src/main/java/org/springframework/data/neo4j/repository/query/CypherQueryCreator.java @@ -244,7 +244,9 @@ private QueryFragments createQueryFragments(@Nullable Condition condition, Sort queryFragments.setRequiresReverseSort(keysetScrollPosition.scrollsBackward()); } else if (scrollPosition instanceof OffsetScrollPosition offsetScrollPosition) { - queryFragments.setSkip(offsetScrollPosition.getOffset()); + if (!offsetScrollPosition.isInitial()) { + queryFragments.setSkip(offsetScrollPosition.getOffset() + 1); + } queryFragments.setLimit(limitModifier.apply(pagingParameter.isUnpaged() ? maxResults.intValue() : pagingParameter.getPageSize())); } diff --git a/src/main/java/org/springframework/data/neo4j/repository/query/FetchableFluentQueryByExample.java b/src/main/java/org/springframework/data/neo4j/repository/query/FetchableFluentQueryByExample.java index b882a28e26..b40fef1245 100644 --- a/src/main/java/org/springframework/data/neo4j/repository/query/FetchableFluentQueryByExample.java +++ b/src/main/java/org/springframework/data/neo4j/repository/query/FetchableFluentQueryByExample.java @@ -170,7 +170,7 @@ public Window scroll(ScrollPosition scrollPosition) { var skip = scrollPosition.isInitial() ? 0 - : (scrollPosition instanceof OffsetScrollPosition offsetScrollPosition) ? offsetScrollPosition.getOffset() + : (scrollPosition instanceof OffsetScrollPosition offsetScrollPosition) ? offsetScrollPosition.getOffset() + 1 : 0; Condition condition = scrollPosition instanceof KeysetScrollPosition keysetScrollPosition diff --git a/src/main/java/org/springframework/data/neo4j/repository/query/FluentQuerySupport.java b/src/main/java/org/springframework/data/neo4j/repository/query/FluentQuerySupport.java index 8cfbe911e4..59fdb44048 100644 --- a/src/main/java/org/springframework/data/neo4j/repository/query/FluentQuerySupport.java +++ b/src/main/java/org/springframework/data/neo4j/repository/query/FluentQuerySupport.java @@ -90,7 +90,7 @@ final Window scroll(ScrollPosition scrollPosition, List rawResult, Neo4jPe var skip = scrollPosition.isInitial() ? 0 - : (scrollPosition instanceof OffsetScrollPosition offsetScrollPosition) ? offsetScrollPosition.getOffset() + : (scrollPosition instanceof OffsetScrollPosition offsetScrollPosition) ? offsetScrollPosition.getOffset() + 1 : 0; var scrollDirection = scrollPosition instanceof KeysetScrollPosition keysetScrollPosition ? keysetScrollPosition.getDirection() : ScrollPosition.Direction.FORWARD; diff --git a/src/main/java/org/springframework/data/neo4j/repository/query/Neo4jQuerySupport.java b/src/main/java/org/springframework/data/neo4j/repository/query/Neo4jQuerySupport.java index 93929982d2..bf133327eb 100644 --- a/src/main/java/org/springframework/data/neo4j/repository/query/Neo4jQuerySupport.java +++ b/src/main/java/org/springframework/data/neo4j/repository/query/Neo4jQuerySupport.java @@ -295,7 +295,7 @@ final Window createWindow(ResultProcessor resultProcessor, boolean incrementL return Window.from(getSubList(rawResult, limit, scrollDirection), v -> { if (scrollPosition instanceof OffsetScrollPosition offsetScrollPosition) { - return offsetScrollPosition.advanceBy(v + limit); + return offsetScrollPosition.advanceBy(v); } else { var accessor = neo4jPersistentEntity.getPropertyAccessor(rawResult.get(v)); var keys = new LinkedHashMap(); diff --git a/src/main/java/org/springframework/data/neo4j/repository/query/QueryFragmentsAndParameters.java b/src/main/java/org/springframework/data/neo4j/repository/query/QueryFragmentsAndParameters.java index 2c5a358a6a..ae732ea85a 100644 --- a/src/main/java/org/springframework/data/neo4j/repository/query/QueryFragmentsAndParameters.java +++ b/src/main/java/org/springframework/data/neo4j/repository/query/QueryFragmentsAndParameters.java @@ -273,7 +273,7 @@ static QueryFragmentsAndParameters forConditionWithScrollPosition(Neo4jPersisten if (scrollPosition instanceof OffsetScrollPosition offsetScrollPosition) { skip = offsetScrollPosition.isInitial() ? 0 - : offsetScrollPosition.getOffset(); + : offsetScrollPosition.getOffset() + 1; return forCondition(entityMetaData, condition, null, sort, null, limit, skip, includeField); } diff --git a/src/main/java/org/springframework/data/neo4j/repository/query/ReactiveFluentQueryByExample.java b/src/main/java/org/springframework/data/neo4j/repository/query/ReactiveFluentQueryByExample.java index 3aba47357e..620ebd36ff 100644 --- a/src/main/java/org/springframework/data/neo4j/repository/query/ReactiveFluentQueryByExample.java +++ b/src/main/java/org/springframework/data/neo4j/repository/query/ReactiveFluentQueryByExample.java @@ -171,7 +171,7 @@ public Mono> scroll(ScrollPosition scrollPosition) { var skip = scrollPosition.isInitial() ? 0 - : (scrollPosition instanceof OffsetScrollPosition offsetScrollPosition) ? offsetScrollPosition.getOffset() + : (scrollPosition instanceof OffsetScrollPosition offsetScrollPosition) ? offsetScrollPosition.getOffset() + 1 : 0; Condition condition = scrollPosition instanceof KeysetScrollPosition keysetScrollPosition diff --git a/src/test/java/org/springframework/data/neo4j/integration/imperative/QuerydslNeo4jPredicateExecutorIT.java b/src/test/java/org/springframework/data/neo4j/integration/imperative/QuerydslNeo4jPredicateExecutorIT.java index a6f8ce8359..fd15b4ac2d 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/imperative/QuerydslNeo4jPredicateExecutorIT.java +++ b/src/test/java/org/springframework/data/neo4j/integration/imperative/QuerydslNeo4jPredicateExecutorIT.java @@ -137,7 +137,7 @@ void scrollByExampleWithNoOffset(@Autowired QueryDSLPersonRepository repository) Predicate predicate = Expressions.predicate(Ops.EQ, firstNamePath, Expressions.asString("Helge")) .or(Expressions.predicate(Ops.EQ, lastNamePath, Expressions.asString("B."))); - Window peopleWindow = repository.findBy(predicate, q -> q.limit(1).sortBy(Sort.by("firstName").descending()).scroll(ScrollPosition.offset(0))); + Window peopleWindow = repository.findBy(predicate, q -> q.limit(1).sortBy(Sort.by("firstName").descending()).scroll(ScrollPosition.offset())); assertThat(peopleWindow.getContent()).extracting(Person::getFirstName) .containsExactlyInAnyOrder("Helge"); @@ -145,7 +145,7 @@ void scrollByExampleWithNoOffset(@Autowired QueryDSLPersonRepository repository) assertThat(peopleWindow.isLast()).isFalse(); assertThat(peopleWindow.hasNext()).isTrue(); - assertThat(peopleWindow.positionAt(peopleWindow.getContent().get(0))).isEqualTo(ScrollPosition.offset(1)); + assertThat(peopleWindow.positionAt(peopleWindow.getContent().get(0))).isEqualTo(ScrollPosition.offset(0)); } @Test @@ -154,14 +154,14 @@ void scrollByExampleWithOffset(@Autowired QueryDSLPersonRepository repository) { Predicate predicate = Expressions.predicate(Ops.EQ, firstNamePath, Expressions.asString("Helge")) .or(Expressions.predicate(Ops.EQ, lastNamePath, Expressions.asString("B."))); - Window peopleWindow = repository.findBy(predicate, q -> q.limit(1).sortBy(Sort.by("firstName").descending()).scroll(ScrollPosition.offset(1))); + Window peopleWindow = repository.findBy(predicate, q -> q.limit(1).sortBy(Sort.by("firstName").descending()).scroll(ScrollPosition.offset(0))); assertThat(peopleWindow.getContent()).extracting(Person::getFirstName) .containsExactlyInAnyOrder("Bela"); assertThat(peopleWindow.isLast()).isTrue(); - assertThat(peopleWindow.positionAt(peopleWindow.getContent().get(0))).isEqualTo(ScrollPosition.offset(2)); + assertThat(peopleWindow.positionAt(peopleWindow.getContent().get(0))).isEqualTo(ScrollPosition.offset(1)); } @Test @@ -170,7 +170,7 @@ void scrollByExampleWithContinuingOffset(@Autowired QueryDSLPersonRepository rep Predicate predicate = Expressions.predicate(Ops.EQ, firstNamePath, Expressions.asString("Helge")) .or(Expressions.predicate(Ops.EQ, lastNamePath, Expressions.asString("B."))); - Window peopleWindow = repository.findBy(predicate, q -> q.limit(1).sortBy(Sort.by("firstName").descending()).scroll(ScrollPosition.offset(0))); + Window peopleWindow = repository.findBy(predicate, q -> q.limit(1).sortBy(Sort.by("firstName").descending()).scroll(ScrollPosition.offset())); ScrollPosition currentPosition = peopleWindow.positionAt(peopleWindow.getContent().get(0)); peopleWindow = repository.findBy(predicate, q -> q.limit(1).scroll(currentPosition)); diff --git a/src/test/java/org/springframework/data/neo4j/integration/imperative/RepositoryIT.java b/src/test/java/org/springframework/data/neo4j/integration/imperative/RepositoryIT.java index 4b9982d535..9ddaf3c172 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/imperative/RepositoryIT.java +++ b/src/test/java/org/springframework/data/neo4j/integration/imperative/RepositoryIT.java @@ -2923,7 +2923,7 @@ void scrollByExample(@Autowired PersonRepository repository) { Example example = Example.of(sameValuePerson, ExampleMatcher.matchingAll().withIgnoreNullValues()); - Window person = repository.findBy(example, q -> q.sortBy(Sort.by("name")).limit(1).scroll(ScrollPosition.offset(0))); + Window person = repository.findBy(example, q -> q.sortBy(Sort.by("name")).limit(1).scroll(ScrollPosition.offset())); assertThat(person).isNotNull(); assertThat(person.getContent().get(0)).isEqualTo(person1); diff --git a/src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveQuerydslNeo4jPredicateExecutorIT.java b/src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveQuerydslNeo4jPredicateExecutorIT.java index d0818a7d27..fc1b511c2c 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveQuerydslNeo4jPredicateExecutorIT.java +++ b/src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveQuerydslNeo4jPredicateExecutorIT.java @@ -196,7 +196,7 @@ void scrollByExampleWithNoOffset(@Autowired QueryDSLPersonRepository repository) Predicate predicate = Expressions.predicate(Ops.EQ, firstNamePath, Expressions.asString("Helge")) .or(Expressions.predicate(Ops.EQ, lastNamePath, Expressions.asString("B."))); - repository.findBy(predicate, q -> q.limit(1).sortBy(Sort.by("firstName").descending()).scroll(ScrollPosition.offset(0))) + repository.findBy(predicate, q -> q.limit(1).sortBy(Sort.by("firstName").descending()).scroll(ScrollPosition.offset())) .as(StepVerifier::create) .expectNextMatches(peopleWindow -> { @@ -206,7 +206,7 @@ void scrollByExampleWithNoOffset(@Autowired QueryDSLPersonRepository repository) assertThat(peopleWindow.isLast()).isFalse(); assertThat(peopleWindow.hasNext()).isTrue(); - assertThat(peopleWindow.positionAt(peopleWindow.getContent().get(0))).isEqualTo(ScrollPosition.offset(1)); + assertThat(peopleWindow.positionAt(peopleWindow.getContent().get(0))).isEqualTo(ScrollPosition.offset(0)); return true; }).verifyComplete(); } @@ -217,14 +217,14 @@ void scrollByExampleWithOffset(@Autowired QueryDSLPersonRepository repository) { Predicate predicate = Expressions.predicate(Ops.EQ, firstNamePath, Expressions.asString("Helge")) .or(Expressions.predicate(Ops.EQ, lastNamePath, Expressions.asString("B."))); - repository.findBy(predicate, q -> q.limit(1).sortBy(Sort.by("firstName").descending()).scroll(ScrollPosition.offset(1))) + repository.findBy(predicate, q -> q.limit(1).sortBy(Sort.by("firstName").descending()).scroll(ScrollPosition.offset(0))) .as(StepVerifier::create) .expectNextMatches(peopleWindow -> { assertThat(peopleWindow.getContent()).extracting(Person::getFirstName) .containsExactlyInAnyOrder("Bela"); assertThat(peopleWindow.isLast()).isTrue(); - assertThat(peopleWindow.positionAt(peopleWindow.getContent().get(0))).isEqualTo(ScrollPosition.offset(2)); + assertThat(peopleWindow.positionAt(peopleWindow.getContent().get(0))).isEqualTo(ScrollPosition.offset(1)); return true; }).verifyComplete(); }