Polishing in SchemaMappingInspector

See gh-934
This commit is contained in:
rstoyanchev
2024-03-26 07:51:43 +00:00
parent eb7bc43ce9
commit 1e6a5a09d7
2 changed files with 51 additions and 61 deletions

View File

@@ -223,7 +223,7 @@ xref:request-execution.adoc#execution.graphqlsource.schema-transformation[Schema
If a query, mutation, or subscription operation does not have a `DataFetcher`, it won't
return any data, and won't do anything useful. Likewise, fields of schema types that are
neither covered explicitly through a `DataFetcher` registration, nor implicitly by the
default `PropertyDataFetcher` that finds matching Java properties, will always be `null`.
default `PropertyDataFetcher` that finds matching `Class` properties, will always be `null`.
GraphQL Java does not perform checks to ensure every schema field is covered, and as a
lower level library, GraphQL Java simply does not know what a `DataFetcher` can return
@@ -239,7 +239,7 @@ are implementations of this interface. For annotated controllers, the return typ
expected arguments are based on the controller method signature. This makes it possible
to inspect schema mappings on startup to ensure the following:
- Schema fields have either a `DataFetcher` registration or a corresponding Java property.
- Schema fields have either a `DataFetcher` registration or a corresponding `Class` property.
- `DataFetcher` registrations refer to a schema field that exists.
- `DataFetcher` arguments have matching schema field arguments.
@@ -271,7 +271,7 @@ GraphQL schema inspection:
<3> `DataFetcher` expected arguments that don't exist
<4> Schema types that have been skipped (explained next)
In some cases, the Java type for a schema type is unknown. Maybe the `DataFetcher` does not
In some cases, the `Class` type for a schema type is unknown. Maybe the `DataFetcher` does not
implement `SelfDescribingDataFetcher`, or the declared return type is too general
(e.g. `Object`) or unknown (e.g. `List<?>`), or a `DataFetcher` could be missing altogether.
In such cases, the schema type is listed as skipped as it could not be verified. For every
@@ -282,11 +282,11 @@ skipped type, a DEBUG message explains why it was skipped.
==== Unions and Interfaces
For unions, the inspection iterates over member types and tries to find the corresponding
Java classes. For interfaces, the inspection iterates over implementation types and looks
for the corresponding Java classes.
classes. For interfaces, the inspection iterates over implementation types and looks
for the corresponding classes.
By default, corresponding Java class can be found if the class name matches that of the
GraphQL union member of interface implementation type, _and_ the Java class is located in
By default, corresponding `Class` can be found if the class name matches that of the
GraphQL union member of interface implementation type, _and_ the `Class` is located in
the same package (and/or outer class) as the return type of the controller method for the
union or interface. In addition, if `ClassNameTypeResolver` is configured as a
xref:request-execution.adoc#execution.graphqlsource.default-type-resolver[TypeResolver]
@@ -295,7 +295,7 @@ with explicit class mapping registrations, those are also checked.
If a union member or an interface implementation type is listed as skipped, you have
the following additional options:
- Register a function to resolve the Java class name for a given GraphQL type to account
- Register a function to resolve the `Class` name for a given GraphQL type to account
for class naming conventions.
- Register a `ClassResolver` with any custom resolution logic.

View File

