Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -457,7 +458,7 @@ internal class SchemaClassScanner(
fieldResolver.scanForMatches().forEach { potentialMatch ->
dictionary.handleFoundType(typeClassMatcher.match(potentialMatch))
}
warnIfParameterMissing(type, fieldResolver)
requireParameterForEachArgument(type, fieldResolver)
}
}

Expand All @@ -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
}
Expand All @@ -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."
Expand Down
102 changes: 69 additions & 33 deletions src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<String, Any>) = "$id $context"
fun users(context: GraphQLContext?, first: Int?) = "$context $first"
Expand Down
38 changes: 18 additions & 20 deletions src/test/kotlin/graphql/kickstart/tools/SourcelessResolverTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading