From 063b01ae72a9b818c157fb79e8e11f5530ca27b4 Mon Sep 17 00:00:00 2001 From: Oryan Date: Fri, 23 Oct 2026 15:35:42 -0400 Subject: [PATCH] Throw when a resolver method is missing a parameter for an argument --- .../kickstart/tools/SchemaClassScanner.kt | 8 +- .../tools/MethodFieldResolverTest.kt | 102 ++++++++++++------ .../kickstart/tools/SourcelessResolverTest.kt | 38 ++++--- 3 files changed, 91 insertions(+), 57 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt index 7ee3c42a..7012e5f4 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt @@ -6,6 +6,7 @@ import graphql.kickstart.tools.ScanDictionary.InitialDictionaryEntry import graphql.kickstart.tools.ScanDictionary.UnionMemberReference import graphql.kickstart.tools.ScanDictionary.UnusedTypeReference import graphql.kickstart.tools.resolver.FieldResolver +import graphql.kickstart.tools.resolver.FieldResolverError import graphql.kickstart.tools.resolver.FieldResolverScanner import graphql.kickstart.tools.resolver.MethodFieldResolver import graphql.kickstart.tools.resolver.PropertyFieldResolver @@ -457,7 +458,7 @@ internal class SchemaClassScanner( fieldResolver.scanForMatches().forEach { potentialMatch -> dictionary.handleFoundType(typeClassMatcher.match(potentialMatch)) } - warnIfParameterMissing(type, fieldResolver) + requireParameterForEachArgument(type, fieldResolver) } } @@ -482,8 +483,7 @@ internal class SchemaClassScanner( ) } - // TODO throw a FieldResolverError instead in 15.0.0 (#889) - private fun warnIfParameterMissing(type: ObjectTypeDefinition, fieldResolver: FieldResolver) { + private fun requireParameterForEachArgument(type: ObjectTypeDefinition, fieldResolver: FieldResolver) { if (fieldResolver !is MethodFieldResolver) { return } @@ -495,7 +495,7 @@ internal class SchemaClassScanner( } val method = fieldResolver.method - log.warn( + throw FieldResolverError( "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." diff --git a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt index 93b84249..33f49bc2 100644 --- a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt @@ -11,8 +11,12 @@ import graphql.language.Value import graphql.schema.Coercing import graphql.schema.CoercingParseLiteralException import graphql.schema.CoercingSerializeException +import graphql.kickstart.tools.resolver.FieldResolverError +import graphql.kickstart.tools.resolver.MissingResolverDataFetcherProvider +import graphql.schema.DataFetcher import graphql.schema.DataFetchingEnvironment import graphql.schema.GraphQLScalarType +import org.junit.Assert.assertThrows import org.junit.Test import java.lang.reflect.InvocationHandler import java.lang.reflect.Method @@ -311,45 +315,77 @@ 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() - } - + fun `should fail the build when a resolver method is missing a parameter for an argument`() { 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 " + + val messagesByQueryField = mapOf( + "user(id: ID!, active: Boolean!): String" to + "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 " + + "users(active: Boolean, first: Int): String" to + "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 " + + "image: Image" to + "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." - )) + ) + + messagesByQueryField.forEach { (queryField, message) -> + val error = assertThrows(FieldResolverError::class.java) { + buildMissingParameterSchema(queryField, SchemaParserOptions.newOptions()) + } + + assertEquals(error.message, message) + } + } + + @Test + fun `should fail the build for a missing parameter when unimplemented resolvers are allowed`() { + val options = listOf( + SchemaParserOptions.newOptions().allowUnimplementedResolvers(true), + SchemaParserOptions.newOptions().missingResolverDataFetcherProvider(MissingResolverDataFetcherProvider { _, _ -> DataFetcher { "provided" } }) + ) + + options.forEach { + val error = assertThrows(FieldResolverError::class.java) { + buildMissingParameterSchema("users(active: Boolean, first: Int): String", it) + } + + assert("Field 'Query.users' has no parameter for its argument 'active: Boolean'" in error.message!!) { error.message!! } + } + } + + @Test + fun `should build when the context class is an argument type`() { + val schema = buildMissingParameterSchema( + "search(filter: Filter): String\ncount(active: Boolean): String", + SchemaParserOptions.newOptions() + ) + + assertEquals(schema.queryType.fieldDefinitions.map { it.name }, listOf("search", "count")) } + private fun buildMissingParameterSchema(queryField: String, options: SchemaParserOptions.Builder) = + SchemaParser.newParser() + .schemaString( + """ + type Query { + $queryField + } + + type Image { + thumbnail(size: Int): String + } + + input Filter { + name: String + } + """) + .resolvers(MissingParameterQuery(), ImageResolver()) + .options(options.contextClass(Map::class).build()) + .build() + .makeExecutableSchema() + class MissingParameterQuery : GraphQLQueryResolver { fun user(id: String, context: Map) = "$id $context" fun users(context: GraphQLContext?, first: Int?) = "$context $first" diff --git a/src/test/kotlin/graphql/kickstart/tools/SourcelessResolverTest.kt b/src/test/kotlin/graphql/kickstart/tools/SourcelessResolverTest.kt index bd22e8fb..fb2aedca 100644 --- a/src/test/kotlin/graphql/kickstart/tools/SourcelessResolverTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/SourcelessResolverTest.kt @@ -432,29 +432,27 @@ class SourcelessResolverTest { } @Test - fun `no argument warning for a method without the source`() { - val (_, warnings) = withWarnings { - SchemaParser.newParser() - .schemaString( - """ - type Query { - folder: Folder! - } + fun `a method without the source needs no parameter for the source`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + folder: Folder! + } - type Folder { - images: [Image!]! - } + type Folder { + images: [Image!]! + } - type Image { - thumbnail(size: Int!): String! - } - """) - .resolvers(FolderQuery(), FolderResolver(), SourcelessImageResolver()) - .build() - .makeExecutableSchema() - } + type Image { + thumbnail(size: Int!): String! + } + """) + .resolvers(FolderQuery(), FolderResolver(), SourcelessImageResolver()) + .build() + .makeExecutableSchema() - assertEquals(warnings.filter { "has no parameter for its argument" in it }, listOf()) + assertEquals(schema.getObjectType("Image")!!.getFieldDefinition("thumbnail").arguments.map { it.name }, listOf("size")) } @Test