diff --git a/platform/build.gradle b/platform/build.gradle index 2ce462fb..90a0fdef 100644 --- a/platform/build.gradle +++ b/platform/build.gradle @@ -12,7 +12,7 @@ dependencies { api(platform("io.projectreactor:reactor-bom:2023.0.3")) api(platform("io.micrometer:micrometer-bom:1.13.0-M1")) api(platform("io.micrometer:micrometer-tracing-bom:1.3.0-M1")) - api(platform("org.springframework.data:spring-data-bom:2023.1.3")) + api(platform("org.springframework.data:spring-data-bom:2024.0.0-SNAPSHOT")) api(platform("org.springframework.security:spring-security-bom:6.3.0-M2")) api(platform("com.querydsl:querydsl-bom:5.0.0")) api(platform("io.rsocket:rsocket-bom:1.1.4")) diff --git a/spring-graphql/build.gradle b/spring-graphql/build.gradle index 1edac9a4..636496c0 100644 --- a/spring-graphql/build.gradle +++ b/spring-graphql/build.gradle @@ -21,7 +21,7 @@ dependencies { compileOnly 'org.springframework.security:spring-security-core' compileOnly 'com.querydsl:querydsl-core' - compileOnly 'org.springframework.data:spring-data-commons:3.1.0-SNAPSHOT' + compileOnly 'org.springframework.data:spring-data-commons' compileOnly 'io.rsocket:rsocket-core' compileOnly 'io.rsocket:rsocket-transport-netty' @@ -51,14 +51,14 @@ dependencies { testImplementation 'org.springframework:spring-websocket' testImplementation 'org.springframework.data:spring-data-commons' testImplementation 'org.springframework.data:spring-data-keyvalue' - testImplementation 'org.springframework.data:spring-data-jpa:3.1.0-SNAPSHOT' + testImplementation 'org.springframework.data:spring-data-jpa' testImplementation 'io.micrometer:micrometer-observation-test' testImplementation 'io.micrometer:micrometer-tracing-test' testImplementation 'com.h2database:h2' testImplementation 'org.hibernate:hibernate-core' testImplementation 'org.hibernate.validator:hibernate-validator' testImplementation 'org.springframework.data:spring-data-mongodb' - testImplementation 'org.springframework.data:spring-data-neo4j:7.1.1' + testImplementation 'org.springframework.data:spring-data-neo4j' testImplementation 'org.mongodb:mongodb-driver-sync' testImplementation 'org.mongodb:mongodb-driver-reactivestreams' testImplementation 'org.testcontainers:mongodb' 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 488bf2f1..595f27ac 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 @@ -24,13 +24,7 @@ import org.springframework.graphql.data.pagination.Subrange; import org.springframework.lang.Nullable; /** - * Container for parameters that limit result elements to a subrange including a - * relative {@link ScrollPosition}, number of elements, and direction. - * - *

For backward pagination, the offset of an {@link OffsetScrollPosition} - * is adjusted to point to the first item in the range by subtracting the count - * from it. Hence, for {@code OffsetScrollPosition} {@link #forward()} is - * always {@code true}. + * {@link Subrange} implementation for a {@link ScrollPosition} cursor. * * @author Rossen Stoyanchev * @author Oliver Drotbohm @@ -40,45 +34,16 @@ public final class ScrollSubrange extends Subrange { @SuppressWarnings("unused") - private ScrollSubrange( - @Nullable ScrollPosition pos, @Nullable Integer count, boolean forward, - @Nullable Object unused /* temporarily to differentiate from deprecated constructor */) { - + private ScrollSubrange(@Nullable ScrollPosition pos, @Nullable Integer count, boolean forward) { super(pos, count, forward); } - /** - * Public constructor. - * @param pos 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) - * @deprecated in favor of {@link #create}, to be removed in 1.3. - */ - @Deprecated(since = "1.2.4", forRemoval = true) - public ScrollSubrange(@Nullable ScrollPosition pos, @Nullable Integer count, boolean forward) { - super(initPosition(pos, count, forward), count, (pos instanceof OffsetScrollPosition || forward)); - } - - @Nullable - private static ScrollPosition initPosition(@Nullable ScrollPosition pos, @Nullable Integer count, boolean forward) { - if (!forward) { - if (pos instanceof OffsetScrollPosition offsetPosition && count != null) { - return offsetPosition.advanceBy(-count); - } - else if (pos instanceof KeysetScrollPosition keysetPosition) { - pos = keysetPosition.backward(); - } - } - return pos; - } - /** * 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. + *

Offset scrolling is always forward and exclusive of the referenced item. + * Therefore, for backward pagination, the offset is advanced back by the + * count + 1, and the direction is 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) @@ -98,23 +63,24 @@ public final class ScrollSubrange extends Subrange { return initFromKeysetPosition(keysetScrollPosition, count, forward); } else { - return new ScrollSubrange(position, count, forward, null); + return new ScrollSubrange(position, count, forward); } } private static ScrollSubrange initFromOffsetPosition( OffsetScrollPosition position, @Nullable Integer count, boolean 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 + // Offset is exclusive: + // - for forward, nothing to do + // - for backward, subtract (count + 1) to get items before position - if (forward) { - position = position.advanceBy(1); - } - else { + if (!forward) { // Advance back by 1 at least to item before position - int advanceCount = (count != null) ? count : 1; + int advanceCount = ((count != null) ? count : 1); + + // Add 1 more to exclude item at reference position + advanceCount++; + if (position.getOffset() >= advanceCount) { position = position.advanceBy(-advanceCount); } @@ -124,7 +90,7 @@ public final class ScrollSubrange extends Subrange { } } - return new ScrollSubrange(position, count, true, null); + return new ScrollSubrange(position, count, true); } private static ScrollSubrange initFromKeysetPosition( @@ -133,7 +99,7 @@ public final class ScrollSubrange extends Subrange { if (!forward) { position = position.backward(); } - return new ScrollSubrange(position, count, forward, null); + return new ScrollSubrange(position, count, forward); } } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/query/WindowConnectionAdapter.java b/spring-graphql/src/main/java/org/springframework/graphql/data/query/WindowConnectionAdapter.java index d6f15a83..6358ae4a 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/query/WindowConnectionAdapter.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/query/WindowConnectionAdapter.java @@ -35,9 +35,6 @@ import org.springframework.graphql.data.pagination.CursorStrategy; public final class WindowConnectionAdapter extends ConnectionAdapterSupport implements ConnectionAdapter { - private static final long ZERO_OFFSET_ADJUSTMENT = - OffsetScrollPosition.positionFunction(0).apply(0).getOffset(); - public WindowConnectionAdapter(CursorStrategy strategy) { super(strategy); @@ -59,10 +56,13 @@ public final class WindowConnectionAdapter public boolean hasPrevious(Object container) { Window window = window(container); if (!window.isEmpty()) { - ScrollPosition position = positionAt(window, 0); + ScrollPosition position = window.positionAt(0); if (position instanceof KeysetScrollPosition keysetPosition) { return (keysetPosition.scrollsBackward() && window.hasNext()); } + else if (position instanceof OffsetScrollPosition offsetPosition) { + return (offsetPosition.getOffset() != 0); + } else { return !position.isInitial(); } @@ -74,7 +74,7 @@ public final class WindowConnectionAdapter public boolean hasNext(Object container) { Window window = window(container); if (!window.isEmpty()) { - ScrollPosition pos = positionAt(window, 0); + ScrollPosition pos = window.positionAt(0); if (pos instanceof KeysetScrollPosition keysetPos) { return (keysetPos.scrollsForward() && window.hasNext()); } @@ -87,22 +87,10 @@ public final class WindowConnectionAdapter @Override public String cursorAt(Object container, int index) { - ScrollPosition position = positionAt(window(container), index); + ScrollPosition position = window(container).positionAt(index); return getCursorStrategy().toCursor(position); } - private ScrollPosition positionAt(Window window, int index) { - ScrollPosition position = window.positionAt(index); - - // Workaround for OffsetScrollPosition#positionFunction adding 1 to the actual offset: - // See https://github.com/spring-projects/spring-data-commons/issues/3070 - if (ZERO_OFFSET_ADJUSTMENT > 0 && position instanceof OffsetScrollPosition offsetPos) { - position = offsetPos.advanceBy(-ZERO_OFFSET_ADJUSTMENT); - } - - return position; - } - @SuppressWarnings("unchecked") private Window window(Object container) { return (Window) container; 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 de8fd528..d25cf288 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 @@ -112,8 +112,8 @@ public class SchemaMappingPaginationTests { @QueryMapping public Window books(ScrollSubrange subrange) { int offset = (int) ((OffsetScrollPosition) subrange.position().orElse(ScrollPosition.offset())).getOffset(); - int count = subrange.count().orElse(5); - List books = BookSource.books().subList(offset, offset + count); + offset++; // data stores treat offset as exclusive + List books = BookSource.books().subList(offset, offset + subrange.count().orElse(5)); return Window.from(books, OffsetScrollPosition.positionFunction(offset)); } 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 ccf75350..d3a9db5b 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 @@ -22,11 +22,11 @@ import java.util.Map; import graphql.schema.DataFetchingEnvironment; import graphql.schema.DataFetchingEnvironmentImpl; import org.junit.jupiter.api.Test; -import org.testcontainers.shaded.org.checkerframework.checker.nullness.qual.Nullable; import org.springframework.data.domain.OffsetScrollPosition; import org.springframework.data.domain.ScrollPosition; import org.springframework.graphql.data.pagination.CursorStrategy; +import org.springframework.lang.Nullable; import static org.assertj.core.api.Assertions.assertThat; @@ -46,7 +46,7 @@ public class RepositoryUtilsTests { DataFetchingEnvironment env = environment(Map.of("first", count, "after", cursorStrategy.toCursor(pos))); ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, cursorStrategy); - assertSubrange(true, count, pos.advanceBy(1), range); + assertSubrange(true, count, pos, range); } @Test @@ -56,7 +56,7 @@ public class RepositoryUtilsTests { DataFetchingEnvironment env = environment(Map.of("last", count, "before", cursorStrategy.toCursor(pos))); ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, cursorStrategy); - assertSubrange(true, count, ScrollPosition.offset(5), range); + assertSubrange(true, count, ScrollPosition.offset(4), range); } @Test @@ -74,7 +74,7 @@ public class RepositoryUtilsTests { DataFetchingEnvironment env = environment(Map.of("after", cursorStrategy.toCursor(pos))); ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, cursorStrategy); - assertSubrange(true, null, pos.advanceBy(1), range); + assertSubrange(true, null, pos, range); } @Test @@ -92,7 +92,7 @@ public class RepositoryUtilsTests { DataFetchingEnvironment env = environment(Map.of("before", cursorStrategy.toCursor(pos))); ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, cursorStrategy); - assertSubrange(true, null, pos.advanceBy(-1), range); + assertSubrange(true, null, pos.advanceBy(-2), range); } @Test 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 bc317ae4..fab3459b 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 @@ -41,7 +41,7 @@ public class ScrollSubrangeTests { int count = 10; ScrollSubrange subrange = ScrollSubrange.create(ScrollPosition.offset(30), count, true); - assertThat(getOffset(subrange)).isEqualTo(31); + assertThat(getOffset(subrange)).isEqualTo(30); assertThat(subrange.count().orElse(0)).isEqualTo(count); assertThat(subrange.forward()).isTrue(); } @@ -51,7 +51,7 @@ public class ScrollSubrangeTests { int count = 10; ScrollSubrange subrange = ScrollSubrange.create(ScrollPosition.offset(30), count, false); - assertThat(getOffset(subrange)).isEqualTo(20); + assertThat(getOffset(subrange)).isEqualTo(19); assertThat(subrange.count().orElse(0)).isEqualTo(count); assertThat(subrange.forward()).isTrue(); } @@ -103,7 +103,7 @@ public class ScrollSubrangeTests { void offsetBackwardWithInsufficientCount() { ScrollSubrange subrange = ScrollSubrange.create(ScrollPosition.offset(5), 10, false); - assertThat(getOffset(subrange)).isEqualTo(0); + assertThat(subrange.position().get().isInitial()).isTrue(); assertThat(subrange.count().getAsInt()).isEqualTo(5); assertThat(subrange.forward()).isTrue(); } @@ -112,7 +112,7 @@ public class ScrollSubrangeTests { void offsetBackwardFromInitialOffset() { ScrollSubrange subrange = ScrollSubrange.create(ScrollPosition.offset(0), 10, false); - assertThat(getOffset(subrange)).isEqualTo(0); + assertThat(subrange.position().get().isInitial()).isTrue(); assertThat(subrange.count().getAsInt()).isEqualTo(0); assertThat(subrange.forward()).isTrue(); } @@ -121,7 +121,7 @@ public class ScrollSubrangeTests { void offsetBackwardWithNullCount() { ScrollSubrange subrange = ScrollSubrange.create(ScrollPosition.offset(30), null, false); - assertThat(getOffset(subrange)).isEqualTo(29); + assertThat(getOffset(subrange)).isEqualTo(28); assertThat(subrange.count()).isNotPresent(); assertThat(subrange.forward()).isTrue(); }