SchemaMappingInspector detects unmapped DataFetcher's

Closes gh-671

Author: rstoyanchev

spring-graphql-docs/src/docs/asciidoc/index.adoc

@@ -418,21 +418,25 @@ controllers, the return type is derived from the declared return type on a
 `@SchemaMapping` method.
 
 On startup, Spring for GraphQL inspects all schema fields, `DataFetcher` registrations,
-and the properties of Java objects returned from `DataFetcher` implementations in order
-to ensure that every schema field has either an explicitly registered `DataFetcher`, or
-a matching Java object property. This inspection is performed automatically, and results
-in a report that is always logged on startup at INFO level. For example:
+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. This inspection is
+performed automatically, and results in a report that is logged at INFO level on startup.
+For example:
 
 ----
 GraphQL schema inspection:
     Unmapped fields: {Book=[title], Author[firstName, lastName]} // <1>
-    Skipped types: [BookOrAuthor] // <2>
+    Unmapped DataFetcher registrations: {Book.reviews=BookController#reviews[1 args]} <2>
+    Skipped types: [BookOrAuthor] // <3>
 ----
 
 <1> List of schema fields and their source types that are not mapped
-<2> List of schema types that are skipped, as explained next
+<2> List of `DataFetcher` registrations on fields that don't exist
+<3> List of schema types that are skipped, as explained next
 
-There are limits to what schema mappings inspection can do, in particular when there is
+There are limits to what schema field 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

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

@@ -640,6 +640,11 @@ static class SchemaMappingDataFetcher implements SelfDescribingDataFetcher<Objec
 			this.subscription = this.info.getCoordinates().getTypeName().equalsIgnoreCase("Subscription");
 		}
 
+		@Override
+		public String getDescription() {
+			return this.info.getHandlerMethod().getShortLogMessage();
+		}
+
 		@Override
 		public ResolvableType getReturnType() {
 			return ResolvableType.forMethodReturnType(this.info.getHandlerMethod().getMethod());
@@ -702,6 +707,11 @@ private <T> Publisher<T> handleSubscriptionError(
 					.switchIfEmpty(Mono.error(ex));
 		}
 
+		@Override
+		public String toString() {
+			return getDescription();
+		}
+
 	}
 
 

spring-graphql/src/main/java/org/springframework/graphql/data/query/QueryByExampleDataFetcher.java

@@ -120,6 +120,15 @@ public abstract class QueryByExampleDataFetcher<T> {
 	}
 
 
+	/**
+	 * Provides shared implementation of
+	 * {@link SelfDescribingDataFetcher#getDescription()} for all subclasses.
+	 * @since 1.2.0
+	 */
+	public String getDescription() {
+		return "QueryByExampleDataFetcher<" + this.domainType.getType().getName() + ">";
+	}
+
 	/**
 	 * Prepare an {@link Example} from GraphQL request arguments.
 	 * @param environment contextual info for the GraphQL request
@@ -170,6 +179,11 @@ protected ScrollSubrange buildScrollSubrange(DataFetchingEnvironment environment
 		return RepositoryUtils.buildScrollSubrange(environment, this.cursorStrategy);
 	}
 
+	@Override
+	public String toString() {
+		return getDescription();
+	}
+
 
 	/**
 	 * Create a new {@link Builder} accepting {@link QueryByExampleExecutor}

spring-graphql/src/main/java/org/springframework/graphql/data/query/QuerydslDataFetcher.java

@@ -134,6 +134,15 @@ public abstract class QuerydslDataFetcher<T> {
 	}
 
 
+	/**
+	 * Provides shared implementation of
+	 * {@link SelfDescribingDataFetcher#getDescription()} for all subclasses.
+	 * @since 1.2.0
+	 */
+	public String getDescription() {
+		return "QuerydslDataFetcher<" + this.domainType.getType().getName() + ">";
+	}
+
 	/**
 	 * Prepare a {@link Predicate} from GraphQL request arguments, also applying
 	 * any {@link QuerydslBinderCustomizer} that may have been configured.
@@ -194,6 +203,11 @@ protected ScrollSubrange buildScrollSubrange(DataFetchingEnvironment environment
 		return RepositoryUtils.buildScrollSubrange(environment, this.cursorStrategy);
 	}
 
+	@Override
+	public String toString() {
+		return getDescription();
+	}
+
 
 	/**
 	 * Create a new {@link Builder} accepting {@link QuerydslPredicateExecutor}

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

@@ -17,11 +17,13 @@
 package org.springframework.graphql.execution;
 
 import java.util.HashSet;
+import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.Map;
 import java.util.Set;
 
 import graphql.schema.DataFetcher;
+import graphql.schema.FieldCoordinates;
 import graphql.schema.GraphQLEnumType;
 import graphql.schema.GraphQLFieldDefinition;
 import graphql.schema.GraphQLFieldsContainer;
@@ -51,9 +53,11 @@
  * Declares an {@link #inspect(GraphQLSchema, RuntimeWiring)} method that checks
  * if schema fields are covered either by a {@link DataFetcher} registration,
  * or match a Java object property. Fields that have neither are reported as
- * "unmapped" in the resulting {@link Report Resport}.
+ * "unmapped" in the resulting {@link Report Resport}. The inspection also
+ * performs a reverse check for {@code DataFetcher} registrations against schema
+ * fields that don't exist.
  *
- * <p>The inspection depends on {@code DataFetcher}s to be
+ * <p>The schema field inspection depends on {@code DataFetcher}s to be
  * {@link SelfDescribingDataFetcher} to be able 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
@@ -108,21 +112,23 @@ private SchemaMappingInspector(GraphQLSchema schema, RuntimeWiring runtimeWiring
 	 */
 	public Report inspect() {
 
-		inspectType(this.schema.getQueryType(), null);
+		inspectSchemaType(this.schema.getQueryType(), null);
 
 		if (this.schema.isSupportingMutations()) {
-			inspectType(this.schema.getMutationType(), null);
+			inspectSchemaType(this.schema.getMutationType(), null);
 		}
 
 		if (this.schema.isSupportingSubscriptions()) {
-			inspectType(this.schema.getSubscriptionType(), null);
+			inspectSchemaType(this.schema.getSubscriptionType(), null);
 		}
 
+		inspectDataFetcherRegistrations();
+
 		return this.reportBuilder.build();
 	}
 
 	@SuppressWarnings("rawtypes")
