From 469d64bd9341e273614d592857fcee4125555eef Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Mon, 17 Apr 2023 12:26:02 +0100 Subject: [PATCH] SchemaMappingInspector enhancements Support not only GraphQLObjectType, but any GraphQLFieldsContainer, effectively also inspecting fields declared on an interface. Add skipped types to report that includes any non-simple types that have been skipped. Any type that is not a GraphQLFieldsContainer is skipped, including unions. Types may also be skipped because of insufficient Java object type information, e.g. controller method declared to return Object or Mono. Do not report issues related to scalar and enum type fields, which don't have any structure and don't need further checks. See gh-662 --- ...ultSchemaResourceGraphQlSourceBuilder.java | 4 +- .../execution/SchemaMappingInspector.java | 297 +++++++++-------- .../execution/SelfDescribingDataFetcher.java | 2 +- .../SchemaMappingInspectorTests.java | 307 ++++++++++++------ 4 files changed, 363 insertions(+), 247 deletions(-) diff --git a/spring-graphql/src/main/java/org/springframework/graphql/execution/DefaultSchemaResourceGraphQlSourceBuilder.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/DefaultSchemaResourceGraphQlSourceBuilder.java index e078995d..57a2689b 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/execution/DefaultSchemaResourceGraphQlSourceBuilder.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/execution/DefaultSchemaResourceGraphQlSourceBuilder.java @@ -136,9 +136,7 @@ final class DefaultSchemaResourceGraphQlSourceBuilder configureGraphQl(builder -> { GraphQLSchema schema = builder.build().getGraphQLSchema(); SchemaMappingInspector.Report report = SchemaMappingInspector.inspect(schema, runtimeWiring); - if (!report.isEmpty()) { - logger.info(report.getSummary()); - } + logger.info(report); }); return (this.schemaFactory != null ? diff --git a/spring-graphql/src/main/java/org/springframework/graphql/execution/SchemaMappingInspector.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/SchemaMappingInspector.java index 33384ce7..ed753fba 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/execution/SchemaMappingInspector.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/execution/SchemaMappingInspector.java @@ -17,23 +17,23 @@ package org.springframework.graphql.execution; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; -import java.util.stream.Stream; import graphql.schema.DataFetcher; +import graphql.schema.GraphQLEnumType; import graphql.schema.GraphQLFieldDefinition; +import graphql.schema.GraphQLFieldsContainer; import graphql.schema.GraphQLList; +import graphql.schema.GraphQLNamedOutputType; import graphql.schema.GraphQLNamedType; import graphql.schema.GraphQLNonNull; import graphql.schema.GraphQLObjectType; +import graphql.schema.GraphQLScalarType; import graphql.schema.GraphQLSchema; import graphql.schema.GraphQLType; import graphql.schema.idl.RuntimeWiring; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; -import org.jetbrains.annotations.NotNull; import org.springframework.beans.BeanUtils; import org.springframework.beans.BeansException; @@ -41,20 +41,29 @@ import org.springframework.core.ReactiveAdapter; import org.springframework.core.ReactiveAdapterRegistry; import org.springframework.core.ResolvableType; import org.springframework.lang.Nullable; +import org.springframework.util.Assert; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; /** - * Inspect the GraphQL schema and look for inconsistencies with declared {@code @SchemaMapping} handlers and {@link DataFetcher}. - * The inspector will produce a {@link Report}, its content can be used for logging purposes. - *

This inspection utility will report to developers: - *

- *

This approach has several known limitations; the corresponding Java types are only discovered through registered - * {@code DataFetcher} instances, if they implement the {@link SelfDescribingDataFetcher} contract. Union types are not supported, - * even if a common interface is declared by a {@link SelfDescribingDataFetcher}. + * Provide {@link #inspect(GraphQLSchema, RuntimeWiring)} method that checks if + * schema fields are covered by either a {@link DataFetcher} registration, or a + * Java object property. Fields that have neither are reported as unmapped in + * the output {@link Report}. + * + *

The inspection depends on {@code DataFetcher}s to expose return type + * information by implementing {@link SelfDescribingDataFetcher}. This allows + * checking if Java object types have properties that match schema fields. + * If a {@code DataFetcher} does not implement this interface, then the Java + * object type is not known, and the field type is reported as skipped. + * + *

The {@link SelfDescribingDataFetcher} for annotated controller methods + * exposes the declared return type of the controller method. If the return type + * is {@link Object} such as for a union, then the Java object structure is + * not known, and the field output type is reported as skipped. + * + *

Union types are automatically skipped because there is no way for an + * annotated controller method to declare the actual Java types. * * @author Brian Clozel * @author Rossen Stoyanchev @@ -62,196 +71,202 @@ import org.springframework.util.MultiValueMap; */ class SchemaMappingInspector { - private static final Log logger = LogFactory.getLog(SchemaMappingInspector.class); - - private final GraphQLSchema schema; private final RuntimeWiring runtimeWiring; + private final ReportBuilder reportBuilder = new ReportBuilder(); + private final Set seenTypes = new HashSet<>(); + private final ReactiveAdapterRegistry reactiveAdapterRegistry = ReactiveAdapterRegistry.getSharedInstance(); + private SchemaMappingInspector(GraphQLSchema schema, RuntimeWiring runtimeWiring) { + Assert.notNull(schema, "GraphQLSchema is required"); + Assert.notNull(runtimeWiring, "RuntimeWiring is required"); this.schema = schema; this.runtimeWiring = runtimeWiring; } - public Report inspectMappings() { - ReportBuilder report = ReportBuilder.create(); - inspectOperation(schema.getQueryType(), report); - inspectOperation(schema.getMutationType(), report); - inspectOperation(schema.getSubscriptionType(), report); - return report.build(); - } + /** + * Inspect all fields, starting from Query, Mutation, and Subscription, and + * working recursively down through the types they return. + * @return a report with unmapped fields and skipped types. + */ + public Report inspect() { + inspectType(this.schema.getQueryType(), null); + + if (this.schema.isSupportingMutations()) { + inspectType(this.schema.getMutationType(), null); + } + + if (this.schema.isSupportingSubscriptions()) { + inspectType(this.schema.getSubscriptionType(), null); + } + + return this.reportBuilder.build(); + } @SuppressWarnings("rawtypes") - private void inspectOperation(@Nullable GraphQLObjectType operationType, ReportBuilder report) { - if (operationType != null) { - Map operationDataFetchers = this.runtimeWiring.getDataFetcherForType(operationType.getName()); - for (GraphQLFieldDefinition fieldDefinition : operationType.getFieldDefinitions()) { - if (operationDataFetchers.containsKey(fieldDefinition.getName())) { - DataFetcher fieldDataFetcher = operationDataFetchers.get(fieldDefinition.getName()); - if (fieldDataFetcher instanceof SelfDescribingDataFetcher selfDescribingDataFetcher) { - inspectType(fieldDefinition.getType(), selfDescribingDataFetcher.getReturnType(), report); - } - } - else { - report.missingOperation(operationType, fieldDefinition); - } - } - } - } + private void inspectType(GraphQLType type, @Nullable ResolvableType resolvableType) { + Assert.notNull(type, "No GraphQLType"); - private void inspectType(GraphQLType type, ResolvableType declaredType, ReportBuilder report) { - if (type instanceof GraphQLObjectType objectType) { - inspectObjectType(objectType, declaredType, report); + type = unwrapNonNull(type); + if (isConnectionType(type)) { + type = getConnectionNodeType(type); + resolvableType = nest(resolvableType, type); } else if (type instanceof GraphQLList listType) { - inspectType(listType.getWrappedType(), declaredType.getNested(2), report); + type = unwrapNonNull(listType.getWrappedType()); + resolvableType = nest(resolvableType, type); } - else if (type instanceof GraphQLNonNull nonNullType) { - inspectType(nonNullType.getWrappedType(), declaredType, report); - } - else if (type instanceof GraphQLNamedType namedType && logger.isTraceEnabled()){ - logger.trace("Cannot inspect type '" + namedType.getName() + "', inspector does not support " - + type.getClass().getSimpleName()); - } - } - @SuppressWarnings("rawtypes") - private void inspectObjectType(GraphQLObjectType objectType, ResolvableType declaredType, ReportBuilder report) { - if (isTypeAlreadyInspected(objectType)) { + if (type instanceof GraphQLNamedOutputType outputType) { + if (!this.seenTypes.add(outputType.getName())) { + return; + } + } + + if (!(type instanceof GraphQLFieldsContainer fieldContainer)) { + if (isNotScalarOrEnumType(type)) { + this.reportBuilder.addSkippedType(getTypeName(type)); + } return; } - if (isConnectionType(objectType)) { - objectType = getEdgeNodeType(objectType); - declaredType = declaredType.getNested(2); + else if (resolvableType != null && resolveClassToCompare(resolvableType) == Object.class) { + this.reportBuilder.addSkippedType(getTypeName(type)); + return; } - Map typeDataFetcher = this.runtimeWiring.getDataFetcherForType(objectType.getName()); - Class declaredClass = unwrapPublisherTypes(declaredType); - for (GraphQLFieldDefinition field : objectType.getFieldDefinitions()) { - if (typeDataFetcher.containsKey(field.getName())) { - DataFetcher fieldDataFetcher = typeDataFetcher.get(field.getName()); - if (fieldDataFetcher instanceof SelfDescribingDataFetcher typedFieldDataFetcher) { - inspectType(field.getType(), typedFieldDataFetcher.getReturnType(), report); + + String typeName = fieldContainer.getName(); + Map dataFetcherMap = this.runtimeWiring.getDataFetcherForType(typeName); + + for (GraphQLFieldDefinition field : fieldContainer.getFieldDefinitions()) { + String fieldName = field.getName(); + if (dataFetcherMap.containsKey(fieldName)) { + DataFetcher fetcher = dataFetcherMap.get(fieldName); + if (fetcher instanceof SelfDescribingDataFetcher selfDescribingDataFetcher) { + inspectType(field.getType(), selfDescribingDataFetcher.getReturnType()); + } + else if (isNotScalarOrEnumType(field.getType())) { + this.reportBuilder.addSkippedType(getTypeName(field.getType())); } } - else { - try { - if (declaredClass == null || BeanUtils.getPropertyDescriptor(declaredClass, field.getName()) == null) { - report.missingField(objectType, field); - } - } - catch (BeansException exc) { - logger.debug("Failed while inspecting " + declaredType + " for property " + field.getName() + "", exc); - } + else if (resolvableType == null || !hasProperty(resolvableType, fieldName)) { + this.reportBuilder.addUnmappedField(typeName, fieldName); } } } - private boolean isConnectionType(GraphQLObjectType objectType) { - return (objectType.getName().endsWith("Connection") && + private GraphQLType unwrapNonNull(GraphQLType type) { + return (type instanceof GraphQLNonNull graphQLNonNull ? graphQLNonNull.getWrappedType() : type); + } + + private boolean isConnectionType(GraphQLType type) { + return (type instanceof GraphQLObjectType objectType && + objectType.getName().endsWith("Connection") && objectType.getField("edges") != null && objectType.getField("pageInfo") != null); } - @NotNull - private GraphQLObjectType getEdgeNodeType(GraphQLObjectType objectType) { - GraphQLList edgesListType = (GraphQLList) unwrapNonNull(objectType.getField("edges").getType()); - GraphQLObjectType edgeType = (GraphQLObjectType) edgesListType.getWrappedType(); - return (GraphQLObjectType) unwrapNonNull(edgeType.getField("node").getType()); + private GraphQLType getConnectionNodeType(GraphQLType type) { + String name = ((GraphQLObjectType) type).getName(); + name = name.substring(0, name.length() - 10); + type = this.schema.getType(name); + Assert.state(type != null, "No '" + name + "' type"); + return type; } - private GraphQLType unwrapNonNull(GraphQLType outputType) { - return (outputType instanceof GraphQLNonNull wrapper ? wrapper.getWrappedType() : outputType); + private static ResolvableType nest(@Nullable ResolvableType resolvableType, GraphQLType type) { + Assert.notNull(resolvableType, "No declaredType for " + getTypeName(type)); + resolvableType = resolvableType.getNested(2); + return resolvableType; } - @Nullable - private Class unwrapPublisherTypes(ResolvableType declaredType) { - Class rawClass = declaredType.getRawClass(); - if (rawClass != null) { - ReactiveAdapter adapter = ReactiveAdapterRegistry.getSharedInstance().getAdapter(declaredType.getRawClass()); - if (adapter != null) { - return declaredType.getNested(2).getRawClass(); - } + private static String getTypeName(GraphQLType type) { + return (type instanceof GraphQLNamedType namedType ? namedType.getName() : type.toString()); + } + + private static boolean isNotScalarOrEnumType(GraphQLType type) { + return !(type instanceof GraphQLScalarType || type instanceof GraphQLEnumType); + } + + private boolean hasProperty(ResolvableType resolvableType, String fieldName) { + try { + Class clazz = resolveClassToCompare(resolvableType); + return (BeanUtils.getPropertyDescriptor(clazz, fieldName) != null); + } + catch (BeansException ex) { + throw new IllegalStateException( + "Failed to introspect " + resolvableType + " for field '" + fieldName + "'", ex); } - return rawClass; } - private boolean isTypeAlreadyInspected(GraphQLObjectType objectType) { - return !this.seenTypes.add(objectType.getName()); + private Class resolveClassToCompare(ResolvableType resolvableType) { + Class clazz = resolvableType.resolve(Object.class); + ReactiveAdapter adapter = this.reactiveAdapterRegistry.getAdapter(clazz); + return (adapter != null ? resolvableType.getNested(2).resolve(Object.class) : clazz); } /** - * Inspect the schema mappings and produce a report. + * Check the schema against {@code DataFetcher} registrations, and produce a report. * @param schema the schema to inspect - * @param runtimeWiring for {@code DataFetcher} mappings - * @return the resulting report + * @param runtimeWiring for {@code DataFetcher} registrations + * @return the created report */ public static Report inspect(GraphQLSchema schema, RuntimeWiring runtimeWiring) { - return new SchemaMappingInspector(schema, runtimeWiring).inspectMappings(); + SchemaMappingInspector inspector = new SchemaMappingInspector(schema, runtimeWiring); + return inspector.inspect(); } - record Report(MultiValueMap missingOperations, MultiValueMap missingFields) { + /** + * Container of unmapped fields and skipped types. + * @param unmappedFields fields with neither {@link DataFetcher} mapping nor Object property + * @param skippedTypes types that could not be verified, e.g. union + */ + public record Report(MultiValueMap unmappedFields, Set skippedTypes) { - String getSummary() { - StringBuilder builder = new StringBuilder("GraphQL schema inspection found "); - if (this.missingOperations.isEmpty() && this.missingFields.isEmpty()) { - builder.append("no missing mapping."); - } - else { - builder.append(getDetailedReport()); - } - return builder.toString(); + @Override + public String toString() { + return "GraphQL schema inspection:\n" + + "\tUnmapped fields: " + this.unmappedFields + "\n" + + "\tSkipped types: " + this.skippedTypes; } - - private String getDetailedReport() { - Stream missingOperationsReport = this.missingOperations.keySet().stream() - .map(operationName -> String.format("%s%s", operationName, this.missingOperations.get(operationName))); - Stream missingFieldsReport = this.missingFields.keySet().stream() - .map(typeName -> String.format("%s%s", typeName, this.missingFields.get(typeName))); - return Stream.concat(missingOperationsReport, missingFieldsReport) - .collect(Collectors.joining(", ", "missing mappings for: ", ".")); - } - - boolean isEmpty() { - return this.missingOperations.isEmpty() && this.missingFields.isEmpty(); - } - } + /** + * Builder for a {@link Report}. + */ private static class ReportBuilder { - private final MultiValueMap missingOperations = new LinkedMultiValueMap<>(); + private final MultiValueMap unmappedFields = new LinkedMultiValueMap<>(); - private final MultiValueMap missingFields = new LinkedMultiValueMap<>(); - - private ReportBuilder() { + private final Set skippedTypes = new LinkedHashSet<>(); + /** + * Add an unmapped field. + */ + public void addUnmappedField(String typeName, String fieldName) { + this.unmappedFields.add(typeName, fieldName); } - static ReportBuilder create() { - return new ReportBuilder(); + /** + * Add a skipped type name. + */ + public void addSkippedType(String typeName) { + this.skippedTypes.add(typeName); } - ReportBuilder missingOperation(GraphQLObjectType operationType, GraphQLFieldDefinition operationDefinition) { - this.missingOperations.add(operationType.getName(), operationDefinition.getName()); - return this; - } - - ReportBuilder missingField(GraphQLObjectType objectType, GraphQLFieldDefinition field) { - this.missingFields.add(objectType.getName(), field.getName()); - return this; - } - - Report build() { - return new Report(this.missingOperations, this.missingFields); + public Report build() { + return new Report( + new LinkedMultiValueMap<>(this.unmappedFields), + new LinkedHashSet<>(this.skippedTypes)); } } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/execution/SelfDescribingDataFetcher.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/SelfDescribingDataFetcher.java index 1252f2a1..12860f60 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/execution/SelfDescribingDataFetcher.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/execution/SelfDescribingDataFetcher.java @@ -36,5 +36,5 @@ public interface SelfDescribingDataFetcher extends DataFetcher { * @return the type of the data returned by the data fetcher. */ ResolvableType getReturnType(); - + } diff --git a/spring-graphql/src/test/java/org/springframework/graphql/execution/SchemaMappingInspectorTests.java b/spring-graphql/src/test/java/org/springframework/graphql/execution/SchemaMappingInspectorTests.java index 6d4f7115..899a763f 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/execution/SchemaMappingInspectorTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/execution/SchemaMappingInspectorTests.java @@ -19,9 +19,11 @@ package org.springframework.graphql.execution; import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Set; import graphql.schema.GraphQLSchema; import graphql.schema.idl.RuntimeWiring; +import graphql.schema.idl.SchemaGenerator; import org.assertj.core.api.AbstractAssert; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -53,21 +55,21 @@ class SchemaMappingInspectorTests { @Nested - class QueriesInspectionTests { + class QueryTests { @Test - void hasMissingQueryEntryWhenMissingQueryMapping() { + void reportHasUnmappedQuery() { String schema = """ type Query { greeting: String } """; SchemaMappingInspector.Report report = inspectSchema(schema, EmptyController.class); - assertThatReport(report).hasSize(1).missesOperations("Query", "greeting"); + assertThatReport(report).hasUnmappedFieldCount(1).containsUnmappedFields("Query", "greeting"); } @Test - void reportIsEmptyWhenQueryMapping() { + void reportIsEmptyWhenQueryIsMapped() { String schema = """ type Query { greeting: String @@ -78,7 +80,7 @@ class SchemaMappingInspectorTests { } @Test - void inspectTypeForCollections() { + void reportWorksForQueryWithList() { String schema = """ type Query { allBooks: [Book] @@ -91,15 +93,30 @@ class SchemaMappingInspectorTests { } """; SchemaMappingInspector.Report report = inspectSchema(schema, BookController.class); - assertThatReport(report).hasSize(1).missesFields("Book", "missing"); + assertThatReport(report).hasUnmappedFieldCount(1).containsUnmappedFields("Book", "missing"); } @Test - void inspectTypeForConnection() { + void reportWorksForQueryWithConnection() { String schema = """ type Query { paginatedBooks: BookConnection } + + type BookConnection { + edges: [BookEdge]! + pageInfo: PageInfo! + } + + type BookEdge { + cursor: String! + # ... + } + + type PageInfo { + startCursor: String + # ... + } type Book { id: ID @@ -108,11 +125,11 @@ class SchemaMappingInspectorTests { } """; SchemaMappingInspector.Report report = inspectSchema(schema, BookController.class); - assertThatReport(report).hasSize(1).missesFields("Book", "missing"); + assertThatReport(report).hasUnmappedFieldCount(1).containsUnmappedFields("Book", "missing"); } @Test - void inspectExtensionTypesForQueries() { + void reportWorksForQueryWithExtensionType() { String schema = """ type Query { } @@ -121,15 +138,17 @@ class SchemaMappingInspectorTests { } """; SchemaMappingInspector.Report report = inspectSchema(schema, EmptyController.class); - assertThatReport(report).hasSize(1).missesOperations("Query", "greeting"); + assertThatReport(report).hasUnmappedFieldCount(1).containsUnmappedFields("Query", "greeting"); } } + @Nested - class MutationInspectionTests { + class MutationTests { + @Test - void hasMissingOperationEntryWhenMissingQueryMapping() { + void reportContainsUnmappedMutation() { String schema = """ type Query{ greeting: String @@ -144,11 +163,11 @@ class SchemaMappingInspectorTests { } """; SchemaMappingInspector.Report report = inspectSchema(schema, GreetingController.class); - assertThatReport(report).hasSize(1).missesOperations("Mutation", "createBook"); + assertThatReport(report).hasUnmappedFieldCount(1).containsUnmappedFields("Mutation", "createBook"); } @Test - void reportIsEmptyWhenMutationMapping() { + void reportIsEmptyWhenMutationIsMapped() { String schema = """ type Query{ greeting: String @@ -167,7 +186,7 @@ class SchemaMappingInspectorTests { } @Test - void inspectExtensionTypesForMutations() { + void reportWorksForMutationWithExtensionType() { String schema = """ type Query { greeting: String @@ -183,15 +202,17 @@ class SchemaMappingInspectorTests { } """; SchemaMappingInspector.Report report = inspectSchema(schema, GreetingController.class); - assertThatReport(report).hasSize(1).missesOperations("Mutation", "createBook"); + assertThatReport(report).hasUnmappedFieldCount(1).containsUnmappedFields("Mutation", "createBook"); } } + @Nested - class SubscriptionInspectionTests { + class SubscriptionTests { + @Test - void hasMissingOperationEntryWhenMissingSubscriptionMapping() { + void reportContainsUnmappedSubscription() { String schema = """ type Query{ greeting: String @@ -206,11 +227,11 @@ class SchemaMappingInspectorTests { } """; SchemaMappingInspector.Report report = inspectSchema(schema, GreetingController.class); - assertThatReport(report).hasSize(1).missesOperations("Subscription", "bookSearch"); + assertThatReport(report).hasUnmappedFieldCount(1).containsUnmappedFields("Subscription", "bookSearch"); } @Test - void reportIsEmptyWhenSubscriptionMapping() { + void reportIsEmptyWhenSubscriptionIsMapped() { String schema = """ type Query{ greeting: String @@ -229,7 +250,7 @@ class SchemaMappingInspectorTests { } @Test - void inspectExtensionTypesForSubscriptions() { + void reportWorksForSubscriptionWithExtensionType() { String schema = """ type Query{ greeting: String @@ -245,15 +266,17 @@ class SchemaMappingInspectorTests { } """; SchemaMappingInspector.Report report = inspectSchema(schema, GreetingController.class); - assertThatReport(report).hasSize(1).missesOperations("Subscription", "bookSearch"); + assertThatReport(report).hasUnmappedFieldCount(1).containsUnmappedFields("Subscription", "bookSearch"); } } + @Nested class TypesInspectionTests { + @Test - void reportIsEmptyWhenPropertyOnType() { + void reportIsEmptyWhenFieldHasMatchingObjectProperty() { String schema = """ type Query { bookById(id: ID): Book @@ -269,7 +292,7 @@ class SchemaMappingInspectorTests { } @Test - void reportIsEmptyWhenDataFetcherForField() { + void reportIsEmptyWhenFieldHasDataFetcherMapping() { String schema = """ type Query { bookById(id: ID): Book @@ -286,7 +309,7 @@ class SchemaMappingInspectorTests { } @Test - void hasMissingFieldEntryWhenMissingPropertyOnType() { + void reportHasUnmappedField() { String schema = """ type Query { bookById(id: ID): Book @@ -299,11 +322,11 @@ class SchemaMappingInspectorTests { } """; SchemaMappingInspector.Report report = inspectSchema(schema, BookController.class); - assertThatReport(report).hasSize(1).missesFields("Book", "missing"); + assertThatReport(report).hasUnmappedFieldCount(1).containsUnmappedFields("Book", "missing"); } @Test - void hasMissingFieldEntryWhenMissingPropertyOnNestedType() { + void reportHasUnmappedFieldOnNestedType() { String schema = """ type Query { bookById(id: ID): Book @@ -322,11 +345,11 @@ class SchemaMappingInspectorTests { } """; SchemaMappingInspector.Report report = inspectSchema(schema, BookController.class); - assertThatReport(report).hasSize(1).missesFields("Author", "missing"); + assertThatReport(report).hasUnmappedFieldCount(1).containsUnmappedFields("Author", "missing"); } @Test - void cyclicRelationBetweenTypesDoNotFail() { + void reportWorksWithCyclicRelations() { String schema = """ type Query { teamById(id: ID): Team @@ -340,14 +363,15 @@ class SchemaMappingInspectorTests { type TeamMember { name: String team: Team + missing: String } """; SchemaMappingInspector.Report report = inspectSchema(schema, TeamController.class); - assertThatReport(report).isEmpty(); + assertThatReport(report).hasUnmappedFieldCount(1).containsUnmappedFields("TeamMember", "missing"); } @Test - void hasMissingFieldEntryWhenMissingPropertyOnTypeProvidedByExtension() { + void reportWorksWithPropertyOnTypeExtension() { String schema = """ type Query { bookById(id: ID): Book @@ -362,27 +386,93 @@ class SchemaMappingInspectorTests { } """; SchemaMappingInspector.Report report = inspectSchema(schema, BookController.class); - assertThatReport(report).hasSize(1).missesFields("Book", "missing"); + assertThatReport(report).hasUnmappedFieldCount(1).containsUnmappedFields("Book", "missing"); } - } + @Test + void reportHasSkippedUnionType() { + String schema = """ + type Query { + fooBar: FooBar + } - @Nested - class ReportFormatTests { + union FooBar = Foo | Bar - @Test - void reportsMissingQuery() { - String schema = """ + type Foo { + name: String + } + + type Bar { + name: String + } + """; + SchemaMappingInspector.Report report = inspectSchema(schema, UnionController.class); + assertThatReport(report).hasUnmappedFieldCount(0).hasSkippedTypeCount(1).containsSkippedTypes("FooBar"); + } + + @Test + void reportHasSkippedTypeForUnknownDataFetcherType() { + String schemaContent = """ + type Query { + bookById(id: ID): Book + } + + type Book { + id: ID + name: String + } + """; + + GraphQLSchema schema = GraphQlSetup.schemaContent(schemaContent).toGraphQlSource().schema(); + RuntimeWiring wiring = RuntimeWiring.newRuntimeWiring() + .type("Query", builder -> builder.dataFetcher("bookById", environment -> null)) + .build(); + + SchemaMappingInspector.Report report = SchemaMappingInspector.inspect(schema, wiring); + assertThatReport(report).hasUnmappedFieldCount(0).hasSkippedTypeCount(1).containsSkippedTypes("Book"); + } + + @Test + void reportIsEmptyIfUnknownDataFetcherReturnsSimpleType() { + String schemaContent = """ type Query { greeting: String } """; - SchemaMappingInspector.Report report = inspectSchema(schema, EmptyController.class); - assertThat(report.getSummary()).isEqualTo("GraphQL schema inspection found missing mappings for: Query[greeting]."); - } - @Test - void reportMissingField() { + GraphQLSchema schema = GraphQlSetup.schemaContent(schemaContent).toGraphQlSource().schema(); + RuntimeWiring wiring = RuntimeWiring.newRuntimeWiring() + .type("Query", builder -> builder.dataFetcher("greeting", environment -> null)) + .build(); + + SchemaMappingInspector.Report report = SchemaMappingInspector.inspect(schema, wiring); + assertThatReport(report).hasUnmappedFieldCount(0).hasSkippedTypeCount(0); + } + + @Test + void reportHasSkippedTypeForObjectReturnType() { + String schemaContent = """ + type Query { + bookObject(id: ID): Book + } + + type Book { + id: ID + name: String + } + """; + SchemaMappingInspector.Report report = inspectSchema(schemaContent, BookController.class); + assertThatReport(report).hasUnmappedFieldCount(0).hasSkippedTypeCount(1).containsSkippedTypes("Book"); + } + + } + + + @Nested + class ReportFormatTests { + + @Test + void reportUnmappedField() { String schema = """ type Query { allBooks: [Book] @@ -395,17 +485,48 @@ class SchemaMappingInspectorTests { } """; SchemaMappingInspector.Report report = inspectSchema(schema, BookController.class); - assertThat(report.getSummary()).isEqualTo("GraphQL schema inspection found missing mappings for: Book[missing]."); + assertThatReport(report).hasUnmappedFieldCount(1).hasSkippedTypeCount(0); + assertThat(report.toString()).isEqualTo(""" + GraphQL schema inspection: + Unmapped fields: {Book=[missing]} + Skipped types: []"""); } } + private SchemaMappingInspector.Report inspectSchema(String schemaContent, Class... controllers) { + GraphQLSchema schema = SchemaGenerator.createdMockedSchema(schemaContent); + RuntimeWiring runtimeWiring = createRuntimeWiring(controllers); + return SchemaMappingInspector.inspect(schema, runtimeWiring); + } + + private RuntimeWiring createRuntimeWiring(Class... controllerTypes) { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); + for (Class controllerType : controllerTypes) { + context.registerBean(controllerType); + } + context.refresh(); + + AnnotatedControllerConfigurer configurer = new AnnotatedControllerConfigurer(); + configurer.setApplicationContext(context); + configurer.afterPropertiesSet(); + + RuntimeWiring.Builder wiringBuilder = RuntimeWiring.newRuntimeWiring(); + configurer.configure(wiringBuilder); + return wiringBuilder.build(); + } + + static SchemaInspectionReportAssert assertThatReport(SchemaMappingInspector.Report actual) { + return new SchemaInspectionReportAssert(actual); + } + @Controller static class EmptyController { } + @Controller static class GreetingController { @@ -415,6 +536,7 @@ class SchemaMappingInspectorTests { } } + @Controller static class BookController { @@ -443,6 +565,11 @@ class SchemaMappingInspectorTests { return "custom fetcher"; } + @QueryMapping + public Object bookObject(@Argument Long id) { + return new Book(); + } + @MutationMapping public Book createBook() { return new Book(); @@ -454,6 +581,7 @@ class SchemaMappingInspectorTests { } } + @Controller static class TeamController { @QueryMapping @@ -473,47 +601,29 @@ class SchemaMappingInspectorTests { } + record Team(String name, List members) { } + record TeamMember(String name, Team team) { } - SchemaMappingInspector.Report inspectSchema(String schemaContent, Class... controllers) { - RuntimeWiringConfigurer wiringConfigurer = createRuntimeWiring(controllers); - RuntimeWiring.Builder wiringBuilder = RuntimeWiring.newRuntimeWiring(); - wiringConfigurer.configure(wiringBuilder); - GraphQLSchema schema = GraphQlSetup.schemaContent(schemaContent) - .runtimeWiring(wiringConfigurer) - .typeDefinitionConfigurer(new ConnectionTypeDefinitionConfigurer()) - .toGraphQlSource() - .schema(); + @Controller + static class UnionController { - return SchemaMappingInspector.inspect(schema, wiringBuilder.build()); - } - - private RuntimeWiringConfigurer createRuntimeWiring(Class... handlerTypes) { - AnnotationConfigApplicationContext appContext = new AnnotationConfigApplicationContext(); - for (Class handlerType : handlerTypes) { - appContext.registerBean(handlerType); + @QueryMapping + Object fooBar() { + return "Hello"; } - appContext.refresh(); - - AnnotatedControllerConfigurer configurer = new AnnotatedControllerConfigurer(); - configurer.setApplicationContext(appContext); - configurer.afterPropertiesSet(); - - return configurer; } - static SchemaInspectionReportAssert assertThatReport(SchemaMappingInspector.Report actual) { - return new SchemaInspectionReportAssert(actual); - } - static class SchemaInspectionReportAssert extends AbstractAssert { + private static class SchemaInspectionReportAssert + extends AbstractAssert { public SchemaInspectionReportAssert(SchemaMappingInspector.Report actual) { super(actual, SchemaInspectionReportAssert.class); @@ -521,54 +631,47 @@ class SchemaMappingInspectorTests { public void isEmpty() { isNotNull(); - if (!this.actual.missingOperations().isEmpty()) { - failWithMessage("Report contains missing operations for %s", - this.actual.missingOperations().keySet()); - } - if (!this.actual.missingFields().isEmpty()) { - failWithMessage("Report contains missing fields for %s", - this.actual.missingFields().keySet()); + if (!this.actual.unmappedFields().isEmpty()) { + failWithMessage("Report contains missing fields for %s", this.actual.unmappedFields().keySet()); } } - public SchemaInspectionReportAssert hasSize(int size) { + public SchemaInspectionReportAssert hasUnmappedFieldCount(int expected) { isNotNull(); - Integer missingOps = this.actual.missingOperations().values().stream().map(List::size).reduce(0, Integer::sum); - Integer missingFields = this.actual.missingFields().values().stream().map(List::size).reduce(0, Integer::sum); - if ((missingOps + missingFields) != size) { - failWithMessage("Expected report with %s entries, found %d.", size, (missingOps + missingFields)); + Integer actual = this.actual.unmappedFields().values().stream().map(List::size).reduce(0, Integer::sum); + if (actual != expected) { + failWithMessage("Expected %s unmapped fields, found %s.", expected, this.actual.unmappedFields()); } return this; } - public SchemaInspectionReportAssert missesOperations(String operationType, String... names) { + public SchemaInspectionReportAssert hasSkippedTypeCount(int expected) { isNotNull(); - List expectedOperations = Arrays.asList(names); - List actualOperations = this.actual.missingOperations().get(operationType); - if (actualOperations != null) { - if (!actualOperations.containsAll(expectedOperations)) { - failWithMessage("Expected missing DataFetchers for %s: %s, found %s", operationType, expectedOperations, actualOperations); - } - } - else { - failWithMessage("No missing DataFetcher for %s", operationType); + if (this.actual.skippedTypes().size() != expected) { + failWithMessage("Expected %s skipped types, found %d.", expected, this.actual.skippedTypes()); } return this; } - public SchemaInspectionReportAssert missesFields(String typeName, String... fieldNames) { + public SchemaInspectionReportAssert containsUnmappedFields(String typeName, String... fieldNames) { isNotNull(); - List expectedFields = Arrays.asList(fieldNames); - List actualFields = this.actual.missingFields().get(typeName); - if (actualFields != null) { - if (!actualFields.containsAll(expectedFields)) { - failWithMessage("Expected missing fields for %s: %s, found %s", typeName, expectedFields, actualFields); - } + List expected = Arrays.asList(fieldNames); + List actual = this.actual.unmappedFields().get(typeName); + if (actual == null || !actual.containsAll(expected)) { + failWithMessage("Expected unmapped fields for %s: %s, found %s", typeName, expected, actual); } - else { - failWithMessage("No missing field for %s", typeName); + return this; + } + + public SchemaInspectionReportAssert containsSkippedTypes(String... typeNames) { + isNotNull(); + List expected = Arrays.asList(typeNames); + Set actual = this.actual.skippedTypes(); + if (!actual.containsAll(expected)) { + failWithMessage("Expected skipped types: %s, found %s", expected, actual); } return this; } } + } \ No newline at end of file