From bb734bf8949ba6f39e893824eb9a59b34e1afe96 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Wed, 27 Mar 2024 11:50:26 +0000 Subject: [PATCH 1/2] Remove session state from onClose only Doing the same eagerly from onError has the side effect of not being able to delegate handleConnectionClosed to interceptors. Closes gh-872 --- .../graphql/server/webmvc/GraphQlWebSocketHandler.java | 8 -------- 1 file changed, 8 deletions(-) diff --git a/spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/GraphQlWebSocketHandler.java b/spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/GraphQlWebSocketHandler.java index 65e3988a..ef5f43b6 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/GraphQlWebSocketHandler.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/server/webmvc/GraphQlWebSocketHandler.java @@ -326,14 +326,6 @@ public class GraphQlWebSocketHandler extends TextWebSocketHandler implements Sub } } - @Override - public void handleTransportError(WebSocketSession session, Throwable exception) { - SessionState info = this.sessionInfoMap.remove(session.getId()); - if (info != null) { - info.dispose(); - } - } - @Override public void afterConnectionClosed(WebSocketSession session, CloseStatus closeStatus) { String id = session.getId(); From b1aa0c67edbf1322142a9e53025530980f542454 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Wed, 27 Mar 2024 17:40:17 +0000 Subject: [PATCH 2/2] 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 --- .../data/query/WindowConnectionAdapter.java | 24 +++++++++++++++---- .../support/SchemaMappingPaginationTests.java | 12 +++++----- .../query/WindowConnectionAdapterTests.java | 7 +++--- .../QueryByExampleDataFetcherJpaTests.java | 8 +++---- ...QueryByExampleDataFetcherMongoDbTests.java | 8 +++---- ...xampleDataFetcherReactiveMongoDbTests.java | 8 +++---- .../QueryByExampleDataFetcherNeo4jTests.java | 8 +++---- ...xampleDataFetcherReactiveNeo4jDbTests.java | 8 +++---- 8 files changed, 50 insertions(+), 33 deletions(-) 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 44534947..d6f15a83 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 @@ -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. @@ -19,6 +19,7 @@ package org.springframework.graphql.data.query; import java.util.Collection; import org.springframework.data.domain.KeysetScrollPosition; +import org.springframework.data.domain.OffsetScrollPosition; import org.springframework.data.domain.ScrollPosition; import org.springframework.data.domain.Window; import org.springframework.graphql.data.pagination.ConnectionAdapter; @@ -34,6 +35,9 @@ 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); @@ -55,7 +59,7 @@ public final class WindowConnectionAdapter public boolean hasPrevious(Object container) { Window window = window(container); if (!window.isEmpty()) { - ScrollPosition position = window.positionAt(0); + ScrollPosition position = positionAt(window, 0); if (position instanceof KeysetScrollPosition keysetPosition) { return (keysetPosition.scrollsBackward() && window.hasNext()); } @@ -70,7 +74,7 @@ public final class WindowConnectionAdapter public boolean hasNext(Object container) { Window window = window(container); if (!window.isEmpty()) { - ScrollPosition pos = window.positionAt(0); + ScrollPosition pos = positionAt(window, 0); if (pos instanceof KeysetScrollPosition keysetPos) { return (keysetPos.scrollsForward() && window.hasNext()); } @@ -83,10 +87,22 @@ public final class WindowConnectionAdapter @Override public String cursorAt(Object container, int index) { - ScrollPosition position = window(container).positionAt(index); + ScrollPosition position = positionAt(window(container), 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 04e2f097..07581069 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 @@ -55,13 +55,13 @@ public class SchemaMappingPaginationTests { ResponseHelper.forResponse(response).assertData( "{\"books\":{" + "\"edges\":[" + - "{\"cursor\":\"O_0\",\"node\":{\"id\":\"4\",\"name\":\"To The Lighthouse\"}}," + - "{\"cursor\":\"O_1\",\"node\":{\"id\":\"5\",\"name\":\"Animal Farm\"}}" + + "{\"cursor\":\"O_3\",\"node\":{\"id\":\"4\",\"name\":\"To The Lighthouse\"}}," + + "{\"cursor\":\"O_4\",\"node\":{\"id\":\"5\",\"name\":\"Animal Farm\"}}" + "]," + "\"pageInfo\":{" + - "\"startCursor\":\"O_0\"," + - "\"endCursor\":\"O_1\"," + - "\"hasPreviousPage\":false," + + "\"startCursor\":\"O_3\"," + + "\"endCursor\":\"O_4\"," + + "\"hasPreviousPage\":true," + "\"hasNextPage\":false" + "}}}"); } @@ -113,7 +113,7 @@ public class SchemaMappingPaginationTests { int offset = (int) ((OffsetScrollPosition) subrange.position().orElse(ScrollPosition.offset())).getOffset(); int count = subrange.count().orElse(5); List books = BookSource.books().subList(offset, offset + count); - return Window.from(books, ScrollPosition::offset); + return Window.from(books, OffsetScrollPosition.positionFunction(offset)); } } diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/query/WindowConnectionAdapterTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/query/WindowConnectionAdapterTests.java index e8d8f65c..0193ff5f 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/query/WindowConnectionAdapterTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/query/WindowConnectionAdapterTests.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. @@ -21,6 +21,7 @@ import java.util.List; import org.junit.jupiter.api.Test; +import org.springframework.data.domain.OffsetScrollPosition; import org.springframework.data.domain.ScrollPosition; import org.springframework.data.domain.Window; import org.springframework.graphql.Book; @@ -42,7 +43,7 @@ public class WindowConnectionAdapterTests { @Test void paged() { List books = BookSource.books(); - Window window = Window.from(books, offset -> ScrollPosition.offset(35 + offset), true); + Window window = Window.from(books, OffsetScrollPosition.positionFunction(35), true); assertThat(this.adapter.getContent(window)).isEqualTo(books); assertThat(this.adapter.hasNext(window)).isTrue(); @@ -53,7 +54,7 @@ public class WindowConnectionAdapterTests { @Test void unpaged() { List books = BookSource.books(); - Window window = Window.from(books, ScrollPosition::offset); + Window window = Window.from(books, OffsetScrollPosition.positionFunction(0)); assertThat(this.adapter.getContent(window)).isEqualTo(books); assertThat(this.adapter.hasNext(window)).isFalse(); diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/query/jpa/QueryByExampleDataFetcherJpaTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/query/jpa/QueryByExampleDataFetcherJpaTests.java index 18d30715..796677f2 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/query/jpa/QueryByExampleDataFetcherJpaTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/query/jpa/QueryByExampleDataFetcherJpaTests.java @@ -141,13 +141,13 @@ class QueryByExampleDataFetcherJpaTests { List> edges = ResponseHelper.forResponse(response).toEntity("books.edges", List.class); assertThat(edges.size()).isEqualTo(2); - assertThat(edges.get(0).get("cursor")).isEqualTo("O_4"); - assertThat(edges.get(1).get("cursor")).isEqualTo("O_5"); + assertThat(edges.get(0).get("cursor")).isEqualTo("O_3"); + assertThat(edges.get(1).get("cursor")).isEqualTo("O_4"); Map pageInfo = ResponseHelper.forResponse(response).toEntity("books.pageInfo", Map.class); assertThat(pageInfo.size()).isEqualTo(4); - assertThat(pageInfo.get("startCursor")).isEqualTo("O_4"); - assertThat(pageInfo.get("endCursor")).isEqualTo("O_5"); + assertThat(pageInfo.get("startCursor")).isEqualTo("O_3"); + assertThat(pageInfo.get("endCursor")).isEqualTo("O_4"); assertThat(pageInfo.get("hasPreviousPage")).isEqualTo(true); assertThat(pageInfo.get("hasNextPage")).isEqualTo(false); }; diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/query/mongo/QueryByExampleDataFetcherMongoDbTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/query/mongo/QueryByExampleDataFetcherMongoDbTests.java index 9cd3d855..b4b21c9d 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/query/mongo/QueryByExampleDataFetcherMongoDbTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/query/mongo/QueryByExampleDataFetcherMongoDbTests.java @@ -138,13 +138,13 @@ class QueryByExampleDataFetcherMongoDbTests { List> edges = ResponseHelper.forResponse(response).toEntity("books.edges", List.class); assertThat(edges.size()).isEqualTo(2); - assertThat(edges.get(0).get("cursor")).isEqualTo("O_4"); - assertThat(edges.get(1).get("cursor")).isEqualTo("O_5"); + assertThat(edges.get(0).get("cursor")).isEqualTo("O_3"); + assertThat(edges.get(1).get("cursor")).isEqualTo("O_4"); Map pageInfo = ResponseHelper.forResponse(response).toEntity("books.pageInfo", Map.class); assertThat(pageInfo.size()).isEqualTo(4); - assertThat(pageInfo.get("startCursor")).isEqualTo("O_4"); - assertThat(pageInfo.get("endCursor")).isEqualTo("O_5"); + assertThat(pageInfo.get("startCursor")).isEqualTo("O_3"); + assertThat(pageInfo.get("endCursor")).isEqualTo("O_4"); assertThat(pageInfo.get("hasPreviousPage")).isEqualTo(true); assertThat(pageInfo.get("hasNextPage")).isEqualTo(false); }; diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/query/mongo/QueryByExampleDataFetcherReactiveMongoDbTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/query/mongo/QueryByExampleDataFetcherReactiveMongoDbTests.java index a5e32c0b..312d8c26 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/query/mongo/QueryByExampleDataFetcherReactiveMongoDbTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/query/mongo/QueryByExampleDataFetcherReactiveMongoDbTests.java @@ -163,13 +163,13 @@ class QueryByExampleDataFetcherReactiveMongoDbTests { List> edges = ResponseHelper.forResponse(response).toEntity("books.edges", List.class); assertThat(edges.size()).isEqualTo(2); - assertThat(edges.get(0).get("cursor")).isEqualTo("O_4"); - assertThat(edges.get(1).get("cursor")).isEqualTo("O_5"); + assertThat(edges.get(0).get("cursor")).isEqualTo("O_3"); + assertThat(edges.get(1).get("cursor")).isEqualTo("O_4"); Map pageInfo = ResponseHelper.forResponse(response).toEntity("books.pageInfo", Map.class); assertThat(pageInfo.size()).isEqualTo(4); - assertThat(pageInfo.get("startCursor")).isEqualTo("O_4"); - assertThat(pageInfo.get("endCursor")).isEqualTo("O_5"); + assertThat(pageInfo.get("startCursor")).isEqualTo("O_3"); + assertThat(pageInfo.get("endCursor")).isEqualTo("O_4"); assertThat(pageInfo.get("hasPreviousPage")).isEqualTo(true); assertThat(pageInfo.get("hasNextPage")).isEqualTo(false); }; diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/query/neo4j/QueryByExampleDataFetcherNeo4jTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/query/neo4j/QueryByExampleDataFetcherNeo4jTests.java index 406f096a..40dc3722 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/query/neo4j/QueryByExampleDataFetcherNeo4jTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/query/neo4j/QueryByExampleDataFetcherNeo4jTests.java @@ -140,13 +140,13 @@ class QueryByExampleDataFetcherNeo4jTests { List> edges = ResponseHelper.forResponse(response).toEntity("books.edges", List.class); assertThat(edges.size()).isEqualTo(2); - assertThat(edges.get(0).get("cursor")).isEqualTo("O_4"); - assertThat(edges.get(1).get("cursor")).isEqualTo("O_5"); + assertThat(edges.get(0).get("cursor")).isEqualTo("O_3"); + assertThat(edges.get(1).get("cursor")).isEqualTo("O_4"); Map pageInfo = ResponseHelper.forResponse(response).toEntity("books.pageInfo", Map.class); assertThat(pageInfo.size()).isEqualTo(4); - assertThat(pageInfo.get("startCursor")).isEqualTo("O_4"); - assertThat(pageInfo.get("endCursor")).isEqualTo("O_5"); + assertThat(pageInfo.get("startCursor")).isEqualTo("O_3"); + assertThat(pageInfo.get("endCursor")).isEqualTo("O_4"); assertThat(pageInfo.get("hasPreviousPage")).isEqualTo(true); assertThat(pageInfo.get("hasNextPage")).isEqualTo(false); }; diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/query/neo4j/QueryByExampleDataFetcherReactiveNeo4jDbTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/query/neo4j/QueryByExampleDataFetcherReactiveNeo4jDbTests.java index 0361f5d1..5c2f6957 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/query/neo4j/QueryByExampleDataFetcherReactiveNeo4jDbTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/query/neo4j/QueryByExampleDataFetcherReactiveNeo4jDbTests.java @@ -176,13 +176,13 @@ class QueryByExampleDataFetcherReactiveNeo4jDbTests { List> edges = ResponseHelper.forResponse(response).toEntity("books.edges", List.class); assertThat(edges.size()).isEqualTo(2); - assertThat(edges.get(0).get("cursor")).isEqualTo("O_4"); - assertThat(edges.get(1).get("cursor")).isEqualTo("O_5"); + assertThat(edges.get(0).get("cursor")).isEqualTo("O_3"); + assertThat(edges.get(1).get("cursor")).isEqualTo("O_4"); Map pageInfo = ResponseHelper.forResponse(response).toEntity("books.pageInfo", Map.class); assertThat(pageInfo.size()).isEqualTo(4); - assertThat(pageInfo.get("startCursor")).isEqualTo("O_4"); - assertThat(pageInfo.get("endCursor")).isEqualTo("O_5"); + assertThat(pageInfo.get("startCursor")).isEqualTo("O_3"); + assertThat(pageInfo.get("endCursor")).isEqualTo("O_4"); assertThat(pageInfo.get("hasPreviousPage")).isEqualTo(true); assertThat(pageInfo.get("hasNextPage")).isEqualTo(false); };