From c152cd78d9956211cfb3617d937db5b3706c17b1 Mon Sep 17 00:00:00 2001
From: rstoyanchev <rstoyanchev@vmware.com>
Date: Tue, 16 May 2023 11:39:53 +0100
Subject: [PATCH] Improve DataFetcher handling for connection field

The ConnectionDataFetcher now handles a null return value from the
delegate DataFetcher, turning leniently into an empty connection.

Also log a debug message in case of a TrivialDataFetcher mapped to
a connection field, and do not decorate it to begin with.

Closes gh-707
---
 .../ConnectionFieldTypeVisitor.java           | 24 ++++++++--
 .../ConnectionFieldTypeVisitorTests.java      | 46 ++++++++++++++++++-
 2 files changed, 66 insertions(+), 4 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 4ad48a75..8d54d462 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
@@ -22,6 +22,7 @@ import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.CompletionStage;
 
+import graphql.TrivialDataFetcher;
 import graphql.relay.Connection;
 import graphql.relay.DefaultConnection;
 import graphql.relay.DefaultConnectionCursor;
@@ -40,8 +41,11 @@ import graphql.schema.GraphQLType;
 import graphql.schema.GraphQLTypeVisitorStub;
 import graphql.util.TraversalControl;
 import graphql.util.TraverserContext;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import reactor.core.publisher.Mono;
 
+import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 
 /**
@@ -57,6 +61,9 @@ import org.springframework.util.Assert;
  */
 public final class ConnectionFieldTypeVisitor extends GraphQLTypeVisitorStub {
 
+	private static Log logger = LogFactory.getLog(ConnectionFieldTypeVisitor.class);
+
+
 	private final ConnectionAdapter adapter;
 
 
@@ -79,7 +86,16 @@ public final class ConnectionFieldTypeVisitor extends GraphQLTypeVisitorStub {
 		}
 
 		if (isConnectionField(fieldDefinition)) {
-			codeRegistry.dataFetcher(parent, fieldDefinition, new ConnectionDataFetcher(dataFetcher, adapter));
+			if (dataFetcher instanceof TrivialDataFetcher<?>) {
+				if (logger.isDebugEnabled()) {
+					logger.debug("Connection field " +
+							"'" + parent.getName() + ":" + fieldDefinition.getName() + "' " +
+							"is mapped to trivial data fetcher: " + dataFetcher.getClass().getName());
+				}
+			}
+			else {
+				codeRegistry.dataFetcher(parent, fieldDefinition, new ConnectionDataFetcher(dataFetcher, adapter));
+			}
 		}
 
 		return TraversalControl.CONTINUE;
@@ -138,12 +154,14 @@ public final class ConnectionFieldTypeVisitor extends GraphQLTypeVisitorStub {
 		}
 
 		@SuppressWarnings("unchecked")
-		private <T> Connection<T> adapt(Object container) {
+		private <T> Connection<T> adapt(@Nullable Object container) {
 			if (container instanceof Connection<?> connection) {
 				return (Connection<T>) connection;
 			}
 
-			Collection<T> nodes = this.adapter.getContent(container);
+			Collection<T> nodes = (container != null ?
+					this.adapter.getContent(container) : Collections.emptyList());
+
 			if (nodes.isEmpty()) {
 				return (Connection<T>) EMPTY_CONNECTION;
 			}
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 ec62118e..6079c8d7 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
@@ -19,6 +19,7 @@ package org.springframework.graphql.data.pagination;
 import java.util.Collection;
 import java.util.List;
 
+import graphql.schema.PropertyDataFetcher;
 import org.junit.jupiter.api.Test;
 import reactor.core.publisher.Mono;
 
@@ -38,7 +39,7 @@ public class ConnectionFieldTypeVisitorTests {
 
 
 	@Test
-	void dataFetcherDecoration() {
+	void paginationDataFetcher() {
 
 		String document = BookSource.booksConnectionQuery("");
 
@@ -73,6 +74,49 @@ public class ConnectionFieldTypeVisitorTests {
 		);
 	}
 
+	@Test // gh-707
+	void trivialDataFetcherIsSkipped() {
+
+		TestConnectionAdapter adapter = new TestConnectionAdapter();
+		adapter.setInitialOffset(30);
+		adapter.setHasNext(true);
+
+		Mono<ExecutionGraphQlResponse> response = GraphQlSetup.schemaResource(BookSource.paginationSchema)
+				.dataFetcher("Query", "books", new PropertyDataFetcher<>("books"))
+				.typeDefinitionConfigurer(new ConnectionTypeDefinitionConfigurer())
+				.typeVisitor(ConnectionFieldTypeVisitor.create(List.of(adapter)))
+				.toGraphQlService()
+				.execute(TestExecutionRequest.forDocument(BookSource.booksConnectionQuery("")));
+
+		ResponseHelper.forResponse(response).assertData("{\"books\":null}");
+	}
+
+	@Test // gh-707
+	void nullValueTreatedAsEmptyConnection() {
+
+		TestConnectionAdapter adapter = new TestConnectionAdapter();
+		adapter.setInitialOffset(30);
+		adapter.setHasNext(true);
+
+		Mono<ExecutionGraphQlResponse> response = GraphQlSetup.schemaResource(BookSource.paginationSchema)
+				.dataFetcher("Query", "books", environment -> null)
+				.typeDefinitionConfigurer(new ConnectionTypeDefinitionConfigurer())
+				.typeVisitor(ConnectionFieldTypeVisitor.create(List.of(adapter)))
+				.toGraphQlService()
+				.execute(TestExecutionRequest.forDocument(BookSource.booksConnectionQuery("")));
+
+		ResponseHelper.forResponse(response).assertData(
+				"{\"books\":{" +
+						"\"edges\":[]," +
+						"\"pageInfo\":{" +
+						"\"startCursor\":null," +
+						"\"endCursor\":null," +
+						"\"hasPreviousPage\":false," +
+						"\"hasNextPage\":false}" +
+						"}}"
+		);
+	}
+
 
 	private static class TestConnectionAdapter implements ConnectionAdapter {