Spring/spring-graphql
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix backward scroll calculations for offset positions

Browse Source 
1. Offset to be advanced back by 1 more than the count in order to
exclude the item at the specified offset.
2. Count to be adjusted down if offset is too low.
3. Count to be set to 0 if offset is 0.

See gh-840
This commit is contained in:
rstoyanchev
2023-10-19 12:17:05 +01:00
parent 7ddb1a9941
commit 2a99c1ffa0
10 changed files with 103 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/ScrollSubrangeMethodArgumentResolver.java
View File

@@ -44,8 +44,9 @@ public class ScrollSubrangeMethodArgumentResolver extends SubrangeMethodArgument
return parameter.getParameterType().equals(ScrollSubrange.class);
}
protected ScrollSubrange createSubrange(@Nullable ScrollPosition pos, @Nullable Integer size, boolean forward) {
return new ScrollSubrange(pos, size, forward);
@Override
protected ScrollSubrange createSubrange(@Nullable ScrollPosition pos, @Nullable Integer count, boolean forward) {
return ScrollSubrange.create(pos, count, forward);
}
}

4
spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/SubrangeMethodArgumentResolver.java
View File

@@ -62,8 +62,8 @@ public class SubrangeMethodArgumentResolver<P> implements HandlerMethodArgumentR
/**
* Allows subclasses to create an extension of {@link Subrange}.
*/
protected Subrange<P> createSubrange(@Nullable P pos, @Nullable Integer size, boolean forward) {
return new Subrange<>(pos, size, forward);
protected Subrange<P> createSubrange(@Nullable P pos, @Nullable Integer count, boolean forward) {
return new Subrange<>(pos, count, forward);
}
}

4
spring-graphql/src/main/java/org/springframework/graphql/data/query/RepositoryUtils.java
View File

@@ -85,7 +85,7 @@ class RepositoryUtils {
}
public static ScrollSubrange defaultScrollSubrange() {
return new ScrollSubrange(ScrollPosition.offset(), 20, true);
return ScrollSubrange.create(ScrollPosition.offset(), 20, true);
}
public static ScrollSubrange buildScrollSubrange(
@@ -96,7 +96,7 @@ class RepositoryUtils {
Integer count = environment.getArgument(forward ? "first" : "last");
String cursor = environment.getArgument(forward ? "after" : "before");
ScrollPosition position = (cursor != null ? cursorStrategy.fromCursor(cursor) : null);
return new ScrollSubrange(position, count, forward);
return ScrollSubrange.create(position, count, forward);
}
}

63
spring-graphql/src/main/java/org/springframework/graphql/data/query/ScrollSubrange.java
View File

@@ -39,6 +39,19 @@ import org.springframework.lang.Nullable;
public final class ScrollSubrange extends Subrange<ScrollPosition> {
@SuppressWarnings("unused")
private ScrollSubrange(
@Nullable ScrollPosition pos, @Nullable Integer count, boolean forward,
@Nullable Object unused /* temporarily to differentiate from deprecated constructor */) {
super(pos, count, forward);
}
/**
* Public constructor.
* @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));
}
@@ -56,4 +69,54 @@ public final class ScrollSubrange extends Subrange<ScrollPosition> {
return pos;
}
/**
* 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
* @since 1.2.4
*/
public static ScrollSubrange create(@Nullable ScrollPosition position, @Nullable Integer count, boolean forward) {
if (position instanceof OffsetScrollPosition offsetScrollPosition) {
return initFromOffsetPosition(offsetScrollPosition, count, forward);
}
else if (position instanceof KeysetScrollPosition keysetScrollPosition) {
return initFromKeysetPosition(keysetScrollPosition, count, forward);
}
else {
return new ScrollSubrange(position, count, forward, null);
}
}
private static ScrollSubrange initFromOffsetPosition(
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);
}
forward = true;
}
return new ScrollSubrange(position, count, forward, null);
}
private static ScrollSubrange initFromKeysetPosition(
KeysetScrollPosition position, @Nullable Integer count, boolean forward) {
if (!forward) {
position = position.backward();
}
return new ScrollSubrange(position, count, forward, null);
}
}

36
spring-graphql/src/test/java/org/springframework/graphql/data/query/ScrollSubrangeTests.java
View File

