Skip to content

Commit c152cd7

Browse files
committed
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
1 parent dceb578 commit c152cd7

File tree

2 files changed

+66
-4
lines changed

2 files changed

+66
-4
lines changed

spring-graphql/src/main/java/org/springframework/graphql/data/pagination/ConnectionFieldTypeVisitor.java

+21-3
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.util.List;
2323
import java.util.concurrent.CompletionStage;
2424

25+
import graphql.TrivialDataFetcher;
2526
import graphql.relay.Connection;
2627
import graphql.relay.DefaultConnection;
2728
import graphql.relay.DefaultConnectionCursor;
@@ -40,8 +41,11 @@
4041
import graphql.schema.GraphQLTypeVisitorStub;
4142
import graphql.util.TraversalControl;
4243
import graphql.util.TraverserContext;
44+
import org.apache.commons.logging.Log;
45+
import org.apache.commons.logging.LogFactory;
4346
import reactor.core.publisher.Mono;
4447

48+
import org.springframework.lang.Nullable;
4549
import org.springframework.util.Assert;
4650

4751
/**
@@ -57,6 +61,9 @@
5761
*/
5862
public final class ConnectionFieldTypeVisitor extends GraphQLTypeVisitorStub {
5963

64+
private static Log logger = LogFactory.getLog(ConnectionFieldTypeVisitor.class);
65+
66+
6067
private final ConnectionAdapter adapter;
6168

6269

@@ -79,7 +86,16 @@ public TraversalControl visitGraphQLFieldDefinition(
7986
}
8087

8188
if (isConnectionField(fieldDefinition)) {
82-
codeRegistry.dataFetcher(parent, fieldDefinition, new ConnectionDataFetcher(dataFetcher, adapter));
89+
if (dataFetcher instanceof TrivialDataFetcher<?>) {
90+
if (logger.isDebugEnabled()) {
91+
logger.debug("Connection field " +
92+
"'" + parent.getName() + ":" + fieldDefinition.getName() + "' " +
93+
"is mapped to trivial data fetcher: " + dataFetcher.getClass().getName());
94+
}
95+
}
96+
else {
97+
codeRegistry.dataFetcher(parent, fieldDefinition, new ConnectionDataFetcher(dataFetcher, adapter));
98+
}
8399
}
84100

85101
return TraversalControl.CONTINUE;
@@ -138,12 +154,14 @@ else if (result instanceof CompletionStage<?> stage) {
138154
}
139155

140156
@SuppressWarnings("unchecked")
141-
private <T> Connection<T> adapt(Object container) {
157+
private <T> Connection<T> adapt(@Nullable Object container) {
142158
if (container instanceof Connection<?> connection) {
143159
return (Connection<T>) connection;
144160
}
145161

146-
Collection<T> nodes = this.adapter.getContent(container);
162+
Collection<T> nodes = (container != null ?
163+
this.adapter.getContent(container) : Collections.emptyList());
164+
147165
if (nodes.isEmpty()) {
148166
return (Connection<T>) EMPTY_CONNECTION;
149167
}

spring-graphql/src/test/java/org/springframework/graphql/data/pagination/ConnectionFieldTypeVisitorTests.java

+45-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.util.Collection;
2020
import java.util.List;
2121

22+
import graphql.schema.PropertyDataFetcher;
2223
import org.junit.jupiter.api.Test;
2324
import reactor.core.publisher.Mono;
2425

@@ -38,7 +39,7 @@ public class ConnectionFieldTypeVisitorTests {
3839

3940

4041
@Test
41-
void dataFetcherDecoration() {
42+
void paginationDataFetcher() {
4243

4344
String document = BookSource.booksConnectionQuery("");
4445

@@ -73,6 +74,49 @@ void dataFetcherDecoration() {
7374
);
7475
}
7576

77+
@Test // gh-707
78+
void trivialDataFetcherIsSkipped() {
79+
80+
TestConnectionAdapter adapter = new TestConnectionAdapter();
81+
adapter.setInitialOffset(30);
82+
adapter.setHasNext(true);
83+
84+
Mono<ExecutionGraphQlResponse> response = GraphQlSetup.schemaResource(BookSource.paginationSchema)
85+
.dataFetcher("Query", "books", new PropertyDataFetcher<>("books"))
86+
.typeDefinitionConfigurer(new ConnectionTypeDefinitionConfigurer())
87+
.typeVisitor(ConnectionFieldTypeVisitor.create(List.of(adapter)))
88+
.toGraphQlService()
89+
.execute(TestExecutionRequest.forDocument(BookSource.booksConnectionQuery("")));
90+
91+
ResponseHelper.forResponse(response).assertData("{\"books\":null}");
92+
}
93+
94+
@Test // gh-707
95+
void nullValueTreatedAsEmptyConnection() {
96+
97+
TestConnectionAdapter adapter = new TestConnectionAdapter();
98+
adapter.setInitialOffset(30);
99+
adapter.setHasNext(true);
100+
101+
Mono<ExecutionGraphQlResponse> response = GraphQlSetup.schemaResource(BookSource.paginationSchema)
102+
.dataFetcher("Query", "books", environment -> null)
103+
.typeDefinitionConfigurer(new ConnectionTypeDefinitionConfigurer())
104+
.typeVisitor(ConnectionFieldTypeVisitor.create(List.of(adapter)))
105+
.toGraphQlService()
106+
.execute(TestExecutionRequest.forDocument(BookSource.booksConnectionQuery("")));
107+
108+
ResponseHelper.forResponse(response).assertData(
109+
"{\"books\":{" +
110+
"\"edges\":[]," +
111+
"\"pageInfo\":{" +
112+
"\"startCursor\":null," +
113+
"\"endCursor\":null," +
114+
"\"hasPreviousPage\":false," +
115+
"\"hasNextPage\":false}" +
116+
"}}"
117+
);
118+
}
119+
76120

77121
private static class TestConnectionAdapter implements ConnectionAdapter {
78122

0 commit comments

Comments
 (0)