diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt index 3d383130..eec6d8fa 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt @@ -9,6 +9,7 @@ import graphql.kickstart.tools.util.* import graphql.language.* import graphql.schema.GraphQLScalarType import graphql.schema.idl.ScalarInfo +import graphql.schema.idl.TypeUtil import org.apache.commons.lang3.ClassUtils import org.apache.commons.lang3.reflect.TypeUtils import org.slf4j.LoggerFactory @@ -528,6 +529,7 @@ internal class SchemaClassScanner( fieldResolver.scanForMatches().forEach { potentialMatch -> handleFoundType(typeClassMatcher.match(potentialMatch)) } + warnIfParameterMissing(type, fieldResolver) } } @@ -552,6 +554,32 @@ internal class SchemaClassScanner( ) } + // TODO throw a FieldResolverError instead in 15.0.0 (#889) + private fun warnIfParameterMissing(type: ObjectTypeDefinition, fieldResolver: FieldResolver) { + if (fieldResolver !is MethodFieldResolver) { + return + } + + fieldResolver.field.inputValueDefinitions.forEachIndexed { index, argument -> + val parameter = fieldResolver.getMethodParameterType(index)?.let(fieldResolver.typeResolver::getRawClass) ?: return@forEachIndexed + if (!isContextParameter(parameter, argument)) { + return@forEachIndexed + } + + val method = fieldResolver.method + log.warn( + "Field '${type.name}.${fieldResolver.field.name}' has no parameter for its argument '${argument.name}: " + + "${TypeUtil.simplePrint(argument.type)}' in ${method.declaringClass.name}.${method.name}, so the ${parameter.simpleName} " + + "parameter gets the argument instead. Add a parameter for each argument, in order, before the ${parameter.simpleName} parameter." + ) + } + } + + // the context class can also be an argument's type (e.g. a Map for an input object), so only spec scalars count for it + private fun isContextParameter(parameter: Class<*>, argument: InputValueDefinition): Boolean = + parameter in fieldResolverScanner.allowedTrailingParameterTypes && + (parameter != options.contextClass || argument.type.unwrap().name in ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS) + private fun handleFoundType(match: TypeClassMatcher.Match) = when (match) { is TypeClassMatcher.ScalarMatch -> handleFoundScalarType(match.type) is TypeClassMatcher.ValidMatch -> handleFoundType(match.type, match.javaType, match.reference) diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt index 0c54fee2..d2e11815 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt @@ -25,7 +25,7 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { private val log = LoggerFactory.getLogger(javaClass) - private val allowedTrailingParameterTypes = listOfNotNull(DataFetchingEnvironment::class.java, GraphQLContext::class.java, options.contextClass) + val allowedTrailingParameterTypes = listOfNotNull(DataFetchingEnvironment::class.java, GraphQLContext::class.java, options.contextClass) private val methodsByNameCache = mutableMapOf, Boolean>, Map>>() diff --git a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt index b630d0da..39130950 100644 --- a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt @@ -10,6 +10,7 @@ import graphql.language.Value import graphql.schema.Coercing import graphql.schema.CoercingParseLiteralException import graphql.schema.CoercingSerializeException +import graphql.schema.DataFetchingEnvironment import graphql.schema.GraphQLScalarType import org.junit.Test import java.lang.reflect.InvocationHandler @@ -283,6 +284,60 @@ class MethodFieldResolverTest { )) } + @Test + fun `should warn when a resolver method is missing a parameter for an argument`() { + val (_, warnings) = withWarnings { + SchemaParser.newParser() + .schemaString( + """ + type Query { + user(id: ID!, active: Boolean!): String + users(active: Boolean, first: Int): String + image: Image + search(filter: Filter): String + count(active: Boolean): String + } + + type Image { + thumbnail(size: Int): String + } + + input Filter { + name: String + } + """) + .resolvers(MissingParameterQuery(), ImageResolver()) + .options(SchemaParserOptions.newOptions().contextClass(Map::class).build()) + .build() + .makeExecutableSchema() + } + + val query = MissingParameterQuery::class.java.name + assertEquals(warnings, listOf( + "Field 'Query.user' has no parameter for its argument 'active: Boolean!' in $query.user, so the Map parameter gets the " + + "argument instead. Add a parameter for each argument, in order, before the Map parameter.", + "Field 'Query.users' has no parameter for its argument 'active: Boolean' in $query.users, so the GraphQLContext parameter " + + "gets the argument instead. Add a parameter for each argument, in order, before the GraphQLContext parameter.", + "Field 'Image.thumbnail' has no parameter for its argument 'size: Int' in ${ImageResolver::class.java.name}.thumbnail, so the " + + "DataFetchingEnvironment parameter gets the argument instead. Add a parameter for each argument, in order, before the " + + "DataFetchingEnvironment parameter." + )) + } + + class MissingParameterQuery : GraphQLQueryResolver { + fun user(id: String, context: Map) = "$id $context" + fun users(context: GraphQLContext?, first: Int?) = "$context $first" + fun image() = Image() + fun search(filter: Map?) = filter.toString() + fun count(active: Boolean?, context: Map) = "$active $context" + } + + class Image + + class ImageResolver : GraphQLResolver { + fun thumbnail(image: Image, env: DataFetchingEnvironment?) = env.toString() + } + sealed class Maybe { data class Some(val value: T) : Maybe() object None : Maybe()