Skip to content

Commit 0109d7b

Browse files
committed
Skip nullness check for Map-like return types of @BatchMapping methods
Prior to this commit, the schema inspector would perform nullness checks for fields and data fetchers. `@BatchMapping` methods can return `Map<Book, Author>` types and such cases can be raised as inconsistent with the schema declaration. This check is invalid because we cannot infer the nullness of the actual schema field given a `Map` fetched using a `DataLoader`. This commit ensures that such cases are skipped. Closes gh-1334
1 parent 15a7412 commit 0109d7b

File tree

2 files changed

+52
-2
lines changed

2 files changed

+52
-2
lines changed

spring-graphql/src/main/java/org/springframework/graphql/execution/SchemaMappingInspector.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,10 +235,17 @@ private void checkDataFetcherNullness(FieldCoordinates fieldCoordinates, SelfDes
235235
Method dataFetcherMethod = dataFetcher.asMethod();
236236
if (dataFetcherMethod != null) {
237237
Nullness applicationNullness = Nullness.forMethodReturnType(dataFetcherMethod);
238-
// we cannot infer nullness if wrapped by reactive types
239238
ReactiveAdapter reactiveAdapter = ReactiveAdapterRegistry.getSharedInstance()
240239
.getAdapter(dataFetcherMethod.getReturnType());
241-
if (reactiveAdapter == null && isMismatch(schemaNullness, applicationNullness)) {
240+
if (reactiveAdapter != null) {
241+
// we cannot infer nullness if wrapped by reactive types
242+
logger.debug("Skip nullness check for data fetcher '" + dataFetcherMethod.getName() + "' because of Reactive return type.");
243+
}
244+
else if (dataFetcher.usesDataLoader() && Map.class.isAssignableFrom(dataFetcherMethod.getReturnType())) {
245+
// we cannot infer nullness if batch loader method returns a Map
246+
logger.debug("Skip nullness check for data fetcher '" + dataFetcherMethod.getName() + "' because of batch loading.");
247+
}
248+
else if (isMismatch(schemaNullness, applicationNullness)) {
242249
DescribedAnnotatedElement annotatedElement = new DescribedAnnotatedElement(dataFetcherMethod, dataFetcher.getDescription());
243250
this.reportBuilder.fieldNullnessMismatch(fieldCoordinates,
244251
new DefaultNullnessMismatch(schemaNullness, applicationNullness, annotatedElement));

spring-graphql/src/test/java/org/springframework/graphql/execution/SchemaMappingInspectorNullnessTests.java

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package org.springframework.graphql.execution;
1818

19+
import java.util.List;
20+
import java.util.Map;
1921
import java.util.concurrent.CompletableFuture;
2022

2123
import graphql.Scalars;
@@ -36,6 +38,7 @@
3638
import org.junit.jupiter.api.Nested;
3739
import org.junit.jupiter.api.Test;
3840

41+
import org.springframework.graphql.data.method.annotation.BatchMapping;
3942
import org.springframework.graphql.data.method.annotation.QueryMapping;
4043
import org.springframework.stereotype.Controller;
4144

@@ -360,6 +363,27 @@ void reportIsEmptyWhenSchemaFieldNonNullAndAsyncTypeNullable() {
360363
assertThatReport(report).isEmpty();
361364
}
362365

366+
@Test
367+
void reportIsEmptyWhenBatchMapping() {
368+
String schema = """
369+
type Query {
370+
bookById(id: ID!): Book
371+
}
372+
type Book {
373+
id: ID!
374+
title: String!
375+
author: Author
376+
}
377+
type Author {
378+
name: String!
379+
}
380+
""";
381+
SchemaReport report = inspectSchema(schema, BatchMappingBookController.class);
382+
assertThatReport(report).isEmpty();
383+
}
384+
385+
386+
363387
@Test
364388
void doesNotFailWhenNullFieldDefinitionType() {
365389
String schemaContent = """
@@ -422,10 +446,29 @@ static class NullableAsyncBookController {
422446

423447
}
424448

449+
@Controller
450+
@NullMarked
451+
static class BatchMappingBookController {
452+
453+
@QueryMapping
454+
public @Nullable Book bookById(String id) {
455+
return new Book("42", "Spring for GraphQL Book");
456+
}
457+
458+
@BatchMapping
459+
public Map<Book, Author> author(List<Book> books) {
460+
return Map.of();
461+
}
462+
}
463+
425464
record Book(String id, String title) {
426465

427466
}
428467

468+
record Author(String name) {
469+
470+
}
471+
429472
static class NullFieldTypeVisitor extends GraphQLTypeVisitorStub {
430473
@Override
431474
public TraversalControl visitGraphQLFieldDefinition(GraphQLFieldDefinition node, TraverserContext<GraphQLSchemaElement> context) {

0 commit comments

Comments
 (0)