@@ -59,28 +59,17 @@ import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
/**
* Declares an {@link #inspect(GraphQLSchema, RuntimeWiring)} method that checks
* if schema mappings.
* Inspect schema mappings on startup to ensure the following:
* <ul>
* <li>Schema fields have either a {@link DataFetcher} registration or a
* corresponding Class property.
* <li>{@code DataFetcher} registrations refer to a schema field that exists.
* <li>{@code DataFetcher} arguments have matching schema field arguments.
* </ul>
*
* <p>Schema mapping checks depend on {@code DataFetcher}s to be
* {@link SelfDescribingDataFetcher} in order to compare schema type and Java
* object type structure. If a {@code DataFetcher} does not implement this
* interface, then the Java type remains unknown, and the field type is reported
* as "skipped".
*
* <p>The {@code SelfDescribingDataFetcher} for an annotated controller method
* derives type information from the controller method signature. If the declared
* return type is {@link Object}, or an unspecified generic parameter such as
* {@code List<?>} then the Java type structure remains unknown, and the field
* output type is reported as skipped.
*
* <p>Unions are always skipped because there is no way for an annotated
* controller method to express that in a return type, and the Java type
* structure remains unknown.
*
* <p>Interfaces are supported only as far as fields declared directly on the
* interface, which are compared against properties of the Java type declared
* by a {@code SelfDescribingDataFetcher}.
* <p>Use methods of {@link GraphQlSource.SchemaResourceBuilder} to enable schema
* inspection on startup. For all other cases, use {@link #initializer()} as a
* starting point or the shortcut {@link #inspect(GraphQLSchema, Map)}.
*
* @author Brian Clozel
* @author Rossen Stoyanchev
@@ -183,16 +172,27 @@ public class SchemaMappingInspector {
}
}
private void checkFieldArguments(GraphQLFieldDefinition field, SelfDescribingDataFetcher<?> dataFetcher) {
List<String> arguments = new ArrayList<>();
for (String name : dataFetcher.getArguments().keySet()) {
if (field.getArgument(name) == null) {
arguments.add(name);
}
}
if (!arguments.isEmpty()) {
this.reportBuilder.unmappedArgument(dataFetcher, arguments);
}
}
/**
* Resolve the field type and its associated Java type, and recurse with
* {@link #checkFieldsContainer} if there is enough type information.
* Resolve field wrapper types (connection, list, non-null), nest into generic types,
* and recurse with {@link #checkFieldsContainer} if there is enough type information.
*/
private void checkField(
GraphQLFieldsContainer parent, GraphQLFieldDefinition field, ResolvableType resolvableType) {
TypePair typePair = TypePair.resolveTypePair(parent, field, resolvableType, schema);
// Type already inspected?
if (addAndCheckIfAlreadyInspected(typePair.outputType())) {
return;
}
@@ -210,21 +210,23 @@ public class SchemaMappingInspector {
}
for (Map.Entry<GraphQLType, List<ResolvableType>> entry : typePairs.entrySet()) {
GraphQLType graphQlType = entry.getKey();
for (ResolvableType currentResolvableType : entry.getValue()) {
// Can we inspect GraphQL type?
if (!(entry.getKey() instanceof GraphQLFieldsContainer fieldContainer)) {
if (isNotScalarOrEnumType(entry.getKey())) {
if (!(graphQlType instanceof GraphQLFieldsContainer fieldContainer)) {
if (isNotScalarOrEnumType(graphQlType)) {
FieldCoordinates coordinates = FieldCoordinates.coordinates(parent.getName(), field.getName());
addSkippedType(entry.getKey(), coordinates, "Unsupported schema type");
addSkippedType(graphQlType, coordinates, "Unsupported schema type");
}
continue;
}
// Can we inspect Java type?
// Can we inspect the Class?
if (currentResolvableType.resolve(Object.class) == Object.class) {
FieldCoordinates coordinates = FieldCoordinates.coordinates(parent.getName(), field.getName());
addSkippedType(entry.getKey(), coordinates, "No Java type information");
addSkippedType(graphQlType, coordinates, "No class information");
continue;
}
@@ -233,22 +235,18 @@ public class SchemaMappingInspector {
}
}
private void checkFieldArguments(GraphQLFieldDefinition field, SelfDescribingDataFetcher<?> dataFetcher) {
List<String> arguments = new ArrayList<>();
for (String name : dataFetcher.getArguments().keySet()) {
if (field.getArgument(name) == null) {
arguments.add(name);
}
@Nullable
private PropertyDescriptor getProperty(ResolvableType resolvableType, String fieldName) {
try {
Class<?> clazz = resolvableType.resolve();
return (clazz != null ? BeanUtils.getPropertyDescriptor(clazz, fieldName) : null);
}
if (!arguments.isEmpty()) {
this.reportBuilder.unmappedArgument(dataFetcher, arguments);
catch (BeansException ex) {
throw new IllegalStateException(
"Failed to get property on " + resolvableType + " for field '" + fieldName + "'", ex);
}
}
private static String typeNameToString(GraphQLType type) {
return (type instanceof GraphQLNamedType namedType ? namedType.getName() : type.toString());
}
private boolean addAndCheckIfAlreadyInspected(GraphQLType type) {
return (type instanceof GraphQLNamedOutputType outputType && !this.inspectedTypes.add(outputType.getName()));
}
@@ -257,18 +255,6 @@ public class SchemaMappingInspector {
return !(type instanceof GraphQLScalarType || type instanceof GraphQLEnumType);
}
@Nullable
private PropertyDescriptor getProperty(ResolvableType resolvableType, String fieldName) {
try {
Class<?> clazz = resolvableType.resolve(Object.class);
return BeanUtils.getPropertyDescriptor(clazz, fieldName);
}
catch (BeansException ex) {
throw new IllegalStateException(
"Failed to introspect " + resolvableType + " for field '" + fieldName + "'", ex);
}
}
private void addSkippedType(GraphQLType type, FieldCoordinates coordinates, String reason) {
String typeName = typeNameToString(type);
this.reportBuilder.skippedType(type, coordinates);
@@ -277,6 +263,10 @@ public class SchemaMappingInspector {
}
}
private static String typeNameToString(GraphQLType type) {
return (type instanceof GraphQLNamedType namedType ? namedType.getName() : type.toString());
}
private void checkDataFetcherRegistrations() {
this.dataFetchers.forEach((typeName, registrations) ->
registrations.forEach((fieldName, dataFetcher) -> {