From 5df008ef859ec97c411ac7cacf995bd4d8b705fa Mon Sep 17 00:00:00 2001 From: James Bodkin Date: Thu, 3 Apr 2025 10:48:39 +0100 Subject: [PATCH] Allow non-nullable types in graphql relay Signed-off-by: James Bodkin --- .../ConnectionFieldTypeVisitor.java | 14 +++++-- .../ConnectionFieldTypeVisitorTests.java | 40 ++++++++++++++++++- .../graphql/execution/BatchLoadingTests.java | 39 ++++++++++++++++++ 3 files changed, 88 insertions(+), 5 deletions(-) diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/pagination/ConnectionFieldTypeVisitor.java b/spring-graphql/src/main/java/org/springframework/graphql/data/pagination/ConnectionFieldTypeVisitor.java index 464f8e7dc..777a24d80 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/pagination/ConnectionFieldTypeVisitor.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/pagination/ConnectionFieldTypeVisitor.java @@ -153,7 +153,7 @@ private static GraphQLObjectType getAsObjectType(@Nullable GraphQLFieldDefinitio @Nullable private static GraphQLObjectType getEdgeType(@Nullable GraphQLFieldDefinition field) { if (getType(field) instanceof GraphQLList listType) { - if (listType.getWrappedType() instanceof GraphQLObjectType type) { + if (getType(listType.getWrappedType()) instanceof GraphQLObjectType type) { return type; } } @@ -169,6 +169,14 @@ private static GraphQLType getType(@Nullable GraphQLFieldDefinition field) { return (type instanceof GraphQLNonNull nonNullType) ? nonNullType.getWrappedType() : type; } + @Nullable + private static GraphQLType getType(@Nullable GraphQLType type) { + if (type == null) { + return null; + } + return (type instanceof GraphQLNonNull nonNullType) ? nonNullType.getWrappedType() : type; + } + /** * Create a {@code ConnectionTypeVisitor} instance that delegates to the @@ -185,13 +193,13 @@ public static ConnectionFieldTypeVisitor create(List adapters /** * {@code DataFetcher} decorator that adapts return values with an adapter. */ - private record ConnectionDataFetcher(DataFetcher delegate, ConnectionAdapter adapter) implements DataFetcher { + record ConnectionDataFetcher(DataFetcher delegate, ConnectionAdapter adapter) implements DataFetcher { private static final Connection EMPTY_CONNECTION = new DefaultConnection<>(Collections.emptyList(), new DefaultPageInfo(null, null, false, false)); - private ConnectionDataFetcher { + ConnectionDataFetcher { Assert.notNull(delegate, "DataFetcher delegate is required"); Assert.notNull(adapter, "ConnectionAdapter is required"); } diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/pagination/ConnectionFieldTypeVisitorTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/pagination/ConnectionFieldTypeVisitorTests.java index 84912e677..717d6a404 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/pagination/ConnectionFieldTypeVisitorTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/pagination/ConnectionFieldTypeVisitorTests.java @@ -205,6 +205,44 @@ void connectionTypeWithoutEdgesIsNotDecorated() throws Exception { assertThat(actual).isSameAs(dataFetcher); } + @Test + void connectionTypeWithNonNullEdgesIsDecorated() throws Exception { + String schemaContent = """ + type Query { + libraries(first: Int, after: String, last: Int, before: String): LibraryConnection! + } + + type LibraryConnection { + edges: [LibraryEdge!]! + pageInfo: PageInfo! + } + + type LibraryEdge { + node: Library! + cursor: String! + } + + type Library { + name: String + } + + type PageInfo { + hasPreviousPage: Boolean! + hasNextPage: Boolean! + startCursor: String + endCursor: String + } + """; + + FieldCoordinates coordinates = FieldCoordinates.coordinates("Query", "libraries"); + DataFetcher dataFetcher = env -> null; + + DataFetcher actual = + applyConnectionFieldTypeVisitor(schemaContent, coordinates, dataFetcher); + + assertThat(actual).isInstanceOf(ConnectionFieldTypeVisitor.ConnectionDataFetcher.class); + } + private static DataFetcher applyConnectionFieldTypeVisitor( Object schemaSource, FieldCoordinates coordinates, DataFetcher fetcher) throws Exception { @@ -291,8 +329,6 @@ public TraversalControl visitGraphQLFieldDefinition(GraphQLFieldDefinition node, } } - - private static class ListConnectionAdapter implements ConnectionAdapter { private int initialOffset = 0; diff --git a/spring-graphql/src/test/java/org/springframework/graphql/execution/BatchLoadingTests.java b/spring-graphql/src/test/java/org/springframework/graphql/execution/BatchLoadingTests.java index 47eca06f6..d6f9023d1 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/execution/BatchLoadingTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/execution/BatchLoadingTests.java @@ -88,4 +88,43 @@ void batchLoader() { assertThat(author.getLastName()).isEqualTo("Orwell"); } + @Test + void batchLoaderWithNullValue() { + String document = "{ " + + " booksByCriteria(criteria: {author:\"Orwell\"}) { " + + " author {" + + " firstName, " + + " lastName " + + " }" + + " }" + + "}"; + + this.registry.forTypePair(Long.class, Author.class) + .registerBatchLoader((ids, env) -> Flux.fromIterable(ids.stream().map(id -> null).toList())); + + TestExecutionGraphQlService service = GraphQlSetup.schemaResource(BookSource.schema) + .queryFetcher("booksByCriteria", env -> { + Map criteria = env.getArgument("criteria"); + String authorName = (String) criteria.get("author"); + return BookSource.findBooksByAuthor(authorName).stream() + .map(book -> new Book(book.getId(), book.getName(), book.getAuthorId())) + .collect(Collectors.toList()); + }) + .dataFetcher("Book", "author", env -> { + Book book = env.getSource(); + DataLoader dataLoader = env.getDataLoader(Author.class.getName()); + return dataLoader.load(book.getAuthorId()); + }) + .dataLoaders(this.registry) + .toGraphQlService(); + + Mono responseMono = service.execute(document); + + List books = ResponseHelper.forResponse(responseMono).toList("booksByCriteria", Book.class); + assertThat(books).hasSize(2); + + Author author = books.get(0).getAuthor(); + assertThat(author).isNull(); + } + }