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
28 changes: 28 additions & 0 deletions src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
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
Expand Down Expand Up @@ -528,6 +529,7 @@
fieldResolver.scanForMatches().forEach { potentialMatch ->
handleFoundType(typeClassMatcher.match(potentialMatch))
}
warnIfParameterMissing(type, fieldResolver)
}
}

Expand All @@ -552,6 +554,32 @@
)
}

// TODO throw a FieldResolverError instead in 15.0.0 (#889)

Check warning on line 557 in src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Complete the task associated to this TODO comment.

See more on https://sonarcloud.io/project/issues?id=graphql-java-kickstart_graphql-java-tools&issues=AaFVb4QLf42XpjZWyoCw&open=AaFVb4QLf42XpjZWyoCw&pullRequest=890
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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Pair<Class<out Any>, Boolean>, Map<String, List<Method>>>()

Expand Down
55 changes: 55 additions & 0 deletions src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<String, Any>) = "$id $context"
fun users(context: GraphQLContext?, first: Int?) = "$context $first"
fun image() = Image()
fun search(filter: Map<String, Any>?) = filter.toString()
fun count(active: Boolean?, context: Map<String, Any>) = "$active $context"
}

class Image

class ImageResolver : GraphQLResolver<Image> {
fun thumbnail(image: Image, env: DataFetchingEnvironment?) = env.toString()
}

sealed class Maybe<out T> {
data class Some<T>(val value: T) : Maybe<T>()
object None : Maybe<Nothing>()
Expand Down
Loading