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

Improve logic to resolve pagination arguments

Browse Source 
The presence of either "after" or "first" leads to forward pagination.
Else if either "before" or "last" leads to backward pagination.
Fall back on forward if none are provided at all.

In addition, a small adjustment to backward pagination. If a count
is not provided, use 1 rather than 0, to advance to previous item.

Closes gh-929
This commit is contained in:
rstoyanchev
2024-03-26 20:34:52 +00:00
parent 1a3ae735c1
commit ce7f77c585
6 changed files with 165 additions and 77 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -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.
@@ -52,11 +52,18 @@ public class SubrangeMethodArgumentResolver<P> implements HandlerMethodArgumentR
@Override
public Object resolveArgument(MethodParameter parameter, DataFetchingEnvironment environment) throws Exception {
boolean forward = !environment.getArguments().containsKey("last");
Integer count = environment.getArgument(forward ? "first" : "last");
String cursor = environment.getArgument(forward ? "after" : "before");
P position = (cursor != null ? this.cursorStrategy.fromCursor(cursor) : null);
return createSubrange(position, count, forward);
boolean forward = true;
String cursor = environment.getArgument("after");
Integer count = environment.getArgument("first");
if (cursor == null && count == null) {
cursor = environment.getArgument("before");
count = environment.getArgument("last");
if (cursor != null || count != null) {
forward = false;
}
}
P pos = (cursor != null ? this.cursorStrategy.fromCursor(cursor) : null);
return createSubrange(pos, count, forward);
}
/**

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

@@ -93,18 +93,21 @@ class RepositoryUtils {
return forward -> ScrollPosition.offset();
}
public static ScrollSubrange defaultScrollSubrange() {
return ScrollSubrange.create(ScrollPosition.offset(), 20, true);
}
public static ScrollSubrange getScrollSubrange(
DataFetchingEnvironment env, CursorStrategy<ScrollPosition> strategy) {
DataFetchingEnvironment env, CursorStrategy<ScrollPosition> cursorStrategy) {
boolean forward = !env.getArguments().containsKey("last");
Integer count = env.getArgument(forward ? "first" : "last");
String cursor = env.getArgument(forward ? "after" : "before");
ScrollPosition position = (cursor != null ? strategy.fromCursor(cursor) : null);
return ScrollSubrange.create(position, count, forward);
boolean forward = true;
String cursor = env.getArgument("after");
Integer count = env.getArgument("first");
if (cursor == null && count == null) {
cursor = env.getArgument("before");
count = env.getArgument("last");
if (cursor != null || count != null) {
forward = false;
}
}
ScrollPosition pos = (cursor != null ? cursorStrategy.fromCursor(cursor) : null);
return ScrollSubrange.create(pos, count, forward);
}
}

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

@@ -110,9 +110,10 @@ public final class ScrollSubrange extends Subrange<ScrollPosition> {
position = position.advanceBy(1);
}
else {
int countOrZero = (count != null ? count : 0);
if (position.getOffset() >= countOrZero) {
position = position.advanceBy(-countOrZero);
// Advance back by 1 at least to item before position
int advanceCount = (count != null ? count : 1);
if (position.getOffset() >= advanceCount) {
position = position.advanceBy(-advanceCount);
}
else {
count = (int) position.getOffset();

82
spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/SubrangeMethodArgumentResolverTests.java
View File

@@ -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.
@@ -16,10 +16,9 @@
package org.springframework.graphql.data.method.annotation.support;
import java.util.Collections;
import java.util.Map;
import graphql.schema.DataFetchingEnvironment;
import graphql.schema.DataFetchingEnvironmentImpl;
import org.junit.jupiter.api.Test;
import org.springframework.core.MethodParameter;
@@ -28,6 +27,7 @@ import org.springframework.graphql.Book;
import org.springframework.graphql.data.method.annotation.QueryMapping;
import org.springframework.graphql.data.pagination.CursorStrategy;
import org.springframework.graphql.data.pagination.Subrange;
import org.springframework.lang.Nullable;
import org.springframework.stereotype.Controller;
import static org.assertj.core.api.Assertions.assertThat;
@@ -41,8 +41,7 @@ public class SubrangeMethodArgumentResolverTests extends ArgumentResolverTestSup
private final SubrangeMethodArgumentResolver<MyPosition> resolver =
new SubrangeMethodArgumentResolver<>(new MyPositionCursorStrategy());
private final MethodParameter param =
methodParam(BookController.class, "getBooks", Subrange.class);
private final MethodParameter param = methodParam(BookController.class, "getBooks", Subrange.class);
@Test
@@ -54,30 +53,87 @@ public class SubrangeMethodArgumentResolverTests extends ArgumentResolverTestSup
}
@Test
void forwardPagination() throws Exception {
void forward() throws Exception {
int count = 10;
int index = 25;
Map<String, Object> arguments = Map.of("first", count, "after", String.valueOf(index));
Object result = this.resolver.resolveArgument(this.param, environment(arguments));
testRequest(count, index, result, true);
assertResult(true, count, index, result);
}
@Test
void backwardPagination() throws Exception {
void forwardWithCountOnly() throws Exception {
int count = 10;
Map<String, Object> arguments = Map.of("first", count);
Object result = this.resolver.resolveArgument(this.param, environment(arguments));
assertResult(true, count, null, result);
}
@Test
void forwardWithIndexOnly() throws Exception {
int index = 25;
Map<String, Object> arguments = Map.of("after", String.valueOf(index));
Object result = this.resolver.resolveArgument(this.param, environment(arguments));
assertResult(true, null, index, result);
}
@Test
void backward() throws Exception {
int count = 20;
int index = 100;
Map<String, Object> arguments = Map.of("last", count, "before", String.valueOf(index));
Object result = this.resolver.resolveArgument(this.param, environment(arguments));
testRequest(count, index, result, false);
assertResult(false, count, index, result);
}
private static void testRequest(int count, int index, Object result, boolean forward) {
@Test
void backwardWithCountOnly() throws Exception {
int count = 10;
Map<String, Object> arguments = Map.of("last", count);
Object result = this.resolver.resolveArgument(this.param, environment(arguments));
assertResult(false, count, null, result);
}
@Test
void backwardWithIndexOnly() throws Exception {
int index = 25;
Map<String, Object> arguments = Map.of("before", String.valueOf(index));
Object result = this.resolver.resolveArgument(this.param, environment(arguments));
assertResult(false, null, index, result);
}
@Test
void noInput() throws Exception {
Object result = this.resolver.resolveArgument(this.param, environment(Collections.emptyMap()));
assertResult(true, null, null, result);
}
private static void assertResult(
boolean forward, @Nullable Integer count, @Nullable Integer index, @Nullable Object result) {
assertThat(result).isNotNull();
Subrange<MyPosition> subrange = (Subrange<MyPosition>) result;
assertThat(subrange.position().get().index()).isEqualTo(index);
assertThat(subrange.count().orElse(0)).isEqualTo(count);
assertThat(subrange.forward()).isEqualTo(forward);
if (count != null) {
assertThat(subrange.count().orElse(0)).isEqualTo(count);
}
else {
assertThat(subrange.count()).isNotPresent();
}
if (index != null) {
assertThat(subrange.position().get().index()).isEqualTo(index);
}
else {
assertThat(subrange.position()).isNotPresent();
}
}
@@ -94,7 +150,6 @@ public class SubrangeMethodArgumentResolverTests extends ArgumentResolverTestSup
public Window<Book> getBooksWithUnknownPosition(Subrange<UnknownPosition> subrange) {
return null;
}
}
@@ -115,7 +170,6 @@ public class SubrangeMethodArgumentResolverTests extends ArgumentResolverTestSup
public MyPosition fromCursor(String cursor) {
return new MyPosition(Integer.parseInt(cursor));
}
}

109
spring-graphql/src/test/java/org/springframework/graphql/data/query/RepositoryUtilsTests.java
View File

@@ -22,6 +22,7 @@ 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;
@@ -39,33 +40,59 @@ public class RepositoryUtilsTests {
@Test
void buildScrollSubrangeForward() {
OffsetScrollPosition offset = ScrollPosition.offset(10);
void forward() {
OffsetScrollPosition pos = ScrollPosition.offset(10);
int count = 5;
DataFetchingEnvironment env = environment(
Map.of("first", count, "after", cursorStrategy.toCursor(offset)));
DataFetchingEnvironment env = environment(Map.of("first", count, "after", cursorStrategy.toCursor(pos)));
ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, cursorStrategy);
assertThat(range.position().get()).isEqualTo(ScrollPosition.offset(11));
assertThat(range.count().getAsInt()).isEqualTo(count);
assertThat(range.forward()).isTrue();
assertSubrange(true, count, pos.advanceBy(1), range);
}
@Test
void buildScrollSubrangeBackward() {
OffsetScrollPosition offset = ScrollPosition.offset(10);
void backward() {
OffsetScrollPosition pos = ScrollPosition.offset(10);
int count = 5;
DataFetchingEnvironment env = environment(
Map.of("last", count, "before", cursorStrategy.toCursor(offset)));
DataFetchingEnvironment env = environment(Map.of("last", count, "before", cursorStrategy.toCursor(pos)));
ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, cursorStrategy);
assertThat(range.position().get()).isEqualTo(ScrollPosition.offset(5));
assertThat(range.count().getAsInt()).isEqualTo(count);
assertThat(range.forward()).isTrue();
assertSubrange(true, count, ScrollPosition.offset(5), range);
}
@Test
void forwardWithCountOnly() {
int count = 5;
DataFetchingEnvironment env = environment(Map.of("first", count));
ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, cursorStrategy);
assertSubrange(true, count, null, range);
}
@Test
void forwardWithPositionOnly() {
OffsetScrollPosition pos = ScrollPosition.offset(10);
DataFetchingEnvironment env = environment(Map.of("after", cursorStrategy.toCursor(pos)));
ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, cursorStrategy);
assertSubrange(true, null, pos.advanceBy(1), range);
}
@Test
void backwardWithCountOnly() {
int count = 5;
DataFetchingEnvironment env = environment(Map.of("last", count));
ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, cursorStrategy);
assertSubrange(false, count, null, range);
}
@Test
void backwardWithPositionOnly() {
OffsetScrollPosition pos = ScrollPosition.offset(10);
DataFetchingEnvironment env = environment(Map.of("before", cursorStrategy.toCursor(pos)));
ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, cursorStrategy);
assertSubrange(true, null, pos.advanceBy(-1), range);
}
@Test
@@ -73,31 +100,7 @@ public class RepositoryUtilsTests {
DataFetchingEnvironment env = environment(Collections.emptyMap());
ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, cursorStrategy);
assertThat(range.position()).isNotPresent();
assertThat(range.count()).isNotPresent();
assertThat(range.forward()).isTrue();
}
@Test
void buildScrollSubrangeForwardWithoutPosition() {
int count = 5;
DataFetchingEnvironment env = environment(Map.of("first", count));
ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, cursorStrategy);
assertThat(range.position()).isNotPresent();
assertThat(range.count().getAsInt()).isEqualTo(count);
assertThat(range.forward()).isTrue();
}
@Test
void buildScrollSubrangeBackwardWithoutPosition() {
int count = 5;
DataFetchingEnvironment env = environment(Map.of("last", count));
ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, cursorStrategy);
assertThat(range.position()).isNotPresent();
assertThat(range.count().getAsInt()).isEqualTo(count);
assertThat(range.forward()).isFalse();
assertSubrange(true, null, null, range);
}
private static DataFetchingEnvironment environment(Map<String, Object> arguments) {
@@ -106,4 +109,24 @@ public class RepositoryUtilsTests {
.build();
}
private static void assertSubrange(
boolean forward, @Nullable Integer count, @Nullable ScrollPosition pos, ScrollSubrange subrange) {
assertThat(subrange.forward()).isEqualTo(forward);
if (count != null) {
assertThat(subrange.count().orElse(0)).isEqualTo(count);
}
else {
assertThat(subrange.count()).isNotPresent();
}
if (pos != null) {
assertThat(subrange.position().get()).isEqualTo(pos);
}
else {
assertThat(subrange.position()).isNotPresent();
}
}
}

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

@@ -121,7 +121,7 @@ public class ScrollSubrangeTests {
void offsetBackwardWithNullCount() {
ScrollSubrange subrange = ScrollSubrange.create(ScrollPosition.offset(30), null, false);
assertThat(getOffset(subrange)).isEqualTo(30);
assertThat(getOffset(subrange)).isEqualTo(29);
assertThat(subrange.count()).isNotPresent();
assertThat(subrange.forward()).isTrue();
}