diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java index 9c40589e..0fbc3e81 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java @@ -62,7 +62,7 @@ import org.springframework.format.FormatterRegistrar; import org.springframework.format.support.DefaultFormattingConversionService; import org.springframework.format.support.FormattingConversionService; import org.springframework.graphql.data.GraphQlArgumentBinder; -import org.springframework.graphql.data.TypedDataFetcher; +import org.springframework.graphql.execution.SelfDescribingDataFetcher; import org.springframework.graphql.data.method.HandlerMethod; import org.springframework.graphql.data.method.HandlerMethodArgumentResolver; import org.springframework.graphql.data.method.HandlerMethodArgumentResolverComposite; @@ -540,7 +540,7 @@ public class AnnotatedControllerConfigurer /** * {@link DataFetcher} that wrap and invokes a {@link HandlerMethod}. */ - static class SchemaMappingDataFetcher implements TypedDataFetcher { + static class SchemaMappingDataFetcher implements SelfDescribingDataFetcher { private final MappingInfo info; @@ -632,7 +632,7 @@ public class AnnotatedControllerConfigurer } @Override - public ResolvableType getDeclaredType() { + public ResolvableType getReturnType() { return ResolvableType.forMethodReturnType(this.info.getHandlerMethod().getMethod()); } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/query/QueryByExampleDataFetcher.java b/spring-graphql/src/main/java/org/springframework/graphql/data/query/QueryByExampleDataFetcher.java index 711142e5..7bbc2126 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/query/QueryByExampleDataFetcher.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/query/QueryByExampleDataFetcher.java @@ -41,7 +41,7 @@ import org.springframework.data.repository.query.ReactiveQueryByExampleExecutor; import org.springframework.data.util.TypeInformation; import org.springframework.graphql.data.GraphQlArgumentBinder; import org.springframework.graphql.data.GraphQlRepository; -import org.springframework.graphql.data.TypedDataFetcher; +import org.springframework.graphql.execution.SelfDescribingDataFetcher; import org.springframework.graphql.execution.RuntimeWiringConfigurer; import org.springframework.util.Assert; import org.springframework.validation.BindException; @@ -442,7 +442,7 @@ public abstract class QueryByExampleDataFetcher { } - private static class SingleEntityFetcher extends QueryByExampleDataFetcher implements TypedDataFetcher { + private static class SingleEntityFetcher extends QueryByExampleDataFetcher implements SelfDescribingDataFetcher { private final QueryByExampleExecutor executor; @@ -482,13 +482,13 @@ public abstract class QueryByExampleDataFetcher { } @Override - public ResolvableType getDeclaredType() { + public ResolvableType getReturnType() { return ResolvableType.forClass(this.resultType); } } - private static class ManyEntityFetcher extends QueryByExampleDataFetcher implements TypedDataFetcher> { + private static class ManyEntityFetcher extends QueryByExampleDataFetcher implements SelfDescribingDataFetcher> { private final QueryByExampleExecutor executor; @@ -528,14 +528,14 @@ public abstract class QueryByExampleDataFetcher { } @Override - public ResolvableType getDeclaredType() { + public ResolvableType getReturnType() { return ResolvableType.forClassWithGenerics(Iterable.class, this.resultType); } } - private static class ReactiveSingleEntityFetcher extends QueryByExampleDataFetcher implements TypedDataFetcher> { + private static class ReactiveSingleEntityFetcher extends QueryByExampleDataFetcher implements SelfDescribingDataFetcher> { private final ReactiveQueryByExampleExecutor executor; @@ -575,14 +575,14 @@ public abstract class QueryByExampleDataFetcher { } @Override - public ResolvableType getDeclaredType() { + public ResolvableType getReturnType() { return ResolvableType.forClassWithGenerics(Mono.class, this.resultType); } } - private static class ReactiveManyEntityFetcher extends QueryByExampleDataFetcher implements TypedDataFetcher> { + private static class ReactiveManyEntityFetcher extends QueryByExampleDataFetcher implements SelfDescribingDataFetcher> { private final ReactiveQueryByExampleExecutor executor; @@ -622,7 +622,7 @@ public abstract class QueryByExampleDataFetcher { } @Override - public ResolvableType getDeclaredType() { + public ResolvableType getReturnType() { return ResolvableType.forClassWithGenerics(Flux.class, this.resultType); } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/query/QuerydslDataFetcher.java b/spring-graphql/src/main/java/org/springframework/graphql/data/query/QuerydslDataFetcher.java index 4f2cdb25..82623660 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/query/QuerydslDataFetcher.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/query/QuerydslDataFetcher.java @@ -47,7 +47,7 @@ import org.springframework.data.repository.query.FluentQuery; import org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery; import org.springframework.data.util.TypeInformation; import org.springframework.graphql.data.GraphQlRepository; -import org.springframework.graphql.data.TypedDataFetcher; +import org.springframework.graphql.execution.SelfDescribingDataFetcher; import org.springframework.graphql.execution.RuntimeWiringConfigurer; import org.springframework.util.Assert; import org.springframework.util.LinkedMultiValueMap; @@ -540,7 +540,7 @@ public abstract class QuerydslDataFetcher { } - private static class SingleEntityFetcher extends QuerydslDataFetcher implements TypedDataFetcher { + private static class SingleEntityFetcher extends QuerydslDataFetcher implements SelfDescribingDataFetcher { private final QuerydslPredicateExecutor executor; @@ -584,13 +584,13 @@ public abstract class QuerydslDataFetcher { } @Override - public ResolvableType getDeclaredType() { + public ResolvableType getReturnType() { return ResolvableType.forClass(this.resultType); } } - private static class ManyEntityFetcher extends QuerydslDataFetcher implements TypedDataFetcher> { + private static class ManyEntityFetcher extends QuerydslDataFetcher implements SelfDescribingDataFetcher> { private final QuerydslPredicateExecutor executor; @@ -632,14 +632,14 @@ public abstract class QuerydslDataFetcher { } @Override - public ResolvableType getDeclaredType() { + public ResolvableType getReturnType() { return ResolvableType.forClassWithGenerics(Iterable.class, this.resultType); } } - private static class ReactiveSingleEntityFetcher extends QuerydslDataFetcher implements TypedDataFetcher> { + private static class ReactiveSingleEntityFetcher extends QuerydslDataFetcher implements SelfDescribingDataFetcher> { private final ReactiveQuerydslPredicateExecutor executor; @@ -682,14 +682,14 @@ public abstract class QuerydslDataFetcher { } @Override - public ResolvableType getDeclaredType() { + public ResolvableType getReturnType() { return ResolvableType.forClassWithGenerics(Mono.class, this.resultType); } } - private static class ReactiveManyEntityFetcher extends QuerydslDataFetcher implements TypedDataFetcher> { + private static class ReactiveManyEntityFetcher extends QuerydslDataFetcher implements SelfDescribingDataFetcher> { private final ReactiveQuerydslPredicateExecutor executor; @@ -732,7 +732,7 @@ public abstract class QuerydslDataFetcher { } @Override - public ResolvableType getDeclaredType() { + public ResolvableType getReturnType() { return ResolvableType.forClassWithGenerics(Flux.class, this.resultType); } 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 3c96076c..9c67a078 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 @@ -139,11 +139,13 @@ final class DefaultSchemaResourceGraphQlSourceBuilder } }); - SchemaInspector.Report schemaInspectionReport = new SchemaInspector().inspectSchema(registry, runtimeWiring); - if(!schemaInspectionReport.isEmpty()) { - logger.info(schemaInspectionReport.getSummary()); - logger.info(schemaInspectionReport.getDetailedReport()); - } + configureGraphQl(builder -> { + GraphQLSchema schema = builder.build().getGraphQLSchema(); + SchemaMappingInspector.Report schemaInspectionReport = new SchemaMappingInspector().inspectSchemaMappings(schema, runtimeWiring); + if(!schemaInspectionReport.isEmpty()) { + logger.info(schemaInspectionReport.getSummary()); + } + }); return (this.schemaFactory != null ? this.schemaFactory.apply(registry, runtimeWiring) : diff --git a/spring-graphql/src/main/java/org/springframework/graphql/execution/SchemaInspector.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/SchemaInspector.java deleted file mode 100644 index 86f3ea4e..00000000 --- a/spring-graphql/src/main/java/org/springframework/graphql/execution/SchemaInspector.java +++ /dev/null @@ -1,270 +0,0 @@ -/* - * Copyright 2020-2023 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.graphql.execution; - -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.function.Consumer; - -import graphql.language.FieldDefinition; -import graphql.language.ImplementingTypeDefinition; -import graphql.language.ListType; -import graphql.language.NonNullType; -import graphql.language.ObjectTypeDefinition; -import graphql.language.ObjectTypeExtensionDefinition; -import graphql.language.SDLExtensionDefinition; -import graphql.language.Type; -import graphql.language.TypeDefinition; -import graphql.language.TypeName; -import graphql.schema.DataFetcher; -import graphql.schema.idl.RuntimeWiring; -import graphql.schema.idl.TypeDefinitionRegistry; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; - -import org.springframework.beans.BeanUtils; -import org.springframework.beans.BeansException; -import org.springframework.core.ReactiveAdapter; -import org.springframework.core.ReactiveAdapterRegistry; -import org.springframework.core.ResolvableType; -import org.springframework.graphql.data.TypedDataFetcher; -import org.springframework.lang.Nullable; -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: - *

    - *
  • {@code Query}, {@code Mutation} and {@code Subscription} fields that have no corresponding {@link DataFetcher} registered - *
  • Fields in other schema types that have no property on the relevant Java type, or no DataFetcher registered - *
- *

This approach has several known limitations; the corresponding Java types are only discovered through registered - * {@code DataFetcher} instances, if they implement the {@link TypedDataFetcher} contract. Union types are not supported, - * even if a common interface is declared by a {@link TypedDataFetcher}. - * - * @author Brian Clozel - * @since 1.2.0 - */ -class SchemaInspector { - - private static final Log logger = LogFactory.getLog(SchemaInspector.class); - - Report inspectSchema(TypeDefinitionRegistry typeDefinitionRegistry, RuntimeWiring runtimeWiring) { - ReportBuilder report = ReportBuilder.create(); - SchemaInspection inspection = new SchemaInspection(typeDefinitionRegistry, runtimeWiring); - inspection.inspectOperation("Query", report); - inspection.inspectOperation("Mutation", report); - inspection.inspectOperation("Subscription", report); - return report.build(); - } - - private static class SchemaInspection { - - private final TypeDefinitionRegistry typeDefinitionRegistry; - - private final RuntimeWiring runtimeWiring; - - private final Set seenTypes = new HashSet<>(); - - SchemaInspection(TypeDefinitionRegistry typeDefinitionRegistry, RuntimeWiring runtimeWiring) { - this.typeDefinitionRegistry = typeDefinitionRegistry; - this.runtimeWiring = runtimeWiring; - } - - @SuppressWarnings("rawtypes") - void inspectOperation(String operationName, ReportBuilder report) { - Map queryFetchers = this.runtimeWiring.getDataFetcherForType(operationName); - this.typeDefinitionRegistry.getType(operationName, ObjectTypeDefinition.class) - .ifPresent(queryType -> inspectOperation(queryType, queryFetchers, report)); - forEachObjectTypeExtension(operationName, objectTypeExtension -> inspectOperation(objectTypeExtension, queryFetchers, report)); - } - - @SuppressWarnings("rawtypes") - private void inspectOperation(ObjectTypeDefinition operationDefinition, Map operationDataFetchers, ReportBuilder report) { - for (FieldDefinition fieldDefinition : operationDefinition.getFieldDefinitions()) { - if (operationDataFetchers.containsKey(fieldDefinition.getName())) { - DataFetcher fieldDataFetcher = operationDataFetchers.get(fieldDefinition.getName()); - if (fieldDataFetcher instanceof TypedDataFetcher typedDataFetcher) { - inspectType(fieldDefinition.getType(), typedDataFetcher.getDeclaredType(), report); - } - } - else { - report.missingOperation(operationDefinition, fieldDefinition); - } - } - } - - private void inspectType(Type fieldType, ResolvableType declaredType, ReportBuilder report) { - if (fieldType instanceof TypeName typeName) { - this.typeDefinitionRegistry.getType(typeName) - .ifPresent(typeDefinition -> inspectTypeDefinition(typeDefinition, declaredType, report)); - forEachObjectTypeExtension(typeName.getName(), - objectTypeExtension -> inspectTypeDefinition(objectTypeExtension, declaredType, report)); - } - else if (fieldType instanceof ListType listType) { - inspectType(listType.getType(), declaredType.getNested(2), report); - } - else if (fieldType instanceof NonNullType nonNullType) { - inspectType(nonNullType.getType(), declaredType, report); - } - } - - private void inspectTypeDefinition(TypeDefinition typeDefinition, ResolvableType declaredType, ReportBuilder report) { - if (typeDefinition instanceof ImplementingTypeDefinition implementingTypeDefinition) { - inspectImplementingType(implementingTypeDefinition, declaredType, report); - } - else if (logger.isDebugEnabled()){ - logger.debug("Cannot inspect type '" + typeDefinition.getName() + "', inspector does not support " - + typeDefinition.getClass().getSimpleName()); - } - } - - @SuppressWarnings("rawtypes") - private void inspectImplementingType(ImplementingTypeDefinition typeDefinition, ResolvableType declaredType, ReportBuilder report) { - if (isTypeAlreadyInspected(typeDefinition)) { - return; - } - Map typeDataFetcher = this.runtimeWiring.getDataFetcherForType(typeDefinition.getName()); - Class declaredClass = unwrapPublisherTypes(declaredType); - for (FieldDefinition field : typeDefinition.getFieldDefinitions()) { - if (typeDataFetcher.containsKey(field.getName())) { - DataFetcher fieldDataFetcher = typeDataFetcher.get(field.getName()); - if (fieldDataFetcher instanceof TypedDataFetcher typedFieldDataFetcher) { - inspectType(field.getType(), typedFieldDataFetcher.getDeclaredType(), report); - } - } - else { - try { - if (declaredClass == null || BeanUtils.getPropertyDescriptor(declaredClass, field.getName()) == null) { - report.missingField(typeDefinition, field); - } - } - catch (BeansException exc) { - logger.debug("Failed while inspecting " + declaredType + " for property " + field.getName() + "", exc); - } - } - } - for (Type interfaceType : typeDefinition.getImplements()) { - inspectType(interfaceType, declaredType, report); - } - } - - private void forEachObjectTypeExtension(String typeName, Consumer extensionsConsumer) { - List objectTypeExtensions = this.typeDefinitionRegistry.objectTypeExtensions().get(typeName); - if (objectTypeExtensions != null) { - objectTypeExtensions.forEach(extensionsConsumer); - } - } - - @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(); - } - } - return rawClass; - } - - private boolean isTypeAlreadyInspected(ImplementingTypeDefinition typeDefinition) { - if (typeDefinition instanceof SDLExtensionDefinition) { - return false; - } - boolean inspectedType = this.seenTypes.contains(typeDefinition.getName()); - if (!inspectedType) { - this.seenTypes.add(typeDefinition.getName()); - } - return inspectedType; - } - - } - - record Report(MultiValueMap missingOperations, MultiValueMap missingFields) { - - String getSummary() { - StringBuilder builder = new StringBuilder("GraphQL schema inspection found "); - if (this.missingOperations.isEmpty()) { - builder.append("no missing mappings for operations"); - } - else { - builder.append("missing mappings for ").append(this.missingOperations.keySet()); - } - if (this.missingFields.isEmpty()) { - builder.append(", no missing data fetchers for inspected types."); - } - else { - builder.append(", missing data fetchers for types ").append(this.missingFields.keySet()).append('.'); - } - return builder.toString(); - } - - String getDetailedReport() { - StringBuilder builder = new StringBuilder(); - this.missingOperations.keySet().forEach(operationName -> { - builder.append(String.format("- on %s: %s", operationName, this.missingOperations.get(operationName))) - .append(System.lineSeparator()); - }); - this.missingFields.keySet().forEach(typeName -> { - builder.append(String.format("- on %s: %s", typeName, this.missingFields.get(typeName))) - .append(System.lineSeparator()); - }); - return builder.toString(); - } - - boolean isEmpty() { - return this.missingOperations.isEmpty() && this.missingFields.isEmpty(); - } - - } - - private static class ReportBuilder { - - private final MultiValueMap missingOperations = new LinkedMultiValueMap<>(); - - private final MultiValueMap missingFields = new LinkedMultiValueMap<>(); - - private ReportBuilder() { - - } - - static ReportBuilder create() { - return new ReportBuilder(); - } - - ReportBuilder missingOperation(ImplementingTypeDefinition operationType, FieldDefinition operationDefinition) { - this.missingOperations.add(operationType.getName(), operationDefinition.getName()); - return this; - } - - ReportBuilder missingField(ImplementingTypeDefinition type, FieldDefinition field) { - this.missingFields.add(type.getName(), field.getName()); - return this; - } - - Report build() { - return new Report(this.missingOperations, this.missingFields); - } - - } - -} 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 new file mode 100644 index 00000000..a5e9a82a --- /dev/null +++ b/spring-graphql/src/main/java/org/springframework/graphql/execution/SchemaMappingInspector.java @@ -0,0 +1,221 @@ +/* + * Copyright 2020-2023 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.graphql.execution; + +import java.util.HashSet; +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.GraphQLFieldDefinition; +import graphql.schema.GraphQLList; +import graphql.schema.GraphQLNamedType; +import graphql.schema.GraphQLNonNull; +import graphql.schema.GraphQLObjectType; +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.springframework.beans.BeanUtils; +import org.springframework.beans.BeansException; +import org.springframework.core.ReactiveAdapter; +import org.springframework.core.ReactiveAdapterRegistry; +import org.springframework.core.ResolvableType; +import org.springframework.lang.Nullable; +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: + *

    + *
  • {@code Query}, {@code Mutation} and {@code Subscription} fields that have no corresponding {@link DataFetcher} registered + *
  • Fields in other schema types that have no property on the relevant Java type, or no DataFetcher registered + *
+ *

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}. + * + * @author Brian Clozel + * @since 1.2.0 + */ +class SchemaMappingInspector { + + private static final Log logger = LogFactory.getLog(SchemaMappingInspector.class); + + Report inspectSchemaMappings(GraphQLSchema schema, RuntimeWiring runtimeWiring) { + ReportBuilder report = ReportBuilder.create(); + SchemaMappingInspection inspection = new SchemaMappingInspection(runtimeWiring); + inspection.inspectOperation(schema.getQueryType(), report); + inspection.inspectOperation(schema.getMutationType(), report); + inspection.inspectOperation(schema.getSubscriptionType(), report); + return report.build(); + } + + private static class SchemaMappingInspection { + + private final RuntimeWiring runtimeWiring; + + private final Set seenTypes = new HashSet<>(); + + SchemaMappingInspection(RuntimeWiring runtimeWiring) { + this.runtimeWiring = runtimeWiring; + } + + @SuppressWarnings("rawtypes") + 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, ResolvableType declaredType, ReportBuilder report) { + if (type instanceof GraphQLObjectType objectType) { + inspectObjectType(objectType, declaredType, report); + } + else if (type instanceof GraphQLList listType) { + inspectType(listType.getWrappedType(), declaredType.getNested(2), report); + } + 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)) { + 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); + } + } + 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); + } + } + } + } + + @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(); + } + } + return rawClass; + } + + private boolean isTypeAlreadyInspected(GraphQLObjectType objectType) { + return !this.seenTypes.add(objectType.getName()); + } + + } + + record Report(MultiValueMap missingOperations, MultiValueMap missingFields) { + + 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(); + } + + 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(); + } + + } + + private static class ReportBuilder { + + private final MultiValueMap missingOperations = new LinkedMultiValueMap<>(); + + private final MultiValueMap missingFields = new LinkedMultiValueMap<>(); + + private ReportBuilder() { + + } + + static ReportBuilder create() { + return new ReportBuilder(); + } + + 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); + } + + } + +} diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/TypedDataFetcher.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/SelfDescribingDataFetcher.java similarity index 51% rename from spring-graphql/src/main/java/org/springframework/graphql/data/TypedDataFetcher.java rename to spring-graphql/src/main/java/org/springframework/graphql/execution/SelfDescribingDataFetcher.java index 4ee14891..1252f2a1 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/TypedDataFetcher.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/execution/SelfDescribingDataFetcher.java @@ -14,28 +14,27 @@ * limitations under the License. */ -package org.springframework.graphql.data; +package org.springframework.graphql.execution; import graphql.schema.DataFetcher; -import graphql.schema.DataFetchingEnvironment; import org.springframework.core.ResolvableType; /** - * Specialized {@link DataFetcher} that can provide {@link ResolvableType type information} - * about the {@link #get(DataFetchingEnvironment) instances returned}. - *

Such {@code DataFetchers} are often backed by actual Java methods with declared return types. - * Declared types might not reflect the concrete type of the returned instance. + * Specialized {@link DataFetcher} that can provide information about itself. * @author Brian Clozel * @since 1.2.0 */ -public interface TypedDataFetcher extends DataFetcher { +public interface SelfDescribingDataFetcher extends DataFetcher { /** - * The type declared by this {@link DataFetcher}. - *

The concrete type of the returned instance might differ from the declared one. - * @return the declared type for the data to be fetched. + * The type that the {@link DataFetcher} returns. + *

This could be a type from a {@code @Controller} method declaration + * or the type expected to be returned by the {@link DataFetcher} + * possibly backed by a Spring Data repository. + * The concrete type of the returned instance might be a subclass. + * @return the type of the data returned by the data fetcher. */ - ResolvableType getDeclaredType(); + ResolvableType getReturnType(); } diff --git a/spring-graphql/src/test/java/org/springframework/graphql/execution/SchemaInspectorTests.java b/spring-graphql/src/test/java/org/springframework/graphql/execution/SchemaMappingInspectorTests.java similarity index 78% rename from spring-graphql/src/test/java/org/springframework/graphql/execution/SchemaInspectorTests.java rename to spring-graphql/src/test/java/org/springframework/graphql/execution/SchemaMappingInspectorTests.java index 2f28e5d3..5372fa87 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/execution/SchemaInspectorTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/execution/SchemaMappingInspectorTests.java @@ -20,9 +20,9 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import graphql.schema.GraphQLSchema; import graphql.schema.idl.RuntimeWiring; -import graphql.schema.idl.SchemaParser; -import graphql.schema.idl.TypeDefinitionRegistry; +import graphql.schema.idl.SchemaGenerator; import org.assertj.core.api.AbstractAssert; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -42,11 +42,11 @@ import org.springframework.stereotype.Controller; import static org.assertj.core.api.Assertions.assertThat; /** - * Tests for {@link SchemaInspector}. + * Tests for {@link SchemaMappingInspector}. * * @author Brian Clozel */ -class SchemaInspectorTests { +class SchemaMappingInspectorTests { @Nested @@ -59,7 +59,7 @@ class SchemaInspectorTests { greeting: String } """; - SchemaInspector.Report report = inspectSchema(schema, EmptyController.class); + SchemaMappingInspector.Report report = inspectSchema(schema, EmptyController.class); assertThatReport(report).hasSize(1).missesOperations("Query", "greeting"); } @@ -70,7 +70,7 @@ class SchemaInspectorTests { greeting: String } """; - SchemaInspector.Report report = inspectSchema(schema, GreetingController.class); + SchemaMappingInspector.Report report = inspectSchema(schema, GreetingController.class); assertThatReport(report).isEmpty(); } @@ -87,7 +87,7 @@ class SchemaInspectorTests { missing: Boolean } """; - SchemaInspector.Report report = inspectSchema(schema, BookController.class); + SchemaMappingInspector.Report report = inspectSchema(schema, BookController.class); assertThatReport(report).hasSize(1).missesFields("Book", "missing"); } @@ -100,7 +100,7 @@ class SchemaInspectorTests { greeting: String } """; - SchemaInspector.Report report = inspectSchema(schema, EmptyController.class); + SchemaMappingInspector.Report report = inspectSchema(schema, EmptyController.class); assertThatReport(report).hasSize(1).missesOperations("Query", "greeting"); } @@ -111,6 +111,9 @@ class SchemaInspectorTests { @Test void hasMissingOperationEntryWhenMissingQueryMapping() { String schema = """ + type Query{ + greeting: String + } type Mutation { createBook: Book } @@ -120,13 +123,16 @@ class SchemaInspectorTests { name: String } """; - SchemaInspector.Report report = inspectSchema(schema, EmptyController.class); + SchemaMappingInspector.Report report = inspectSchema(schema, GreetingController.class); assertThatReport(report).hasSize(1).missesOperations("Mutation", "createBook"); } @Test void reportIsEmptyWhenMutationMapping() { String schema = """ + type Query{ + greeting: String + } type Mutation { createBook: Book } @@ -136,13 +142,16 @@ class SchemaInspectorTests { name: String } """; - SchemaInspector.Report report = inspectSchema(schema, BookController.class); + SchemaMappingInspector.Report report = inspectSchema(schema, GreetingController.class, BookController.class); assertThatReport(report).isEmpty(); } @Test void inspectExtensionTypesForMutations() { String schema = """ + type Query { + greeting: String + } type Mutation { } extend type Mutation { @@ -153,7 +162,7 @@ class SchemaInspectorTests { name: String } """; - SchemaInspector.Report report = inspectSchema(schema, EmptyController.class); + SchemaMappingInspector.Report report = inspectSchema(schema, GreetingController.class); assertThatReport(report).hasSize(1).missesOperations("Mutation", "createBook"); } @@ -164,6 +173,9 @@ class SchemaInspectorTests { @Test void hasMissingOperationEntryWhenMissingSubscriptionMapping() { String schema = """ + type Query{ + greeting: String + } type Subscription { bookSearch(author: String) : Book! } @@ -173,13 +185,16 @@ class SchemaInspectorTests { name: String } """; - SchemaInspector.Report report = inspectSchema(schema, EmptyController.class); + SchemaMappingInspector.Report report = inspectSchema(schema, GreetingController.class); assertThatReport(report).hasSize(1).missesOperations("Subscription", "bookSearch"); } @Test void reportIsEmptyWhenSubscriptionMapping() { String schema = """ + type Query{ + greeting: String + } type Subscription { bookSearch(author: String) : Book! } @@ -189,13 +204,16 @@ class SchemaInspectorTests { name: String } """; - SchemaInspector.Report report = inspectSchema(schema, BookController.class); + SchemaMappingInspector.Report report = inspectSchema(schema, GreetingController.class, BookController.class); assertThatReport(report).isEmpty(); } @Test void inspectExtensionTypesForSubscriptions() { String schema = """ + type Query{ + greeting: String + } type Subscription { } extend type Subscription { @@ -206,7 +224,7 @@ class SchemaInspectorTests { name: String } """; - SchemaInspector.Report report = inspectSchema(schema, EmptyController.class); + SchemaMappingInspector.Report report = inspectSchema(schema, GreetingController.class); assertThatReport(report).hasSize(1).missesOperations("Subscription", "bookSearch"); } @@ -226,7 +244,7 @@ class SchemaInspectorTests { name: String } """; - SchemaInspector.Report report = inspectSchema(schema, BookController.class); + SchemaMappingInspector.Report report = inspectSchema(schema, BookController.class); assertThatReport(report).isEmpty(); } @@ -243,7 +261,7 @@ class SchemaInspectorTests { fetcher: String } """; - SchemaInspector.Report report = inspectSchema(schema, BookController.class); + SchemaMappingInspector.Report report = inspectSchema(schema, BookController.class); assertThatReport(report).isEmpty(); } @@ -260,7 +278,7 @@ class SchemaInspectorTests { missing: Boolean } """; - SchemaInspector.Report report = inspectSchema(schema, BookController.class); + SchemaMappingInspector.Report report = inspectSchema(schema, BookController.class); assertThatReport(report).hasSize(1).missesFields("Book", "missing"); } @@ -283,7 +301,7 @@ class SchemaInspectorTests { missing: String } """; - SchemaInspector.Report report = inspectSchema(schema, BookController.class); + SchemaMappingInspector.Report report = inspectSchema(schema, BookController.class); assertThatReport(report).hasSize(1).missesFields("Author", "missing"); } @@ -304,7 +322,7 @@ class SchemaInspectorTests { team: Team } """; - SchemaInspector.Report report = inspectSchema(schema, TeamController.class); + SchemaMappingInspector.Report report = inspectSchema(schema, TeamController.class); assertThatReport(report).isEmpty(); } @@ -323,29 +341,10 @@ class SchemaInspectorTests { missing: Boolean } """; - SchemaInspector.Report report = inspectSchema(schema, BookController.class); + SchemaMappingInspector.Report report = inspectSchema(schema, BookController.class); assertThatReport(report).hasSize(1).missesFields("Book", "missing"); } - @Test - void hasMissingFieldEntryWhenMissingPropertyOnTypeProvidedByInterface() { - String schema = """ - type Query { - bookById(id: ID): Book - } - - interface LibraryItem { - missing: Boolean - } - type Book implements LibraryItem { - id: ID - name: String - } - """; - SchemaInspector.Report report = inspectSchema(schema, BookController.class); - assertThatReport(report).hasSize(1).missesFields("LibraryItem", "missing"); - } - } @Nested @@ -358,11 +357,8 @@ class SchemaInspectorTests { greeting: String } """; - SchemaInspector.Report report = inspectSchema(schema, EmptyController.class); - assertThat(report.getSummary()).isEqualTo("GraphQL schema inspection found missing mappings for [Query], no missing data fetchers for inspected types."); - assertThat(report.getDetailedReport()).isEqualTo(""" - - on Query: [greeting] - """); + SchemaMappingInspector.Report report = inspectSchema(schema, EmptyController.class); + assertThat(report.getSummary()).isEqualTo("GraphQL schema inspection found missing mappings for: Query[greeting]."); } @Test @@ -378,11 +374,8 @@ class SchemaInspectorTests { missing: Boolean } """; - SchemaInspector.Report report = inspectSchema(schema, BookController.class); - assertThat(report.getSummary()).isEqualTo("GraphQL schema inspection found no missing mappings for operations, missing data fetchers for types [Book]."); - assertThat(report.getDetailedReport()).isEqualTo(""" - - on Book: [missing] - """); + SchemaMappingInspector.Report report = inspectSchema(schema, BookController.class); + assertThat(report.getSummary()).isEqualTo("GraphQL schema inspection found missing mappings for: Book[missing]."); } } @@ -463,18 +456,12 @@ class SchemaInspectorTests { } - SchemaInspector.Report inspectSchema(String schema, Class... controllers) { - TypeDefinitionRegistry typeDefinitionRegistry = loadTypeDefinitionRegistryFromSchema(schema); + SchemaMappingInspector.Report inspectSchema(String schemaContent, Class... controllers) { + GraphQLSchema schema = SchemaGenerator.createdMockedSchema(schemaContent); RuntimeWiring.Builder builder = createRuntimeWiring(controllers); - return new SchemaInspector().inspectSchema(typeDefinitionRegistry, builder.build()); + return new SchemaMappingInspector().inspectSchemaMappings(schema, builder.build()); } - - TypeDefinitionRegistry loadTypeDefinitionRegistryFromSchema(String schema) { - return new SchemaParser().parse(schema); - } - - RuntimeWiring.Builder createRuntimeWiring(Class... handlerTypes) { AnnotationConfigApplicationContext appContext = new AnnotationConfigApplicationContext(); for (Class handlerType : handlerTypes) { @@ -491,13 +478,13 @@ class SchemaInspectorTests { return wiringBuilder; } - static SchemaInspectionReportAssert assertThatReport(SchemaInspector.Report actual) { + static SchemaInspectionReportAssert assertThatReport(SchemaMappingInspector.Report actual) { return new SchemaInspectionReportAssert(actual); } - static class SchemaInspectionReportAssert extends AbstractAssert { + static class SchemaInspectionReportAssert extends AbstractAssert { - public SchemaInspectionReportAssert(SchemaInspector.Report actual) { + public SchemaInspectionReportAssert(SchemaMappingInspector.Report actual) { super(actual, SchemaInspectionReportAssert.class); }