From b7842ff72f5f632bfbd937a05478c53f95543652 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Fri, 19 Jan 2024 21:19:09 +0000 Subject: [PATCH] Fix count and offset calculations for backwards pagination Closes gh-873 --- .../graphql/data/query/ScrollSubrange.java | 23 ++++++++++--------- .../data/query/ScrollSubrangeTests.java | 8 +++---- 2 files changed, 16 insertions(+), 15 deletions(-) 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 79c89b2e..3cd347d3 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 @@ -1,5 +1,5 @@ /* - * Copyright 2020-2023 the original author or authors. + * Copyright 2020-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. @@ -81,6 +81,9 @@ public final class ScrollSubrange extends Subrange { * @since 1.2.4 */ public static ScrollSubrange create(@Nullable ScrollPosition position, @Nullable Integer count, boolean forward) { + if (count != null && count < 0) { + count = null; + } if (position instanceof OffsetScrollPosition offsetScrollPosition) { return initFromOffsetPosition(offsetScrollPosition, count, forward); } @@ -96,18 +99,16 @@ public final class ScrollSubrange extends Subrange { OffsetScrollPosition position, @Nullable Integer count, boolean forward) { if (!forward) { - if (count != null) { - if (position.getOffset() == 0) { - count = 0; - } - else if (count >= position.getOffset()) { - count = (int) (position.getOffset() - 1); - } - position = position.advanceBy(-count - 1); + int countOrZero = (count != null ? count : 0); + if (countOrZero < position.getOffset()) { + position = position.advanceBy(-countOrZero-1); + } + else { + count = (position.getOffset() > 0 ? (int) (position.getOffset() - 1) : 0); + position = null; } - forward = true; } - return new ScrollSubrange(position, count, forward, null); + return new ScrollSubrange(position, count, true, null); } private static ScrollSubrange initFromKeysetPosition( 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 a829751d..0776e8ac 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 @@ -1,5 +1,5 @@ /* - * Copyright 2020-2023 the original author or authors. + * Copyright 2020-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. @@ -91,7 +91,7 @@ public class ScrollSubrangeTests { ScrollPosition position = ScrollPosition.offset(5); ScrollSubrange subrange = ScrollSubrange.create(position, 10, false); - assertThat(((OffsetScrollPosition) subrange.position().get()).getOffset()).isEqualTo(0); + assertThat(subrange.position()).isNotPresent(); assertThat(subrange.count().getAsInt()).isEqualTo(4); assertThat(subrange.forward()).isTrue(); } @@ -101,7 +101,7 @@ public class ScrollSubrangeTests { ScrollPosition position = ScrollPosition.offset(0); ScrollSubrange subrange = ScrollSubrange.create(position, 10, false); - assertThat(((OffsetScrollPosition) subrange.position().get()).getOffset()).isEqualTo(0); + assertThat(subrange.position()).isNotPresent(); assertThat(subrange.count().getAsInt()).isEqualTo(0); assertThat(subrange.forward()).isTrue(); } @@ -111,7 +111,7 @@ public class ScrollSubrangeTests { ScrollPosition position = ScrollPosition.offset(30); ScrollSubrange subrange = ScrollSubrange.create(position, null, false); - assertThat(((OffsetScrollPosition) subrange.position().get())).isEqualTo(position); + assertThat(subrange.position()).hasValue(ScrollPosition.offset(29)); assertThat(subrange.count()).isNotPresent(); assertThat(subrange.forward()).isTrue(); }