Skip to content

Commit d87dffd

Browse files
committed
Return null values from Connection visitor when expected
Prior to this commit, a schema declaring a `BookConnection` nullable type would decorate the relevant data fetcher and return an `EMPTY_CONNECTION` value (a full Connection instance with empty nodes and edges) if the original DataFetcher returns `null`. This is unexpected for applications because the type is declared as nullable in the schema and the application returns a `null` value. This commit ensures that `EMPTY_CONNECTION` is only returned as a value if the Connection type is marked as non nullable, `BookConnection!`. This change should only affect applications with custom pagination support, as Spring Data never returns `null` for empty pages. Closes gh-1295
1 parent b8e7a95 commit d87dffd

File tree

2 files changed

+37
-7
lines changed

2 files changed

+37
-7
lines changed

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

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
import graphql.TrivialDataFetcher;
2626
import graphql.execution.DataFetcherResult;
27+
import graphql.language.NonNullType;
2728
import graphql.relay.Connection;
2829
import graphql.relay.DefaultConnection;
2930
import graphql.relay.DefaultConnectionCursor;
@@ -101,7 +102,7 @@ public TraversalControl visitGraphQLFieldDefinition(
101102
}
102103
}
103104
else {
104-
dataFetcher = new ConnectionDataFetcher(dataFetcher, this.adapter);
105+
dataFetcher = new ConnectionDataFetcher(dataFetcher, this.adapter, fieldDefinition);
105106
codeRegistry.dataFetcher(fieldCoordinates, dataFetcher);
106107
}
107108
}
@@ -190,10 +191,11 @@ public static ConnectionFieldTypeVisitor create(List<ConnectionAdapter> adapters
190191

191192
/**
192193
* {@code DataFetcher} decorator that adapts return values with an adapter.
193-
* @param delegate the datafetcher delegate
194+
* @param delegate the DataFetcher delegate
194195
* @param adapter the connection adapter to use
196+
* @param connectionFieldDefinition the field definition for the connection type
195197
*/
196-
record ConnectionDataFetcher(DataFetcher<?> delegate, ConnectionAdapter adapter) implements DataFetcher<Object> {
198+
record ConnectionDataFetcher(DataFetcher<?> delegate, ConnectionAdapter adapter, GraphQLFieldDefinition connectionFieldDefinition) implements DataFetcher<Object> {
197199

198200
private static final Connection<?> EMPTY_CONNECTION =
199201
new DefaultConnection<>(Collections.emptyList(), new DefaultPageInfo(null, null, false, false));
@@ -202,6 +204,7 @@ record ConnectionDataFetcher(DataFetcher<?> delegate, ConnectionAdapter adapter)
202204
ConnectionDataFetcher {
203205
Assert.notNull(delegate, "DataFetcher delegate is required");
204206
Assert.notNull(adapter, "ConnectionAdapter is required");
207+
Assert.notNull(connectionFieldDefinition, "GraphQLFieldDefinition is required");
205208
}
206209

207210

@@ -232,9 +235,9 @@ private Object adaptDataFetcherResult(@Nullable Object value) {
232235
}
233236
}
234237

235-
private <T> Object adaptDataContainer(@Nullable Object container) {
238+
private <T> @Nullable Object adaptDataContainer(@Nullable Object container) {
236239
if (container == null) {
237-
return EMPTY_CONNECTION;
240+
return isConnectionTypeNullable() ? null : EMPTY_CONNECTION;
238241
}
239242

240243
if (container instanceof Connection<?>) {
@@ -268,6 +271,13 @@ private <T> Object adaptDataContainer(@Nullable Object container) {
268271
return new DefaultConnection<>(edges, pageInfo);
269272
}
270273

274+
private boolean isConnectionTypeNullable() {
275+
if (this.connectionFieldDefinition.getDefinition() != null) {
276+
return !(this.connectionFieldDefinition.getDefinition().getType() instanceof NonNullType);
277+
}
278+
return true;
279+
}
280+
271281
}
272282

273283
}

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

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,15 +137,35 @@ void customConnectionTypeIsPassedThrough() {
137137
);
138138
}
139139

140-
@Test // gh-707
141-
void nullValueTreatedAsEmptyConnection() {
140+
@Test // gh-707, gh-1295
141+
void nullValueTreatedAsNullConnectionWhenNullable() {
142142

143143
Mono<ExecutionGraphQlResponse> response = GraphQlSetup.schemaResource(BookSource.paginationSchema)
144144
.dataFetcher("Query", "books", environment -> null)
145145
.connectionSupport(new ListConnectionAdapter())
146146
.toGraphQlService()
147147
.execute(BookSource.booksConnectionQuery(null));
148148

149+
ResponseHelper.forResponse(response).assertData("{\"books\":null}");
150+
}
151+
152+
@Test // gh-1295
153+
void nullValueTreatedAsEmptyConnectionWhenNonNullable() {
154+
Mono<ExecutionGraphQlResponse> response = GraphQlSetup.schemaContent("""
155+
type Query {
156+
books(first:Int, after:String): BookConnection!
157+
}
158+
159+
type Book {
160+
id: ID
161+
name: String
162+
}
163+
""")
164+
.dataFetcher("Query", "books", environment -> null)
165+
.connectionSupport(new ListConnectionAdapter())
166+
.toGraphQlService()
167+
.execute(BookSource.booksConnectionQuery(null));
168+
149169
ResponseHelper.forResponse(response).assertData(
150170
"{\"books\":{" +
151171
"\"edges\":[]," +

0 commit comments

Comments
 (0)