From 3ec8e504f0975ef844727535bbe1699f0f06a30d Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 10 Apr 2024 09:45:24 +0200 Subject: [PATCH] Align OffsetScrolling to zero-based indexes. 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. Remove unused variable. Closes #2890 Original pull request: #2891 --- .../neo4j/repository/query/CypherQueryCreator.java | 4 +++- .../query/FetchableFluentQueryByExample.java | 2 +- .../neo4j/repository/query/FluentQuerySupport.java | 2 +- .../data/neo4j/repository/query/Neo4jQuerySupport.java | 3 +-- .../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(+), 17 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 924e5eec7..d90b0f0bf 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 @@ final class CypherQueryCreator extends AbstractQueryCreator extends FluentQuerySupport im 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 8cfbe911e..59fdb4404 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 @@ abstract class FluentQuerySupport { 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 93929982d..a9a02fb4f 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 @@ -285,7 +285,6 @@ abstract class Neo4jQuerySupport { var domainType = resultProcessor.getReturnedType().getDomainType(); var neo4jPersistentEntity = mappingContext.getPersistentEntity(domainType); var limit = orderBy.getQueryFragments().getLimit().intValue() - (incrementLimit ? 1 : 0); - var conversionService = mappingContext.getConversionService(); var scrollPosition = parameterAccessor.getScrollPosition(); var scrollDirection = scrollPosition instanceof KeysetScrollPosition keysetScrollPosition ? keysetScrollPosition.getDirection() : Direction.FORWARD; @@ -295,7 +294,7 @@ abstract class Neo4jQuerySupport { 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 2c5a358a6..ae732ea85 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 @@ public final class QueryFragmentsAndParameters { 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 3aba47357..620ebd36f 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 @@ final class ReactiveFluentQueryByExample extends FluentQuerySupport imp 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 a6f8ce835..fd15b4ac2 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 @@ class QuerydslNeo4jPredicateExecutorIT { 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 @@ class QuerydslNeo4jPredicateExecutorIT { 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 @@ class QuerydslNeo4jPredicateExecutorIT { 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 @@ class QuerydslNeo4jPredicateExecutorIT { 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 4b9982d53..9ddaf3c17 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 @@ class RepositoryIT { 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 d0818a7d2..fc1b511c2 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 @@ class ReactiveQuerydslNeo4jPredicateExecutorIT { 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 @@ class ReactiveQuerydslNeo4jPredicateExecutorIT { 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 @@ class ReactiveQuerydslNeo4jPredicateExecutorIT { 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(); }