Skip to content

Commit b1aa0c6

Browse files
rstoyanchevrstoyanchev
committed
Use 0-based offset values for pagination
This is a temporary workaround to fix off-by-1 misalignment with Spring Data, which uses 1-based offset values. Once the changes in Spring Data become clear, we'll also adjust accordingly. Closes gh-925
1 parent bb734bf commit b1aa0c6

File tree

8 files changed

+50
-33
lines changed

8 files changed

+50
-33
lines changed

‎spring-graphql/src/main/java/org/springframework/graphql/data/query/WindowConnectionAdapter.java‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2020-2023 the original author or authors.
2+
* Copyright 2020-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -19,6 +19,7 @@
1919
import java.util.Collection;
2020

2121
import org.springframework.data.domain.KeysetScrollPosition;
22+
import org.springframework.data.domain.OffsetScrollPosition;
2223
import org.springframework.data.domain.ScrollPosition;
2324
import org.springframework.data.domain.Window;
2425
import org.springframework.graphql.data.pagination.ConnectionAdapter;
@@ -34,6 +35,9 @@
3435
public final class WindowConnectionAdapter
3536
extends ConnectionAdapterSupport<ScrollPosition> implements ConnectionAdapter {
3637

38+
private static final long ZERO_OFFSET_ADJUSTMENT =
39+
OffsetScrollPosition.positionFunction(0).apply(0).getOffset();
40+
3741

3842
public WindowConnectionAdapter(CursorStrategy<ScrollPosition> strategy) {
3943
super(strategy);
@@ -55,7 +59,7 @@ public <T> Collection<T> getContent(Object container) {
5559
public boolean hasPrevious(Object container) {
5660
Window<?> window = window(container);
5761
if (!window.isEmpty()) {
58-
ScrollPosition position = window.positionAt(0);
62+
ScrollPosition position = positionAt(window, 0);
5963
if (position instanceof KeysetScrollPosition keysetPosition) {
6064
return (keysetPosition.scrollsBackward() && window.hasNext());
6165
}
@@ -70,7 +74,7 @@ public boolean hasPrevious(Object container) {
7074
public boolean hasNext(Object container) {
7175
Window<?> window = window(container);
7276
if (!window.isEmpty()) {
73-
ScrollPosition pos = window.positionAt(0);
77+
ScrollPosition pos = positionAt(window, 0);
7478
if (pos instanceof KeysetScrollPosition keysetPos) {
7579
return (keysetPos.scrollsForward() && window.hasNext());
7680
}
@@ -83,10 +87,22 @@ public boolean hasNext(Object container) {
8387

8488
@Override
8589
public String cursorAt(Object container, int index) {
86-
ScrollPosition position = window(container).positionAt(index);
90+
ScrollPosition position = positionAt(window(container), index);
8791
return getCursorStrategy().toCursor(position);
8892
}
8993

94+
private ScrollPosition positionAt(Window<?> window, int index) {
95+
ScrollPosition position = window.positionAt(index);
96+
97+
// Workaround for OffsetScrollPosition#positionFunction adding 1 to the actual offset:
98+
// See https://github.com/spring-projects/spring-data-commons/issues/3070
99+
if (ZERO_OFFSET_ADJUSTMENT > 0 && position instanceof OffsetScrollPosition offsetPos) {
100+
position = offsetPos.advanceBy(-ZERO_OFFSET_ADJUSTMENT);
101+
}
102+
103+
return position;
104+
}
105+
90106
@SuppressWarnings("unchecked")
91107
private <T> Window<T> window(Object container) {
92108
return (Window<T>) container;

‎spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/SchemaMappingPaginationTests.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,13 @@ void forwardPagination() {
5555
ResponseHelper.forResponse(response).assertData(
5656
"{\"books\":{" +
5757
"\"edges\":[" +
58-
"{\"cursor\":\"O_0\",\"node\":{\"id\":\"4\",\"name\":\"To The Lighthouse\"}}," +
59-
"{\"cursor\":\"O_1\",\"node\":{\"id\":\"5\",\"name\":\"Animal Farm\"}}" +
58+
"{\"cursor\":\"O_3\",\"node\":{\"id\":\"4\",\"name\":\"To The Lighthouse\"}}," +
59+
"{\"cursor\":\"O_4\",\"node\":{\"id\":\"5\",\"name\":\"Animal Farm\"}}" +
6060
"]," +
6161
"\"pageInfo\":{" +
62-
"\"startCursor\":\"O_0\"," +
63-
"\"endCursor\":\"O_1\"," +
64-
"\"hasPreviousPage\":false," +
62+
"\"startCursor\":\"O_3\"," +
63+
"\"endCursor\":\"O_4\"," +
64+
"\"hasPreviousPage\":true," +
6565
"\"hasNextPage\":false" +
6666
"}}}");
6767
}
@@ -113,7 +113,7 @@ public Window<Book> books(ScrollSubrange subrange) {
113113
int offset = (int) ((OffsetScrollPosition) subrange.position().orElse(ScrollPosition.offset())).getOffset();
114114
int count = subrange.count().orElse(5);
115115
List<Book> books = BookSource.books().subList(offset, offset + count);
116-
return Window.from(books, ScrollPosition::offset);
116+
return Window.from(books, OffsetScrollPosition.positionFunction(offset));
117117
}
118118

119119
}

‎spring-graphql/src/test/java/org/springframework/graphql/data/query/WindowConnectionAdapterTests.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2020-2023 the original author or authors.
2+
* Copyright 2020-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -21,6 +21,7 @@
2121

2222
import org.junit.jupiter.api.Test;
2323

24+
import org.springframework.data.domain.OffsetScrollPosition;
2425
import org.springframework.data.domain.ScrollPosition;
2526
import org.springframework.data.domain.Window;
2627
import org.springframework.graphql.Book;
@@ -42,7 +43,7 @@ public class WindowConnectionAdapterTests {
4243
@Test
4344
void paged() {
4445
List<Book> books = BookSource.books();
45-
Window<Book> window = Window.from(books, offset -> ScrollPosition.offset(35 + offset), true);
46+
Window<Book> window = Window.from(books, OffsetScrollPosition.positionFunction(35), true);
4647

4748
assertThat(this.adapter.getContent(window)).isEqualTo(books);
4849
assertThat(this.adapter.hasNext(window)).isTrue();
@@ -53,7 +54,7 @@ void paged() {
5354
@Test
5455
void unpaged() {
5556
List<Book> books = BookSource.books();
56-
Window<Book> window = Window.from(books, ScrollPosition::offset);
57+
Window<Book> window = Window.from(books, OffsetScrollPosition.positionFunction(0));
5758

5859
assertThat(this.adapter.getContent(window)).isEqualTo(books);
5960
assertThat(this.adapter.hasNext(window)).isFalse();

‎spring-graphql/src/test/java/org/springframework/graphql/data/query/jpa/QueryByExampleDataFetcherJpaTests.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,13 +141,13 @@ void shouldFetchWindow() {
141141

142142
List<Map<String, Object>> edges = ResponseHelper.forResponse(response).toEntity("books.edges", List.class);
143143
assertThat(edges.size()).isEqualTo(2);
144-
assertThat(edges.get(0).get("cursor")).isEqualTo("O_4");
145-
assertThat(edges.get(1).get("cursor")).isEqualTo("O_5");
144+
assertThat(edges.get(0).get("cursor")).isEqualTo("O_3");
145+
assertThat(edges.get(1).get("cursor")).isEqualTo("O_4");
146146

147147
Map<String, Object> pageInfo = ResponseHelper.forResponse(response).toEntity("books.pageInfo", Map.class);
148148
assertThat(pageInfo.size()).isEqualTo(4);
149-
assertThat(pageInfo.get("startCursor")).isEqualTo("O_4");
150-
assertThat(pageInfo.get("endCursor")).isEqualTo("O_5");
149+
assertThat(pageInfo.get("startCursor")).isEqualTo("O_3");
150+
assertThat(pageInfo.get("endCursor")).isEqualTo("O_4");
151151
assertThat(pageInfo.get("hasPreviousPage")).isEqualTo(true);
152152
assertThat(pageInfo.get("hasNextPage")).isEqualTo(false);
153153
};

‎spring-graphql/src/test/java/org/springframework/graphql/data/query/mongo/QueryByExampleDataFetcherMongoDbTests.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -138,13 +138,13 @@ void shouldFetchWindow() {
138138

139139
List<Map<String, Object>> edges = ResponseHelper.forResponse(response).toEntity("books.edges", List.class);
140140
assertThat(edges.size()).isEqualTo(2);
141-
assertThat(edges.get(0).get("cursor")).isEqualTo("O_4");
142-
assertThat(edges.get(1).get("cursor")).isEqualTo("O_5");
141+
assertThat(edges.get(0).get("cursor")).isEqualTo("O_3");
142+
assertThat(edges.get(1).get("cursor")).isEqualTo("O_4");
143143

144144
Map<String, Object> pageInfo = ResponseHelper.forResponse(response).toEntity("books.pageInfo", Map.class);
145145
assertThat(pageInfo.size()).isEqualTo(4);
146-
assertThat(pageInfo.get("startCursor")).isEqualTo("O_4");
147-
assertThat(pageInfo.get("endCursor")).isEqualTo("O_5");
146+
assertThat(pageInfo.get("startCursor")).isEqualTo("O_3");
147+
assertThat(pageInfo.get("endCursor")).isEqualTo("O_4");
148148
assertThat(pageInfo.get("hasPreviousPage")).isEqualTo(true);
149149
assertThat(pageInfo.get("hasNextPage")).isEqualTo(false);
150150
};

‎spring-graphql/src/test/java/org/springframework/graphql/data/query/mongo/QueryByExampleDataFetcherReactiveMongoDbTests.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -163,13 +163,13 @@ void shouldFetchWindow() {
163163

164164
List<Map<String, Object>> edges = ResponseHelper.forResponse(response).toEntity("books.edges", List.class);
165165
assertThat(edges.size()).isEqualTo(2);
166-
assertThat(edges.get(0).get("cursor")).isEqualTo("O_4");
167-
assertThat(edges.get(1).get("cursor")).isEqualTo("O_5");
166+
assertThat(edges.get(0).get("cursor")).isEqualTo("O_3");
167+
assertThat(edges.get(1).get("cursor")).isEqualTo("O_4");
168168

169169
Map<String, Object> pageInfo = ResponseHelper.forResponse(response).toEntity("books.pageInfo", Map.class);
170170
assertThat(pageInfo.size()).isEqualTo(4);
171-
assertThat(pageInfo.get("startCursor")).isEqualTo("O_4");
172-
assertThat(pageInfo.get("endCursor")).isEqualTo("O_5");
171+
assertThat(pageInfo.get("startCursor")).isEqualTo("O_3");
172+
assertThat(pageInfo.get("endCursor")).isEqualTo("O_4");
173173
assertThat(pageInfo.get("hasPreviousPage")).isEqualTo(true);
174174
assertThat(pageInfo.get("hasNextPage")).isEqualTo(false);
175175
};

‎spring-graphql/src/test/java/org/springframework/graphql/data/query/neo4j/QueryByExampleDataFetcherNeo4jTests.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -140,13 +140,13 @@ void shouldFetchWindow() {
140140

141141
List<Map<String, Object>> edges = ResponseHelper.forResponse(response).toEntity("books.edges", List.class);
142142
assertThat(edges.size()).isEqualTo(2);
143-
assertThat(edges.get(0).get("cursor")).isEqualTo("O_4");
144-
assertThat(edges.get(1).get("cursor")).isEqualTo("O_5");
143+
assertThat(edges.get(0).get("cursor")).isEqualTo("O_3");
144+
assertThat(edges.get(1).get("cursor")).isEqualTo("O_4");
145145

146146
Map<String, Object> pageInfo = ResponseHelper.forResponse(response).toEntity("books.pageInfo", Map.class);
147147
assertThat(pageInfo.size()).isEqualTo(4);
148-
assertThat(pageInfo.get("startCursor")).isEqualTo("O_4");
149-
assertThat(pageInfo.get("endCursor")).isEqualTo("O_5");
148+
assertThat(pageInfo.get("startCursor")).isEqualTo("O_3");
149+
assertThat(pageInfo.get("endCursor")).isEqualTo("O_4");
150150
assertThat(pageInfo.get("hasPreviousPage")).isEqualTo(true);
151151
assertThat(pageInfo.get("hasNextPage")).isEqualTo(false);
152152
};

‎spring-graphql/src/test/java/org/springframework/graphql/data/query/neo4j/QueryByExampleDataFetcherReactiveNeo4jDbTests.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,13 +176,13 @@ void shouldFetchWindow() {
176176

177177
List<Map<String, Object>> edges = ResponseHelper.forResponse(response).toEntity("books.edges", List.class);
178178
assertThat(edges.size()).isEqualTo(2);
179-
assertThat(edges.get(0).get("cursor")).isEqualTo("O_4");
180-
assertThat(edges.get(1).get("cursor")).isEqualTo("O_5");
179+
assertThat(edges.get(0).get("cursor")).isEqualTo("O_3");
180+
assertThat(edges.get(1).get("cursor")).isEqualTo("O_4");
181181

182182
Map<String, Object> pageInfo = ResponseHelper.forResponse(response).toEntity("books.pageInfo", Map.class);
183183
assertThat(pageInfo.size()).isEqualTo(4);
184-
assertThat(pageInfo.get("startCursor")).isEqualTo("O_4");
185-
assertThat(pageInfo.get("endCursor")).isEqualTo("O_5");
184+
assertThat(pageInfo.get("startCursor")).isEqualTo("O_3");
185+
assertThat(pageInfo.get("endCursor")).isEqualTo("O_4");
186186
assertThat(pageInfo.get("hasPreviousPage")).isEqualTo(true);
187187
assertThat(pageInfo.get("hasNextPage")).isEqualTo(false);
188188
};

0 commit comments

Comments
 (0)