@@ -41,13 +41,13 @@ public class ScrollSubrangeTests {
ScrollPosition position = ScrollPosition.offset(30);
int count = 10;
ScrollSubrange subrange = new ScrollSubrange(position, 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 = new ScrollSubrange(position, count, false);
assertThat(((OffsetScrollPosition) subrange.position().get()).getOffset()).isEqualTo(20);
subrange = ScrollSubrange.create(position, count, false);
assertThat(((OffsetScrollPosition) subrange.position().get()).getOffset()).isEqualTo(19);
assertThat(subrange.count().orElse(0)).isEqualTo(count);
assertThat(subrange.forward()).isTrue();
}
@@ -62,14 +62,14 @@ public class ScrollSubrangeTests {
ScrollPosition position = ScrollPosition.forward(keys);
int count = 10;
ScrollSubrange subrange = new ScrollSubrange(position, 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 = new ScrollSubrange(position, count, false);
subrange = ScrollSubrange.create(position, count, false);
actualPosition = (KeysetScrollPosition) subrange.position().get();
assertThat(actualPosition.getKeys()).isEqualTo(keys);
assertThat(actualPosition.getDirection()).isEqualTo(Direction.BACKWARD);
@@ -79,7 +79,7 @@ public class ScrollSubrangeTests {
@Test
void nullInput() {
ScrollSubrange subrange = new ScrollSubrange(null, null, true);
ScrollSubrange subrange = ScrollSubrange.create(null, null, true);
assertThat(subrange.position()).isNotPresent();
assertThat(subrange.count()).isNotPresent();
@@ -87,9 +87,29 @@ public class ScrollSubrangeTests {
}
@Test
void offsetBackwardPaginationNullSize() {
void offsetBackwardPaginationWithInsufficientCount() {
ScrollPosition position = ScrollPosition.offset(5);
ScrollSubrange subrange = ScrollSubrange.create(position, 10, false);
assertThat(((OffsetScrollPosition) subrange.position().get()).getOffset()).isEqualTo(0);
assertThat(subrange.count().getAsInt()).isEqualTo(4);
assertThat(subrange.forward()).isTrue();
}
@Test
void offsetBackwardPaginationWithOffsetZero() {
ScrollPosition position = ScrollPosition.offset(0);
ScrollSubrange subrange = ScrollSubrange.create(position, 10, false);
assertThat(((OffsetScrollPosition) subrange.position().get()).getOffset()).isEqualTo(0);
assertThat(subrange.count().getAsInt()).isEqualTo(0);
assertThat(subrange.forward()).isTrue();
}
@Test
void offsetBackwardPaginationWithNullCount() {
ScrollPosition position = ScrollPosition.offset(30);
ScrollSubrange subrange = new ScrollSubrange(position, null, false);
ScrollSubrange subrange = ScrollSubrange.create(position, null, false);
assertThat(((OffsetScrollPosition) subrange.position().get())).isEqualTo(position);
assertThat(subrange.count()).isNotPresent();

2
spring-graphql/src/test/java/org/springframework/graphql/data/query/jpa/QueryByExampleDataFetcherJpaTests.java
View File

@@ -256,7 +256,7 @@ class QueryByExampleDataFetcherJpaTests {
executor != null ? Collections.singletonList(executor) : Collections.emptyList(),
Collections.emptyList(),
new ScrollPositionCursorStrategy(),
new ScrollSubrange(ScrollPosition.offset(), 10, true));
ScrollSubrange.create(ScrollPosition.offset(), 10, true));
}
private WebGraphQlRequest request(String query) {

2
spring-graphql/src/test/java/org/springframework/graphql/data/query/mongo/QueryByExampleDataFetcherMongoDbTests.java
View File

@@ -236,7 +236,7 @@ class QueryByExampleDataFetcherMongoDbTests {
(executor != null ? Collections.singletonList(executor) : Collections.emptyList()),
Collections.emptyList(),
new ScrollPositionCursorStrategy(),
new ScrollSubrange(ScrollPosition.offset(), 10, true));
ScrollSubrange.create(ScrollPosition.offset(), 10, true));
}
private WebGraphQlRequest request(String query) {

2
spring-graphql/src/test/java/org/springframework/graphql/data/query/mongo/QueryByExampleDataFetcherReactiveMongoDbTests.java
View File

@@ -207,7 +207,7 @@ class QueryByExampleDataFetcherReactiveMongoDbTests {
Collections.emptyList(),
(executor != null ? Collections.singletonList(executor) : Collections.emptyList()),
new ScrollPositionCursorStrategy(),
new ScrollSubrange(ScrollPosition.offset(), 10, true));
ScrollSubrange.create(ScrollPosition.offset(), 10, true));
}
private WebGraphQlRequest request(String query) {

2
spring-graphql/src/test/java/org/springframework/graphql/data/query/neo4j/QueryByExampleDataFetcherNeo4jTests.java
View File

@@ -235,7 +235,7 @@ class QueryByExampleDataFetcherNeo4jTests {
(executor != null ? Collections.singletonList(executor) : Collections.emptyList()),
Collections.emptyList(),
new ScrollPositionCursorStrategy(),
new ScrollSubrange(ScrollPosition.offset(), 10, true));
ScrollSubrange.create(ScrollPosition.offset(), 10, true));
}
private WebGraphQlRequest request(String query) {

2
spring-graphql/src/test/java/org/springframework/graphql/data/query/neo4j/QueryByExampleDataFetcherReactiveNeo4jDbTests.java
View File

@@ -219,7 +219,7 @@ class QueryByExampleDataFetcherReactiveNeo4jDbTests {
Collections.emptyList(),
(executor != null ? Collections.singletonList(executor) : Collections.emptyList()),
new ScrollPositionCursorStrategy(),
new ScrollSubrange(ScrollPosition.offset(), 10, true));
ScrollSubrange.create(ScrollPosition.offset(), 10, true));
}
private WebGraphQlRequest request(String query) {