diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/query/ScrollSubrange.java b/spring-graphql/src/main/java/org/springframework/graphql/data/query/ScrollSubrange.java index 3cd347d3..4aefdcf6 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/query/ScrollSubrange.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/query/ScrollSubrange.java @@ -71,16 +71,20 @@ public final class ScrollSubrange extends Subrange { /** - * Create a {@link ScrollSubrange} instance. - * @param position the position relative to which to scroll, or {@code null} - * for scrolling from the beginning - * @param count the number of elements requested - * @param forward whether to return elements after (true) or before (false) - * the element at the given position - * @return the created subrange + * Create a {@link ScrollSubrange} from the given inputs. + *

Pagination with offset-based scrolling is always forward and inclusive + * of the referenced item. Therefore, an {@link OffsetScrollPosition} is + * adjusted as follows. For forward pagination, advanced by 1. For backward + * pagination, advanced back by the count, and switched to forward. + * @param position the reference position, or {@code null} if not specified + * @param count how many to return, or {@code null} if not specified + * @param forward whether scroll forward (true) or backward (false) + * @return the created instance * @since 1.2.4 */ - public static ScrollSubrange create(@Nullable ScrollPosition position, @Nullable Integer count, boolean forward) { + public static ScrollSubrange create( + @Nullable ScrollPosition position, @Nullable Integer count, boolean forward) { + if (count != null && count < 0) { count = null; } @@ -98,16 +102,24 @@ public final class ScrollSubrange extends Subrange { private static ScrollSubrange initFromOffsetPosition( OffsetScrollPosition position, @Nullable Integer count, boolean forward) { - if (!forward) { + // Offset is inclusive, adapt to exclusive: + // - for forward, add 1 to return items after position + // - for backward, subtract count to get items before position + + if (forward) { + position = position.advanceBy(1); + } + else { int countOrZero = (count != null ? count : 0); - if (countOrZero < position.getOffset()) { - position = position.advanceBy(-countOrZero-1); + if (position.getOffset() >= countOrZero) { + position = position.advanceBy(-countOrZero); } else { - count = (position.getOffset() > 0 ? (int) (position.getOffset() - 1) : 0); - position = null; + count = (int) position.getOffset(); + position = ScrollPosition.offset(); } } + return new ScrollSubrange(position, count, true, null); } diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/SchemaMappingPaginationTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/SchemaMappingPaginationTests.java index 8008091a..04e2f097 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/SchemaMappingPaginationTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/SchemaMappingPaginationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -49,7 +49,7 @@ public class SchemaMappingPaginationTests { @Test void forwardPagination() { - String document = BookSource.booksConnectionQuery("first:2, after:\"O_3\""); + String document = BookSource.booksConnectionQuery("first:2, after:\"O_2\""); Mono response = graphQlService().execute(document); ResponseHelper.forResponse(response).assertData( diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/query/QuerydslDataFetcherTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/query/QuerydslDataFetcherTests.java index b8aa82af..0eb54c67 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/query/QuerydslDataFetcherTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/query/QuerydslDataFetcherTests.java @@ -139,12 +139,12 @@ class QuerydslDataFetcherTests { ResponseHelper.forResponse(response).assertData( "{\"books\":{" + "\"edges\":[" + - "{\"cursor\":\"O_4\",\"node\":{\"id\":\"42\",\"name\":\"Hitchhiker's Guide to the Galaxy\"}}," + - "{\"cursor\":\"O_5\",\"node\":{\"id\":\"53\",\"name\":\"Breaking Bad\"}}" + + "{\"cursor\":\"O_0\",\"node\":{\"id\":\"42\",\"name\":\"Hitchhiker's Guide to the Galaxy\"}}," + + "{\"cursor\":\"O_1\",\"node\":{\"id\":\"53\",\"name\":\"Breaking Bad\"}}" + "]," + "\"pageInfo\":{" + - "\"startCursor\":\"O_4\"," + - "\"endCursor\":\"O_5\"," + + "\"startCursor\":\"O_0\"," + + "\"endCursor\":\"O_1\"," + "\"hasPreviousPage\":true," + "\"hasNextPage\":false" + "}}}" diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/query/RepositoryUtilsTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/query/RepositoryUtilsTests.java index 0a523e3b..11a8acc4 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/query/RepositoryUtilsTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/query/RepositoryUtilsTests.java @@ -50,7 +50,7 @@ public class RepositoryUtilsTests { ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, cursorStrategy, defaultSubrange); - assertThat(range.position().get()).isEqualTo(offset); + assertThat(range.position().get()).isEqualTo(ScrollPosition.offset(11)); assertThat(range.count().getAsInt()).isEqualTo(count); assertThat(range.forward()).isTrue(); } @@ -65,7 +65,7 @@ public class RepositoryUtilsTests { ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, cursorStrategy, defaultSubrange); - assertThat(range.position().get()).isEqualTo(offset.advanceBy(-count-1)); + assertThat(range.position().get()).isEqualTo(ScrollPosition.offset(5)); assertThat(range.count().getAsInt()).isEqualTo(count); assertThat(range.forward()).isTrue(); } @@ -75,7 +75,7 @@ public class RepositoryUtilsTests { DataFetchingEnvironment env = environment(Collections.emptyMap()); ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, cursorStrategy, defaultSubrange); - assertThat(range.position().get()).isEqualTo(getDefaultPosition()); + assertThat(range.position().get()).isEqualTo(getDefaultPosition().advanceBy(1)); assertThat(range.count().getAsInt()).isEqualTo(this.defaultSubrange.count().getAsInt()); assertThat(range.forward()).isTrue(); } @@ -86,7 +86,7 @@ public class RepositoryUtilsTests { DataFetchingEnvironment env = environment(Map.of("last", count)); ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, cursorStrategy, defaultSubrange); - assertThat(range.position().get()).isEqualTo(getDefaultPosition().advanceBy(-count-1)); + assertThat(range.position().get()).isEqualTo(getDefaultPosition().advanceBy(-count)); assertThat(range.count().getAsInt()).isEqualTo(count); assertThat(range.forward()).isTrue(); } diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/query/ScrollSubrangeTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/query/ScrollSubrangeTests.java index 0776e8ac..c8babeea 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/query/ScrollSubrangeTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/query/ScrollSubrangeTests.java @@ -37,40 +37,53 @@ import static org.assertj.core.api.Assertions.assertThat; public class ScrollSubrangeTests { @Test - void offset() { - ScrollPosition position = ScrollPosition.offset(30); + void offsetForward() { int count = 10; + ScrollSubrange subrange = ScrollSubrange.create(ScrollPosition.offset(30), count, true); - ScrollSubrange subrange = ScrollSubrange.create(position, count, true); - assertThat(((OffsetScrollPosition) subrange.position().get())).isEqualTo(position); - assertThat(subrange.count().orElse(0)).isEqualTo(count); - assertThat(subrange.forward()).isTrue(); - - subrange = ScrollSubrange.create(position, count, false); - assertThat(((OffsetScrollPosition) subrange.position().get()).getOffset()).isEqualTo(19); + assertThat(getOffset(subrange)).isEqualTo(31); assertThat(subrange.count().orElse(0)).isEqualTo(count); assertThat(subrange.forward()).isTrue(); } @Test - void keyset() { + void offsetBackward() { + int count = 10; + ScrollSubrange subrange = ScrollSubrange.create(ScrollPosition.offset(30), count, false); + + assertThat(getOffset(subrange)).isEqualTo(20); + assertThat(subrange.count().orElse(0)).isEqualTo(count); + assertThat(subrange.forward()).isTrue(); + } + + @Test + void keysetForward() { Map keys = new LinkedHashMap<>(); keys.put("firstName", "Joseph"); keys.put("lastName", "Heller"); keys.put("id", 103); - ScrollPosition position = ScrollPosition.forward(keys); int count = 10; + ScrollSubrange subrange = ScrollSubrange.create(ScrollPosition.forward(keys), count, true); - ScrollSubrange subrange = ScrollSubrange.create(position, count, true); KeysetScrollPosition actualPosition = (KeysetScrollPosition) subrange.position().get(); assertThat(actualPosition.getKeys()).isEqualTo(keys); assertThat(actualPosition.getDirection()).isEqualTo(Direction.FORWARD); assertThat(subrange.count().orElse(0)).isEqualTo(count); assertThat(subrange.forward()).isTrue(); + } - subrange = ScrollSubrange.create(position, count, false); - actualPosition = (KeysetScrollPosition) subrange.position().get(); + @Test + void keysetBackward() { + Map keys = new LinkedHashMap<>(); + keys.put("firstName", "Joseph"); + keys.put("lastName", "Heller"); + keys.put("id", 103); + + int count = 10; + ScrollSubrange subrange = ScrollSubrange.create(ScrollPosition.forward(keys), count, false); + + KeysetScrollPosition actualPosition = (KeysetScrollPosition) subrange.position().get(); assertThat(actualPosition.getKeys()).isEqualTo(keys); assertThat(actualPosition.getDirection()).isEqualTo(Direction.BACKWARD); assertThat(subrange.count().orElse(0)).isEqualTo(count); @@ -87,33 +100,34 @@ public class ScrollSubrangeTests { } @Test - void offsetBackwardPaginationWithInsufficientCount() { - ScrollPosition position = ScrollPosition.offset(5); - ScrollSubrange subrange = ScrollSubrange.create(position, 10, false); + void offsetBackwardWithInsufficientCount() { + ScrollSubrange subrange = ScrollSubrange.create(ScrollPosition.offset(5), 10, false); - assertThat(subrange.position()).isNotPresent(); - assertThat(subrange.count().getAsInt()).isEqualTo(4); + assertThat(getOffset(subrange)).isEqualTo(0); + assertThat(subrange.count().getAsInt()).isEqualTo(5); assertThat(subrange.forward()).isTrue(); } @Test - void offsetBackwardPaginationWithOffsetZero() { - ScrollPosition position = ScrollPosition.offset(0); - ScrollSubrange subrange = ScrollSubrange.create(position, 10, false); + void offsetBackwardFromInitialOffset() { + ScrollSubrange subrange = ScrollSubrange.create(ScrollPosition.offset(0), 10, false); - assertThat(subrange.position()).isNotPresent(); + assertThat(getOffset(subrange)).isEqualTo(0); assertThat(subrange.count().getAsInt()).isEqualTo(0); assertThat(subrange.forward()).isTrue(); } @Test - void offsetBackwardPaginationWithNullCount() { - ScrollPosition position = ScrollPosition.offset(30); - ScrollSubrange subrange = ScrollSubrange.create(position, null, false); + void offsetBackwardWithNullCount() { + ScrollSubrange subrange = ScrollSubrange.create(ScrollPosition.offset(30), null, false); - assertThat(subrange.position()).hasValue(ScrollPosition.offset(29)); + assertThat(getOffset(subrange)).isEqualTo(30); assertThat(subrange.count()).isNotPresent(); assertThat(subrange.forward()).isTrue(); } + private static long getOffset(ScrollSubrange subrange) { + return ((OffsetScrollPosition) subrange.position().get()).getOffset(); + } + } diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/query/jpa/QueryByExampleDataFetcherJpaTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/query/jpa/QueryByExampleDataFetcherJpaTests.java index 0a41f632..18d30715 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/query/jpa/QueryByExampleDataFetcherJpaTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/query/jpa/QueryByExampleDataFetcherJpaTests.java @@ -137,7 +137,7 @@ class QueryByExampleDataFetcherJpaTests { Mono response = graphQlSetup .toWebGraphQlHandler() - .handleRequest(request(BookSource.booksConnectionQuery("first:2, after:\"O_3\""))); + .handleRequest(request(BookSource.booksConnectionQuery("first:2, after:\"O_2\""))); List> edges = ResponseHelper.forResponse(response).toEntity("books.edges", List.class); assertThat(edges.size()).isEqualTo(2); diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/query/mongo/QueryByExampleDataFetcherMongoDbTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/query/mongo/QueryByExampleDataFetcherMongoDbTests.java index 58dbf134..9cd3d855 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/query/mongo/QueryByExampleDataFetcherMongoDbTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/query/mongo/QueryByExampleDataFetcherMongoDbTests.java @@ -134,7 +134,7 @@ class QueryByExampleDataFetcherMongoDbTests { Mono response = graphQlSetup .toWebGraphQlHandler() - .handleRequest(request(BookSource.booksConnectionQuery("first:2, after:\"O_3\""))); + .handleRequest(request(BookSource.booksConnectionQuery("first:2, after:\"O_2\""))); List> edges = ResponseHelper.forResponse(response).toEntity("books.edges", List.class); assertThat(edges.size()).isEqualTo(2); diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/query/mongo/QueryByExampleDataFetcherReactiveMongoDbTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/query/mongo/QueryByExampleDataFetcherReactiveMongoDbTests.java index 3019acd6..a5e32c0b 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/query/mongo/QueryByExampleDataFetcherReactiveMongoDbTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/query/mongo/QueryByExampleDataFetcherReactiveMongoDbTests.java @@ -159,7 +159,7 @@ class QueryByExampleDataFetcherReactiveMongoDbTests { Mono response = graphQlSetup .toWebGraphQlHandler() - .handleRequest(request(BookSource.booksConnectionQuery("first:2, after:\"O_3\""))); + .handleRequest(request(BookSource.booksConnectionQuery("first:2, after:\"O_2\""))); List> edges = ResponseHelper.forResponse(response).toEntity("books.edges", List.class); assertThat(edges.size()).isEqualTo(2); diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/query/neo4j/QueryByExampleDataFetcherNeo4jTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/query/neo4j/QueryByExampleDataFetcherNeo4jTests.java index 9943dd6d..406f096a 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/query/neo4j/QueryByExampleDataFetcherNeo4jTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/query/neo4j/QueryByExampleDataFetcherNeo4jTests.java @@ -136,7 +136,7 @@ class QueryByExampleDataFetcherNeo4jTests { Mono response = graphQlSetup .toWebGraphQlHandler() - .handleRequest(request(BookSource.booksConnectionQuery("first:2, after:\"O_3\""))); + .handleRequest(request(BookSource.booksConnectionQuery("first:2, after:\"O_2\""))); List> edges = ResponseHelper.forResponse(response).toEntity("books.edges", List.class); assertThat(edges.size()).isEqualTo(2); diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/query/neo4j/QueryByExampleDataFetcherReactiveNeo4jDbTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/query/neo4j/QueryByExampleDataFetcherReactiveNeo4jDbTests.java index 994162d2..0361f5d1 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/query/neo4j/QueryByExampleDataFetcherReactiveNeo4jDbTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/query/neo4j/QueryByExampleDataFetcherReactiveNeo4jDbTests.java @@ -172,7 +172,7 @@ class QueryByExampleDataFetcherReactiveNeo4jDbTests { Mono response = graphQlSetup .toWebGraphQlHandler() - .handleRequest(request(BookSource.booksConnectionQuery("first:2, after:\"O_3\""))); + .handleRequest(request(BookSource.booksConnectionQuery("first:2, after:\"O_2\""))); List> edges = ResponseHelper.forResponse(response).toEntity("books.edges", List.class); assertThat(edges.size()).isEqualTo(2);