diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt index eec6d8fa..83196e79 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt @@ -58,7 +58,8 @@ internal class SchemaClassScanner( private val objectDefinitionsByName = objectDefinitions.associateBy { it.name } private val interfaceDefinitionsByName = definitionsByName.values.filterIsInstance().associateBy { it.name } - private val fieldResolverScanner = FieldResolverScanner(options) + private val scalarNames = ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS.keys + scalarDefinitions.map { it.name } + private val fieldResolverScanner = FieldResolverScanner(options, scalarNames) private val typeClassMatcher = TypeClassMatcher(definitionsByName) private val dictionary = mutableMapOf, DictionaryEntry>() private val unvalidatedTypes = mutableSetOf>() diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt index d2e11815..25935a63 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt @@ -21,7 +21,7 @@ import kotlin.reflect.jvm.kotlinFunction /** * @author Andrew Potter */ -internal class FieldResolverScanner(val options: SchemaParserOptions) { +internal class FieldResolverScanner(val options: SchemaParserOptions, private val scalarNames: Set) { private val log = LoggerFactory.getLogger(javaClass) @@ -45,7 +45,7 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) { private fun findFieldResolver(field: FieldDefinition, search: Search, scanProperties: Boolean): FieldResolver? { val method = findResolverMethod(field, search) if (method != null) { - return MethodFieldResolver(field, search, options, method.apply(trySetAccessible(field, search.type))) + return MethodFieldResolver(field, search, options, method.apply(trySetAccessible(field, search.type)), scalarNames) } if (scanProperties) { diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt index b936b763..d9d0229d 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt @@ -15,7 +15,6 @@ import graphql.language.* import graphql.schema.DataFetcher import graphql.schema.DataFetchingEnvironment import graphql.schema.GraphQLFieldDefinition -import graphql.schema.GraphQLTypeUtil.isScalar import graphql.schema.LightDataFetcher import kotlinx.coroutines.CoroutineStart import kotlinx.coroutines.ensureActive @@ -40,7 +39,8 @@ internal class MethodFieldResolver( field: FieldDefinition, search: FieldResolverScanner.Search, options: SchemaParserOptions, - val method: Method + val method: Method, + private val scalarNames: Set ) : FieldResolver(field, search, options) { private val log = LoggerFactory.getLogger(javaClass) @@ -78,6 +78,8 @@ internal class MethodFieldResolver( val isNonNull = definition.type is NonNullType val isOptional = this.typeResolver.getRawClass(parameterType) == Optional::class.java val argumentConverter = ArgumentConverter(mapper, parameterType) + val parameterClass = parameterType.unwrap() + val isConcreteScalar = isConcreteScalarType(definition.type, parameterType) args.add { environment -> val argumentPresent = environment.arguments.containsKey(definition.name) @@ -94,7 +96,7 @@ internal class MethodFieldResolver( return@add Optional.empty() } - if (value == null || shouldValueBeConverted(value, definition, parameterType, environment)) { + if (value == null || !isConcreteScalar || !parameterClass.isInstance(value)) { return@add argumentConverter.convert(value) } @@ -141,24 +143,18 @@ internal class MethodFieldResolver( } } - private fun shouldValueBeConverted(value: Any, definition: InputValueDefinition, parameterType: JavaType, environment: DataFetchingEnvironment): Boolean { - return !parameterType.unwrap().isAssignableFrom(value.javaClass) || !isConcreteScalarType(environment, definition.type, parameterType) - } - /** * A concrete scalar type is a scalar type where values always coerce to the same Java type. The ID scalar type is not concrete * because values can be coerced to multiple different Java types (eg. String, Long, UUID). All values of a non-concrete scalar * type must be converted to the target method parameter type. */ - private fun isConcreteScalarType(environment: DataFetchingEnvironment, type: Type<*>, genericParameterType: JavaType): Boolean { + private fun isConcreteScalarType(type: Type<*>, genericParameterType: JavaType): Boolean { return when (type) { - is ListType -> List::class.java.isAssignableFrom(this.typeResolver.getRawClass(genericParameterType)) - && isConcreteScalarType(environment, type.type, this.typeResolver.resolve(genericParameterType)) - - is TypeName -> environment.graphQLSchema.getType(type.name!!)?.let { isScalar(it) && type.name != "ID" } - ?: false + is ListType -> genericParameterType.typeArgument(List::class.java) + ?.let { isConcreteScalarType(type.type, it) } ?: false - is NonNullType -> isConcreteScalarType(environment, type.type, genericParameterType) + is TypeName -> type.name in scalarNames && type.name != "ID" + is NonNullType -> isConcreteScalarType(type.type, genericParameterType) else -> false } } diff --git a/src/test/kotlin/graphql/kickstart/tools/FieldResolverScannerTest.kt b/src/test/kotlin/graphql/kickstart/tools/FieldResolverScannerTest.kt index 01c83fb1..c760c2a7 100644 --- a/src/test/kotlin/graphql/kickstart/tools/FieldResolverScannerTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/FieldResolverScannerTest.kt @@ -18,7 +18,7 @@ import java.util.* class FieldResolverScannerTest { private val options = defaultOptions() - private val scanner = FieldResolverScanner(options) + private val scanner = FieldResolverScanner(options, GRAPHQL_SCALARS.keys) @Test fun `scanner finds fields on multiple root types`() { diff --git a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt index 3d8d20d5..2373c0a5 100644 --- a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt @@ -2,7 +2,6 @@ package graphql.kickstart.tools import graphql.ExecutionResult import graphql.GraphQLContext -import graphql.Scalars import graphql.execution.* import graphql.execution.instrumentation.SimplePerformantInstrumentation import graphql.kickstart.tools.resolver.FieldResolverError @@ -14,8 +13,6 @@ import graphql.language.TypeName import graphql.schema.DataFetcher import graphql.schema.DataFetchingEnvironment import graphql.schema.DataFetchingEnvironmentImpl -import graphql.schema.GraphQLObjectType -import graphql.schema.GraphQLSchema import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.Dispatchers @@ -421,7 +418,7 @@ class MethodFieldResolverDataFetcherTest { } else { NormalResolverInfo(resolver, options) } - return FieldResolverScanner(options).findFieldResolver(field, resolverInfo).createDataFetcher() + return FieldResolverScanner(options, GRAPHQL_SCALARS.keys).findFieldResolver(field, resolverInfo).createDataFetcher() } private fun createEnvironment( @@ -446,11 +443,7 @@ class MethodFieldResolverDataFetcherTest { } } val executionId = ExecutionId.from("executionId123") - val schema = GraphQLSchema.newSchema() - .query(GraphQLObjectType.newObject().name("Query").field { it.name("active").type(Scalars.GraphQLBoolean) }) - .build() return ExecutionContextBuilder.newExecutionContextBuilder() - .graphQLSchema(schema) .graphQLContext(context) .instrumentation(SimplePerformantInstrumentation.INSTANCE) .executionId(executionId) diff --git a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt index 39130950..2125997a 100644 --- a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt @@ -4,6 +4,7 @@ import graphql.ExceptionWhileDataFetching import graphql.ExecutionInput import graphql.GraphQL import graphql.GraphQLContext +import graphql.GraphQLError import graphql.execution.CoercedVariables import graphql.language.StringValue import graphql.language.Value @@ -167,6 +168,31 @@ class MethodFieldResolverTest { assertEquals(result.getData(), mapOf("test" to 6)) } + @Test + fun `should handle nested lists of scalar types`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + scalar CustomScalar + type Query { + count(tags: [[String]]): Int + test(input: [[CustomScalar]]): Int + } + """) + .scalars(customScalarType) + .resolvers(object : GraphQLQueryResolver { + fun count(tags: List>) = tags.sumOf { it.size } + fun test(scalars: List>) = scalars.sumOf { set -> set.sumOf { it.value.length } } + }) + .build() + .makeExecutableSchema() + + val result = GraphQL.newGraphQL(schema).build().execute("""{ count(tags: [["a", "a", "b"]]) test(input: [["Foo"], ["Bar"]]) }""") + + assertEquals(result.errors, emptyList()) + assertEquals(result.getData(), mapOf("count" to 2, "test" to 6)) + } + @Test fun `should handle proxies`() { val invocationHandler = object : InvocationHandler { diff --git a/src/test/kotlin/graphql/kickstart/tools/TypeClassMatcherTest.kt b/src/test/kotlin/graphql/kickstart/tools/TypeClassMatcherTest.kt index 4fcb4b34..06868a81 100644 --- a/src/test/kotlin/graphql/kickstart/tools/TypeClassMatcherTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/TypeClassMatcherTest.kt @@ -47,7 +47,7 @@ class TypeClassMatcherTest { ) { x -> x } ).build() - private val scanner: FieldResolverScanner = FieldResolverScanner(options) + private val scanner: FieldResolverScanner = FieldResolverScanner(options, GRAPHQL_SCALARS.keys) private val resolver = RootResolverInfo(listOf(QueryMethods()), options) private fun createPotentialMatch(methodName: String, graphQLType: Type<*>): TypeClassMatcher.PotentialMatch {