Skip to content

Commit 73c2bec

Browse files
bclozelbclozel
committed
Merge branch '1.4.x'
2 parents c8abf73 + d87dffd commit 73c2bec

File tree

4 files changed

+81
-16
lines changed

4 files changed

+81
-16
lines changed

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

Lines changed: 19 additions & 7 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;
@@ -50,6 +51,7 @@
5051
import reactor.core.publisher.Mono;
5152

5253
import org.springframework.graphql.execution.TypeVisitorHelper;
54+
import org.springframework.lang.Contract;
5355
import org.springframework.util.Assert;
5456

5557
/**
@@ -101,7 +103,7 @@ public TraversalControl visitGraphQLFieldDefinition(
101103
}
102104
}
103105
else {
104-
dataFetcher = new ConnectionDataFetcher(dataFetcher, this.adapter);
106+
dataFetcher = new ConnectionDataFetcher(dataFetcher, this.adapter, fieldDefinition);
105107
codeRegistry.dataFetcher(fieldCoordinates, dataFetcher);
106108
}
107109
}
@@ -186,10 +188,11 @@ public static ConnectionFieldTypeVisitor create(List<ConnectionAdapter> adapters
186188

187189
/**
188190
* {@code DataFetcher} decorator that adapts return values with an adapter.
189-
* @param delegate the datafetcher delegate
191+
* @param delegate the DataFetcher delegate
190192
* @param adapter the connection adapter to use
193+
* @param connectionFieldDefinition the field definition for the connection type
191194
*/
192-
record ConnectionDataFetcher(DataFetcher<?> delegate, ConnectionAdapter adapter) implements DataFetcher<Object> {
195+
record ConnectionDataFetcher(DataFetcher<?> delegate, ConnectionAdapter adapter, GraphQLFieldDefinition connectionFieldDefinition) implements DataFetcher<Object> {
193196

194197
private static final Connection<?> EMPTY_CONNECTION =
195198
new DefaultConnection<>(Collections.emptyList(), new DefaultPageInfo(null, null, false, false));
@@ -198,11 +201,12 @@ record ConnectionDataFetcher(DataFetcher<?> delegate, ConnectionAdapter adapter)
198201
ConnectionDataFetcher {
199202
Assert.notNull(delegate, "DataFetcher delegate is required");
200203
Assert.notNull(adapter, "ConnectionAdapter is required");
204+
Assert.notNull(connectionFieldDefinition, "GraphQLFieldDefinition is required");
201205
}
202206

203207

204208
@Override
205-
public Object get(DataFetchingEnvironment environment) throws Exception {
209+
public @Nullable Object get(DataFetchingEnvironment environment) throws Exception {
206210
Object result = this.delegate.get(environment);
207211
if (result instanceof Mono<?> mono) {
208212
return mono.map(this::adaptDataFetcherResult);
@@ -215,7 +219,8 @@ else if (result instanceof CompletionStage<?> stage) {
215219
}
216220
}
217221

218-
private Object adaptDataFetcherResult(@Nullable Object value) {
222+
@Contract("!null -> !null")
223+
private @Nullable Object adaptDataFetcherResult(@Nullable Object value) {
219224
if (value instanceof DataFetcherResult<?> dataFetcherResult) {
220225
Object adapted = adaptDataContainer(dataFetcherResult.getData());
221226
return DataFetcherResult.newResult()
@@ -228,9 +233,9 @@ private Object adaptDataFetcherResult(@Nullable Object value) {
228233
}
229234
}
230235

231-
private <T> Object adaptDataContainer(@Nullable Object container) {
236+
private <T> @Nullable Object adaptDataContainer(@Nullable Object container) {
232237
if (container == null) {
233-
return EMPTY_CONNECTION;
238+
return isConnectionTypeNullable() ? null : EMPTY_CONNECTION;
234239
}
235240

236241
if (container instanceof Connection<?>) {
@@ -264,6 +269,13 @@ private <T> Object adaptDataContainer(@Nullable Object container) {
264269
return new DefaultConnection<>(edges, pageInfo);
265270
}
266271

272+
private boolean isConnectionTypeNullable() {
273+
if (this.connectionFieldDefinition.getDefinition() != null) {
274+
return !(this.connectionFieldDefinition.getDefinition().getType() instanceof NonNullType);
275+
}
276+
return true;
277+
}
278+
267279
}
268280

269281
}

‎spring-graphql/src/main/java/org/springframework/graphql/data/query/PropertySelection.java

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424

2525
import graphql.schema.DataFetchingFieldSelectionSet;
2626
import graphql.schema.GraphQLNamedOutputType;
27+
import graphql.schema.GraphQLNonNull;
28+
import graphql.schema.GraphQLType;
2729
import graphql.schema.SelectedField;
2830

2931
import org.springframework.data.mapping.PropertyPath;
@@ -108,9 +110,16 @@ else if (isConnectionEdgeNode(selectedField)) {
108110
}
109111

110112
private static boolean isConnectionEdges(SelectedField selectedField) {
111-
return selectedField.getName().equals("edges") &&
112-
selectedField.getParentField().getType() instanceof GraphQLNamedOutputType namedType &&
113-
namedType.getName().endsWith("Connection");
113+
if (selectedField.getName().equals("edges")) {
114+
GraphQLType fieldType = selectedField.getParentField().getType();
115+
if (fieldType instanceof GraphQLNonNull nonNullType) {
116+
fieldType = nonNullType.getWrappedType();
117+
}
118+
if (fieldType instanceof GraphQLNamedOutputType namedType) {
119+
return namedType.getName().endsWith("Connection");
120+
}
121+
}
122+
return false;
114123
}
115124

116125
private static boolean isConnectionEdgeNode(SelectedField selectedField) {

‎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\":[]," +

‎spring-graphql/src/test/java/org/springframework/graphql/data/query/PropertySelectionTests.java

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,19 @@
1616

1717
package org.springframework.graphql.data.query;
1818

19+
import java.nio.charset.StandardCharsets;
1920
import java.util.List;
2021
import java.util.concurrent.atomic.AtomicReference;
22+
import java.util.stream.Stream;
2123

2224
import graphql.schema.DataFetcher;
2325
import graphql.schema.DataFetchingFieldSelectionSet;
24-
import org.junit.jupiter.api.Test;
26+
import org.junit.jupiter.params.ParameterizedTest;
27+
import org.junit.jupiter.params.provider.Arguments;
28+
import org.junit.jupiter.params.provider.MethodSource;
2529

30+
import org.springframework.core.io.ByteArrayResource;
31+
import org.springframework.core.io.Resource;
2632
import org.springframework.data.util.TypeInformation;
2733
import org.springframework.graphql.BookSource;
2834
import org.springframework.graphql.GraphQlSetup;
@@ -34,19 +40,21 @@
3440
* Unit test for {@link PropertySelection}.
3541
*
3642
* @author Rossen Stoyanchev
43+
* @author Brian Clozel
3744
*/
3845
class PropertySelectionTests {
3946

40-
@Test
41-
void propertySelectionWithConnection() {
47+
@ParameterizedTest
48+
@MethodSource("schemaResource")
49+
void propertySelectionWithConnection(Resource schemaResource) {
4250

4351
AtomicReference<DataFetchingFieldSelectionSet> ref = new AtomicReference<>();
4452
DataFetcher<?> dataFetcher = environment -> {
4553
ref.set(environment.getSelectionSet());
4654
return null;
4755
};
4856

49-
GraphQlSetup.schemaResource(BookSource.paginationSchema)
57+
GraphQlSetup.schemaResource(schemaResource)
5058
.typeDefinitionConfigurer(new ConnectionTypeDefinitionConfigurer())
5159
.dataFetcher("Query", "books", dataFetcher)
5260
.toGraphQlService()
@@ -59,4 +67,20 @@ void propertySelectionWithConnection() {
5967
assertThat(list).containsExactly("id", "name");
6068
}
6169

70+
static Stream<Arguments> schemaResource() {
71+
return Stream.of(
72+
Arguments.of(BookSource.paginationSchema),
73+
Arguments.of(new ByteArrayResource("""
74+
type Query {
75+
books(first:Int, after:String): BookConnection!
76+
}
77+
78+
type Book {
79+
id: ID
80+
name: String
81+
}
82+
""".getBytes(StandardCharsets.UTF_8)))
83+
);
84+
}
85+
6286
}

0 commit comments

Comments
 (0)