-	private void inspectType(GraphQLType type, @Nullable ResolvableType resolvableType) {
+	private void inspectSchemaType(GraphQLType type, @Nullable ResolvableType resolvableType) {
 		Assert.notNull(type, "No GraphQLType");
 
 		type = unwrapNonNull(type);
@@ -166,9 +172,9 @@ else if (resolvableType != null && resolveClassToCompare(resolvableType) == Obje
 		for (GraphQLFieldDefinition field : fieldContainer.getFieldDefinitions()) {
 			String fieldName = field.getName();
 			if (dataFetcherMap.containsKey(fieldName)) {
-				DataFetcher fetcher = dataFetcherMap.get(fieldName);
+				DataFetcher<?> fetcher = dataFetcherMap.get(fieldName);
 				if (fetcher instanceof SelfDescribingDataFetcher<?> selfDescribingDataFetcher) {
-					inspectType(field.getType(), selfDescribingDataFetcher.getReturnType());
+					inspectSchemaType(field.getType(), selfDescribingDataFetcher.getReturnType());
 				}
 				else if (isNotScalarOrEnumType(field.getType())) {
 					if (logger.isDebugEnabled()) {
@@ -233,6 +239,17 @@ private Class<?> resolveClassToCompare(ResolvableType resolvableType) {
 		return (adapter != null ? resolvableType.getNested(2).resolve(Object.class) : clazz);
 	}
 
+	@SuppressWarnings("rawtypes")
+	private void inspectDataFetcherRegistrations() {
+		this.runtimeWiring.getDataFetchers().forEach((typeName, registrations) ->
+				registrations.forEach((fieldName, fetcher) -> {
+					FieldCoordinates coordinates = FieldCoordinates.coordinates(typeName, fieldName);
+					if (this.schema.getFieldDefinition(coordinates) == null) {
+						this.reportBuilder.addUnmappedDataFetcher(coordinates, fetcher);
+					}
+				}));
+	}
+
 
 	/**
 	 * Check the schema against {@code DataFetcher} registrations, and produce a report.
@@ -249,15 +266,20 @@ public static Report inspect(GraphQLSchema schema, RuntimeWiring runtimeWiring)
 
 	/**
 	 * The report produced as a result of schema mappings inspection.
-	 * @param unmappedFields a map with type names as keys, and unmapped field names as values
+	 * @param unmappedFields map with type names as keys, and unmapped field names as values
+	 * @param unmappedDataFetchers map with unmapped {@code DataFetcher}s and their field coordinates
 	 * @param skippedTypes the names of types skipped by the inspection
 	 */
-	public record Report(MultiValueMap<String, String> unmappedFields, Set<String> skippedTypes) {
+	public record Report(
+			MultiValueMap<String, String> unmappedFields,
+			Map<FieldCoordinates, DataFetcher<?>> unmappedDataFetchers,
+			Set<String> skippedTypes) {
 
 		@Override
 		public String toString() {
 			return "GraphQL schema inspection:\n" +
 					"\tUnmapped fields: " + this.unmappedFields + "\n" +
+					"\tUnmapped DataFetcher registrations: " + this.unmappedDataFetchers + "\n" +
 					"\tSkipped types: " + this.skippedTypes;
 		}
 	}
@@ -270,6 +292,8 @@ private static class ReportBuilder {
 
 		private final MultiValueMap<String, String> unmappedFields = new LinkedMultiValueMap<>();
 
+		private final Map<FieldCoordinates, DataFetcher<?>> unmappedDataFetchers = new LinkedHashMap<>();
+
 		private final Set<String> skippedTypes = new LinkedHashSet<>();
 
 		/**
@@ -279,6 +303,13 @@ public void addUnmappedField(String typeName, String fieldName) {
 			this.unmappedFields.add(typeName, fieldName);
 		}
 
+		/**
+		 * Add an unmapped {@code DataFetcher} registration.
+		 */
+		public void addUnmappedDataFetcher(FieldCoordinates coordinates, DataFetcher<?> dataFetcher) {
+			this.unmappedDataFetchers.put(coordinates, dataFetcher);
+		}
+
 		/**
 		 * Add a skipped type name.
 		 */
@@ -289,6 +320,7 @@ public void addSkippedType(String typeName) {
 		public Report build() {
 			return new Report(
 					new LinkedMultiValueMap<>(this.unmappedFields),
+					new LinkedHashMap<>(this.unmappedDataFetchers),
 					new LinkedHashSet<>(this.skippedTypes));
 		}
 

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

@@ -30,6 +30,14 @@
  */
 public interface SelfDescribingDataFetcher<T> extends DataFetcher<T> {
 
+	/**
+	 * Provide a description of the {@code DataFetcher} for display or logging
+	 * purposes. Depending on the underlying implementation, this could be a
+	 * controller method, a Spring Data repository backed {@code DataFetcher},
+	 * or other.
+	 */
+	String getDescription();
+
 	/**
 	 * The return type of this {@link DataFetcher}.
 	 * <p>This could be derived from the method signature of an annotated