Spring/spring-graphql
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Schema inspection support for unmapped arguments

Browse Source 
Closes: gh-740
This commit is contained in:
rstoyanchev
2024-03-15 10:01:18 +00:00
parent dcaa1511a1
commit 267b514cf2
6 changed files with 166 additions and 62 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
spring-graphql-docs/modules/ROOT/pages/request-execution.adoc
View File

@@ -229,24 +229,25 @@ matching Java object property, will always be `null`.
GraphQL Java does not perform checks to ensure every schema field is covered, and that
can result in gaps that might not be discovered depending on test coverage. At runtime
you may get a "silent" `null`, or an error if the field is not nullable. As a lower level
library, GraphQL Java simply does not know enough about `DataFetcher` implementations and
their return types, and therefore can't compare schema type structure against Java object
structure.
library, GraphQL Java simply does not have enough information about `DataFetcher`
implementations to know their return types or what arguments they depend on, and as a result
cannot perform such verifications.
Spring for GraphQL defines the `SelfDescribingDataFetcher` interface to allow a
`DataFetcher` to expose return type information. All Spring `DataFetcher` implementations
`DataFetcher` to expose information about itself. All Spring `DataFetcher` implementations
implement this interface. That includes those for xref:controllers.adoc[Annotated Controllers], and those for
xref:data.adoc#data.querydsl[Querydsl] and xref:data.adoc#data.querybyexample[Query by Example] Spring Data repositories. For annotated
controllers, the return type is derived from the declared return type on a
`@SchemaMapping` method.
xref:data.adoc#data.querydsl[Querydsl] and
xref:data.adoc#data.querybyexample[Query by Example] Spring Data repositories.
For annotated controllers, the return type is derived from the declared return type on
a `@SchemaMapping` method, while arguments are dervied from `@Argument` method parameters.
On startup, Spring for GraphQL can inspect schema fields, `DataFetcher` registrations,
and the properties of Java objects returned from `DataFetcher` implementations to check
if all schema fields are covered either by an explicitly registered `DataFetcher`, or
a matching Java object property. The inspection also performs a reverse check looking for
`DataFetcher` registrations against schema fields that don't exist.
Spring for GraphQL can perform an inspection on startup to ensure the following:
To enable inspection of schema mappings:
- Schema fields have a `DataFetcher` registration or a corresponding Java property.
- `DataFetcher` registrations refer to a schema field that does exist.
- `DataFetcher` refers to schema arguments that exist.
You can enable the inspection and take an appropriate action as follows:
[source,java,indent=0,subs="verbatim,quotes"]
----
@@ -264,14 +265,16 @@ Below is an example report:
GraphQL schema inspection:
Unmapped fields: {Book=[title], Author[firstName, lastName]} // <1>
Unmapped registrations: {Book.reviews=BookController#reviews[1 args]} <2>
Skipped types: [BookOrAuthor] // <3>
Unmapped arguments: {BookController#bookSearch[1 args]=[myAuthor]} // <3>
Skipped types: [BookOrAuthor] // <4>
----
<1> List of schema fields and their source types that are not mapped
<2> List of `DataFetcher` registrations on fields that don't exist
<3> List of schema types that are skipped, as explained next
<1> Coordinates of schema fields that are not covered
<2> ``DataFetcher`` registered mapped to fields that don't exist
<3> `DataFetcher` arguments that don't exist
<4> Schema types that have been skipped (explained next)
There are limits to what schema field inspection can do, in particular when there is
There are limits to what schema inspection can do, in particular when there is
insufficient Java type information. This is the case if an annotated controller method is
declared to return `java.lang.Object`, or if the return type has an unspecified generic
parameter such as `List<?>`, or if the `DataFetcher` does not implement

22
spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java
View File

@@ -29,6 +29,8 @@ import java.util.concurrent.CompletableFuture;
import java.util.concurrent.Executor;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import graphql.execution.DataFetcherResult;
import graphql.schema.DataFetcher;
@@ -45,15 +47,19 @@ import reactor.core.publisher.Mono;
import org.springframework.beans.factory.NoSuchBeanDefinitionException;
import org.springframework.context.ApplicationContext;
import org.springframework.context.expression.BeanFactoryResolver;
import org.springframework.core.DefaultParameterNameDiscoverer;
import org.springframework.core.KotlinDetector;
import org.springframework.core.MethodParameter;
import org.springframework.core.ParameterNameDiscoverer;
import org.springframework.core.ResolvableType;
import org.springframework.core.annotation.AnnotatedElementUtils;
import org.springframework.data.domain.ScrollPosition;
import org.springframework.graphql.data.ArgumentValue;
import org.springframework.graphql.data.GraphQlArgumentBinder;
import org.springframework.graphql.data.method.HandlerMethod;
import org.springframework.graphql.data.method.HandlerMethodArgumentResolver;
import org.springframework.graphql.data.method.HandlerMethodArgumentResolverComposite;
import org.springframework.graphql.data.method.annotation.Argument;
import org.springframework.graphql.data.method.annotation.BatchMapping;
import org.springframework.graphql.data.method.annotation.SchemaMapping;
import org.springframework.graphql.data.pagination.CursorStrategy;
@@ -380,6 +386,8 @@ public class AnnotatedControllerConfigurer
*/
static class SchemaMappingDataFetcher implements SelfDescribingDataFetcher<Object> {
private static final ParameterNameDiscoverer parameterNameDiscoverer = new DefaultParameterNameDiscoverer();
private final DataFetcherMappingInfo mappingInfo;
private final HandlerMethodArgumentResolverComposite argumentResolvers;
@@ -424,6 +432,20 @@ public class AnnotatedControllerConfigurer
return ResolvableType.forMethodReturnType(this.mappingInfo.getHandlerMethod().getMethod());
}
@Override
public Map<String, ResolvableType> getArguments() {
Predicate<MethodParameter> argumentPredicate = p ->
(p.getParameterAnnotation(Argument.class) != null || p.getParameterType() == ArgumentValue.class);
return Arrays.stream(this.mappingInfo.getHandlerMethod().getMethodParameters())
.filter(argumentPredicate)
.peek(p -> p.initParameterNameDiscovery(parameterNameDiscoverer))
.collect(Collectors.toMap(
ArgumentMethodArgumentResolver::getArgumentName,
ResolvableType::forMethodParameter));
}
/**
* Return the {@link HandlerMethod} used to fetch data.
*/

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

@@ -49,6 +49,7 @@ import org.springframework.core.ReactiveAdapterRegistry;
import org.springframework.core.ResolvableType;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.CollectionUtils;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
@@ -135,13 +136,13 @@ public class SchemaMappingInspector {
}
/**
* Check the given {@code GraphQLFieldsContainer} against {@code DataFetcher}
* registrations, or Java properties of the given {@code ResolvableType}.
* @param fieldContainer the GraphQL interface or object type to check
* @param resolvableType the Java type to match against, or {@code null} if
* not applicable such as for Query, Mutation, or Subscription
* Check fields of the given {@code GraphQLFieldsContainer} to make sure there
* is either a {@code DataFetcher} registration, or a corresponding property
* in the given Java type, which may be {@code null} for the top-level types
* Query, Mutation, and Subscription.
*/
private void checkFieldsContainer(GraphQLFieldsContainer fieldContainer, @Nullable ResolvableType resolvableType) {
private void checkFieldsContainer(
GraphQLFieldsContainer fieldContainer, @Nullable ResolvableType resolvableType) {
String typeName = fieldContainer.getName();
Map<String, DataFetcher> dataFetcherMap = this.dataFetchers.getOrDefault(typeName, Collections.emptyMap());
@@ -159,16 +160,21 @@ public class SchemaMappingInspector {
}
/**
* Check the output {@link GraphQLType} of a field against the given DataFetcher return type.
* @param parent the parent of the field
* @param field the field to inspect
* @param dataFetcher the registered DataFetcher
* Perform the following:
* <ul>
* <li>Resolve the field type and the {@code DataFetcher} return type, and recurse
* with {@link #checkFieldsContainer} if there is sufficient type information.
* <li>Resolve the arguments the {@code DataFetcher} depends on and check they
* are defined in the schema.
* </ul>
*/
private void checkField(GraphQLFieldsContainer parent, GraphQLFieldDefinition field, DataFetcher<?> dataFetcher) {
private void checkField(
GraphQLFieldsContainer parent, GraphQLFieldDefinition field, DataFetcher<?> dataFetcher) {
ResolvableType resolvableType = ResolvableType.NONE;
if (dataFetcher instanceof SelfDescribingDataFetcher<?> selfDescribingDataFetcher) {
resolvableType = selfDescribingDataFetcher.getReturnType();
if (dataFetcher instanceof SelfDescribingDataFetcher<?> selfDescribing) {
resolvableType = selfDescribing.getReturnType();
checkFieldArguments(field, selfDescribing);
}
// Remove GraphQL type wrappers, and nest within Java generic types
@@ -210,6 +216,17 @@ public class SchemaMappingInspector {
checkFieldsContainer(fieldContainer, resolvableType);
}
private void checkFieldArguments(GraphQLFieldDefinition field, SelfDescribingDataFetcher<?> dataFetcher) {
List<String> arguments = dataFetcher.getArguments().keySet().stream()
.filter(name -> field.getArgument(name) == null)
.toList();
if (!arguments.isEmpty()) {
this.reportBuilder.unmappedArgument(dataFetcher, arguments);
}
}
private GraphQLType unwrapIfNonNull(GraphQLType type) {
return (type instanceof GraphQLNonNull graphQLNonNull ? graphQLNonNull.getWrappedType() : type);
}
@@ -347,6 +364,8 @@ public class SchemaMappingInspector {
private final Map<FieldCoordinates, DataFetcher<?>> unmappedRegistrations = new LinkedHashMap<>();
private final MultiValueMap<DataFetcher<?>, String> unmappedArguments = new LinkedMultiValueMap<>();
private final List<SchemaReport.SkippedType> skippedTypes = new ArrayList<>();
public void unmappedField(FieldCoordinates coordinates) {
@@ -357,12 +376,17 @@ public class SchemaMappingInspector {
this.unmappedRegistrations.put(coordinates, dataFetcher);
}
public void unmappedArgument(DataFetcher<?> dataFetcher, List<String> arguments) {
this.unmappedArguments.put(dataFetcher, arguments);
}
public void skippedType(GraphQLType type, FieldCoordinates coordinates) {
this.skippedTypes.add(new DefaultSkippedType(type, coordinates));
}
public SchemaReport build() {
return new DefaultSchemaReport(this.unmappedFields, this.unmappedRegistrations, this.skippedTypes);
return new DefaultSchemaReport(
this.unmappedFields, this.unmappedRegistrations, this.unmappedArguments, this.skippedTypes);
}
}
@@ -377,14 +401,17 @@ public class SchemaMappingInspector {
private final Map<FieldCoordinates, DataFetcher<?>> unmappedRegistrations;
private final MultiValueMap<DataFetcher<?>, String> unmappedArguments;
private final List<SchemaReport.SkippedType> skippedTypes;
public DefaultSchemaReport(
List<FieldCoordinates> unmappedFields, Map<FieldCoordinates, DataFetcher<?>> unmappedRegistrations,
List<SkippedType> skippedTypes) {
MultiValueMap<DataFetcher<?>, String> unmappedArguments, List<SkippedType> skippedTypes) {
this.unmappedFields = Collections.unmodifiableList(unmappedFields);
this.unmappedRegistrations = Collections.unmodifiableMap(unmappedRegistrations);
this.unmappedArguments = CollectionUtils.unmodifiableMultiValueMap(unmappedArguments);
this.skippedTypes = Collections.unmodifiableList(skippedTypes);
}
@@ -398,6 +425,11 @@ public class SchemaMappingInspector {
return this.unmappedRegistrations;
}
@Override
public MultiValueMap<DataFetcher<?>, String> unmappedArguments() {
return this.unmappedArguments;
}
@Override
public List<SkippedType> skippedTypes() {
return this.skippedTypes;
@@ -421,6 +453,7 @@ public class SchemaMappingInspector {
return "GraphQL schema inspection:\n" +
"\tUnmapped fields: " + formatUnmappedFields() + "\n" +
"\tUnmapped registrations: " + this.unmappedRegistrations + "\n" +
"\tUnmapped arguments: " + this.unmappedArguments + "\n" +
"\tSkipped types: " + this.skippedTypes;
}

10
spring-graphql/src/main/java/org/springframework/graphql/execution/SchemaReport.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2020-2023 the original author or authors.
* Copyright 2020-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -27,6 +27,7 @@ import graphql.schema.GraphQLSchema;
import graphql.schema.GraphQLType;
import org.springframework.lang.Nullable;
import org.springframework.util.MultiValueMap;
/**
* Report produced as a result of inspecting schema mappings.
@@ -61,6 +62,13 @@ public interface SchemaReport {
*/
Map<FieldCoordinates, DataFetcher<?>> unmappedRegistrations();
/**
* Return a map with {@link DataFetcher}s and the names of arguments they
* depend on that don't exist.
* @since 1.3
*/
MultiValueMap<DataFetcher<?>, String> unmappedArguments();
/**
* Return types skipped during the inspection, either because the schema type
* is not supported, e.g. union, or because there is insufficient Java type

14
spring-graphql/src/main/java/org/springframework/graphql/execution/SelfDescribingDataFetcher.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2020-2023 the original author or authors.
* Copyright 2020-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -16,6 +16,9 @@
package org.springframework.graphql.execution;
import java.util.Collections;
import java.util.Map;
import graphql.schema.DataFetcher;
import org.springframework.core.ResolvableType;
@@ -46,4 +49,13 @@ public interface SelfDescribingDataFetcher<T> extends DataFetcher<T> {
*/
ResolvableType getReturnType();
/**
* Return a map with arguments that this {@link DataFetcher} looks up
* along with the Java types they are mapped to.
* @since 1.3
*/
default Map<String, ResolvableType> getArguments() {
return Collections.emptyMap();
}
}

80
spring-graphql/src/test/java/org/springframework/graphql/execution/SchemaMappingInspectorTests.java
View File

@@ -235,7 +235,7 @@ class SchemaMappingInspectorTests {
class SubscriptionTests {
@Test
void reportContainsUnmappedSubscription() {
void unmappedSubscription() {
String schema = """
type Query{
greeting: String
@@ -254,26 +254,7 @@ class SchemaMappingInspectorTests {
}
@Test
void reportIsEmptyWhenSubscriptionIsMapped() {
String schema = """
type Query{
greeting: String
}
type Subscription {
bookSearch(author: String) : [Book!]!
}
type Book {
id: ID
name: String
}
""";
SchemaReport report = inspectSchema(schema, GreetingController.class, BookController.class);
assertThatReport(report).hasUnmappedFieldCount(0).hasSkippedTypeCount(0);
}
@Test
void reportWorksForSubscriptionWithExtensionType() {
void unmappedSubscriptionWithExtensionType() {
String schema = """
type Query{
greeting: String
@@ -292,6 +273,26 @@ class SchemaMappingInspectorTests {
assertThatReport(report).hasUnmappedFieldCount(1).containsUnmappedFields("Subscription", "bookSearch");
}
@Test
void mappedSubscriptionWithUnmappedArgument() {
String schema = """
type Query{
greeting: String
}
type Subscription {
bookSearch(author: String) : [Book!]!
}
type Book {
id: ID
name: String
}
""";
SchemaReport report = inspectSchema(schema, GreetingController.class, BookController.class);
assertThatReport(report).hasUnmappedFieldCount(0).hasSkippedTypeCount(0);
assertThatReport(report).hasUnmappedArgumentCount(1).containsUnmappedArguments("myAuthor");
}
}
@@ -552,11 +553,16 @@ class SchemaMappingInspectorTests {
""";
SchemaReport report = inspectSchema(schema, BookController.class);
assertThatReport(report).hasUnmappedFieldCount(1).hasSkippedTypeCount(0);
assertThat(report.toString())
.contains("GraphQL schema inspection:", "Unmapped fields: {Book=[missing]}", "Unmapped registrations:",
"Book.fetcher=BookController#fetcher[1 args]", "Query.paginatedBooks=BookController#paginatedBooks[0 args]",
"Query.bookObject=BookController#bookObject[1 args]", "Query.bookById=BookController#bookById[1 args]",
"Skipped types: []");
assertThat(report.toString()).contains(
"GraphQL schema inspection:",
"Unmapped fields: {Book=[missing]}",
"Unmapped registrations:",
"Book.fetcher=BookController#fetcher[1 args]",
"Query.paginatedBooks=BookController#paginatedBooks[0 args]",
"Query.bookObject=BookController#bookObject[1 args]",
"Query.bookById=BookController#bookById[1 args]",
"{BookController#bookSearch[1 args]=[myAuthor]}",
"Skipped types: []");
}
}
@@ -649,7 +655,7 @@ class SchemaMappingInspectorTests {
}
@SubscriptionMapping
public Flux<List<Book>> bookSearch(@Argument String author) {
public Flux<List<Book>> bookSearch(@Argument String myAuthor) {
return Flux.empty();
}
}
@@ -746,6 +752,14 @@ class SchemaMappingInspectorTests {
return this;
}
public SchemaInspectionReportAssert hasUnmappedArgumentCount(int expected) {
isNotNull();
if (this.actual.unmappedArguments().size() != expected) {
failWithMessage("Expected %s unmapped arguments, found %s.", expected, this.actual.unmappedArguments());
}
return this;
}
public SchemaInspectionReportAssert hasSkippedTypeCount(int expected) {
isNotNull();
if (this.actual.skippedTypes().size() != expected) {
@@ -778,6 +792,18 @@ class SchemaMappingInspectorTests {
return this;
}
public SchemaInspectionReportAssert containsUnmappedArguments(String... arguments) {
isNotNull();
List<String> expected = Arrays.asList(arguments);
List<String> actual = this.actual.unmappedArguments().entrySet().stream()
.flatMap(entry -> entry.getValue().stream())
.toList();
if (!actual.containsAll(expected)) {
failWithMessage("Expected unmapped arguments: %s, found %s", expected, actual);
}
return this;
}
public SchemaInspectionReportAssert containsSkippedTypes(String... fieldCoordinates) {
isNotNull();
List<String> expected = Arrays.asList(fieldCoordinates);