Rework input parsing in MethodFieldResolver to handle scalars within input objects

src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt

@@ -363,8 +363,8 @@ class SchemaParser internal constructor(
     private fun buildDefaultValue(value: Value<*>?): Any? {
         return when (value) {
             null -> null
-            is IntValue -> value.value
-            is FloatValue -> value.value
+            is IntValue -> value.value.toInt()
+            is FloatValue -> value.value.toDouble()
             is StringValue -> value.value
             is EnumValue -> value.name
             is BooleanValue -> value.isValue

src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt

@@ -1,6 +1,6 @@
 package graphql.kickstart.tools.resolver
 
-import com.fasterxml.jackson.core.type.TypeReference
+import com.fasterxml.jackson.annotation.JsonIgnore
 import graphql.TrivialDataFetcher
 import graphql.kickstart.tools.*
 import graphql.kickstart.tools.SchemaParserOptions.GenericWrapper
@@ -9,13 +9,16 @@ import graphql.kickstart.tools.util.coroutineScope
 import graphql.kickstart.tools.util.isTrivialDataFetcher
 import graphql.kickstart.tools.util.unwrap
 import graphql.language.*
-import graphql.schema.DataFetcher
-import graphql.schema.DataFetchingEnvironment
-import graphql.schema.GraphQLTypeUtil.isScalar
+import graphql.schema.*
+import graphql.schema.GraphQLTypeUtil.*
 import kotlinx.coroutines.future.future
+import java.lang.reflect.Field
 import java.lang.reflect.Method
+import java.lang.reflect.ParameterizedType
+import java.lang.reflect.WildcardType
 import java.util.*
 import kotlin.coroutines.intrinsics.suspendCoroutineUninterceptedOrReturn
+import kotlin.reflect.KClass
 import kotlin.reflect.full.valueParameters
 import kotlin.reflect.jvm.javaType
 import kotlin.reflect.jvm.kotlinFunction
@@ -39,7 +42,6 @@ internal class MethodFieldResolver(
 
     override fun createDataFetcher(): DataFetcher<*> {
         val args = mutableListOf<ArgumentPlaceholder>()
-        val mapper = options.objectMapperProvider.provide(field)
 
         // Add source argument if this is a resolver (but not a root resolver)
         if (this.search.requiredFirstParameterType != null) {
@@ -80,13 +82,8 @@ internal class MethodFieldResolver(
                     return@add Optional.empty<Any>()
                 }
 
-                if (value == null || shouldValueBeConverted(value, definition, parameterType, environment)) {
-                    return@add mapper.convertValue(value, object : TypeReference<Any>() {
-                        override fun getType() = parameterType
-                    })
-                }
 
-                return@add value
+                return@add parseInput(value, definition.type, parameterType, environment)
             }
         }
 
@@ -106,23 +103,143 @@ 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)
+    private fun parseInput(
+        value: Any?,
+        type: Type<*>,
+        javaType: JavaType,
+        environment: DataFetchingEnvironment
+    ): Any? {
+        return when (type) {
+            is NonNullType -> parseInput(value, type.type, javaType, environment)
+            is TypeName -> {
+                if (javaType is ParameterizedType && Optional::class.isAssignableFrom(javaType.rawType)) {
+                    // parse and wrap with optional
+                    return Optional.ofNullable(parseInput(value, type, javaType.actualTypeArguments[0], environment))
+                }
+
+                when (val graphQLType = environment.graphQLSchema?.getType(type.name)) {
+                    is GraphQLInputObjectType -> {
+                        if (value == null) return value
+                        return if ((javaType as Class<*>).constructors.any { it.parameters.isEmpty() }) {
+                            parseInputObjectWithNoArgsConstructor(javaType, graphQLType, value, environment)
+                        } else {
+                            parseInputObjectWithAllArgsConstructor(javaType, graphQLType, value, environment)
+                        }
+                    }
+                    is GraphQLScalarType -> when {
+                        type.name == "ID" -> parseIdInput(value, javaType)
+                        javaType is Class<*> && javaType.isPrimitive && value == null -> getPrimitiveDefault(javaType)
+                        else -> value
+                    }
+                    is GraphQLEnumType -> when (value) {
+                        is String -> (javaType as Class<*>)
+                            .getMethod("valueOf", String::class.java)
+                            .invoke(null, value)
+                        else -> value
+                    }
+                    else -> value
+                }
+            }
+            is ListType -> when {
+                value == null -> value
+                javaType is ParameterizedType && Optional::class.isAssignableFrom(javaType.rawType) -> {
+                    // parse and wrap with optional
+                    Optional.ofNullable(parseInput(value, type, javaType.actualTypeArguments[0], environment))
+                }
+                javaType is ParameterizedType && Collection::class.isAssignableFrom(javaType.rawType) -> {
+                    val collection = (value as Collection<*>).map {
+                        parseInput(it, type.type, javaType.actualTypeArguments[0], environment)
+                    }
+
+                    return when {
+                        List::class.isAssignableFrom(javaType.rawType) -> collection.toList()
+                        Set::class.isAssignableFrom(javaType.rawType) -> collection.toSet()
+                        else -> collection
+                    }
+                }
+                javaType is WildcardType -> parseInput(value, type, javaType.upperBounds[0], environment)
+                else -> value
+            }
+            else -> value
+        }
+    }
+
+    private fun parseInputObjectWithAllArgsConstructor(
+        javaType: Class<*>,
+        graphQLType: GraphQLInputObjectType,
+        value: Any?,
+        environment: DataFetchingEnvironment
+    ): Any? {
+        val fields = parseInputObjectFields(javaType, graphQLType, value, environment)
+
+        return javaType
+            .getDeclaredConstructor(*fields.map { it.first.type }.toTypedArray())
+            .newInstance(*fields.map { it.second }.toTypedArray())
     }
 
-    /**
-     * 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 {
-        return when (type) {
-            is ListType -> List::class.java.isAssignableFrom(this.genericType.getRawClass(genericParameterType))
-                && isConcreteScalarType(environment, type.type, this.genericType.unwrapGenericType(genericParameterType))
-            is TypeName -> environment.graphQLSchema?.getType(type.name)?.let { isScalar(it) && type.name != "ID" }
-                ?: false
-            is NonNullType -> isConcreteScalarType(environment, type.type, genericParameterType)
-            else -> false
+    private fun parseInputObjectWithNoArgsConstructor(
+        javaType: Class<*>,
+        graphQLType: GraphQLInputObjectType,
+        value: Any?,
+        environment: DataFetchingEnvironment
+    ): Any? {
+        val inputObject = javaType.getDeclaredConstructor().newInstance()
+
+        parseInputObjectFields(javaType, graphQLType, value, environment)
+            .forEach {
+                val field = it.first
+                field.isAccessible = true
+                field.set(inputObject, it.second)
+            }
+
+        return inputObject
+    }
+
+    private fun parseInputObjectFields(
+        javaType: Class<*>,
+        graphQLType: GraphQLInputObjectType,
+        value: Any?,
+        environment: DataFetchingEnvironment
+    ): List<Pair<Field, Any?>> {
+        return javaType.declaredFields
+            .filterNot { it.isSynthetic }
+            // TODO use an annotation specific to graphql (i.e. GraphQLIgnore?)
+            .filterNot { it.isAnnotationPresent(JsonIgnore::class.java) }
+            .map {
+                val graphQLField = graphQLType.fields.find { t -> t.definition.name == it.name }
+                    ?: throw IllegalArgumentException("Could not construct input object: missing field '${it.name}' in '${graphQLType.name}' ")
+                val fieldValue = (value as Map<*, *>)[graphQLField.definition.name]
+                val parsedValue = parseInput(fieldValue, graphQLField.definition.type, it.genericType, environment)
+                Pair(it, parsedValue)
+            }
+    }
+
+    private fun parseIdInput(value: Any?, javaType: JavaType) = when {
+        value !is String // if value was already coerced
+            || String::class.isAssignableFrom(javaType) -> value
+        Int::class.isAssignableFrom(javaType)
+            || Integer::class.isAssignableFrom(javaType) -> Integer.parseInt(value)
+        Long::class.isAssignableFrom(javaType)
+            || java.lang.Long::class.isAssignableFrom(javaType) -> java.lang.Long.parseLong(value)
+        UUID::class.isAssignableFrom(javaType) -> UUID.fromString(value)
+        else -> value
+    }
+
+    private fun KClass<*>.isAssignableFrom(javaType: JavaType): Boolean {
+        return this.java.isAssignableFrom(javaType.unwrap())
+    }
+
+    private fun getPrimitiveDefault(javaType: Class<*>): Any? {
+        return when (javaType) {
+            Boolean::class.java -> false
+            Byte::class.java -> 0
+            Char::class.java -> '\u0000'
+            Int::class.java -> 0
+            Short::class.java -> 0
+            Long::class.java -> 0L
+            Float::class.java -> 0.0f
+            Double::class.java -> 0.0
+            else -> null
         }
     }
 

src/test/groovy/graphql/kickstart/tools/EndToEndSpec.groovy

@@ -203,6 +203,20 @@ class EndToEndSpec extends Specification {
         (data["echoFiles"] as ArrayList<String>).join(",") == "Hello,World"
     }
 
+    def "generated schema should handle input types with scalar fields"() {
+        when:
+        def part = new MockPart("test.doc", "Hello")
+        def input = [name: "filename", part: part]
+        def args = ["input": input]
+        def data = Utils.assertNoGraphQlErrors(gql, args) {
+            '''
+              mutation ($input: FileInput) { echoFile(input: $input)}
+            '''
+        }
+        then:
+        (data["echoFile"] as String) == "Hello"
+    }
+
     def "generated schema should handle any java.util.Map (using HashMap) types as property maps"() {
         when:
         def data = Utils.assertNoGraphQlErrors(gql) {

src/test/groovy/graphql/kickstart/tools/MethodFieldResolverDataFetcherSpec.groovy

@@ -228,12 +228,25 @@ class MethodFieldResolverDataFetcherSpec extends Specification {
             }
         }
         ExecutionId executionId = ExecutionId.from("executionId123")
+
+        def schema = SchemaParser.newParser()
+                .schemaString("""
+                    type Query { test(input: InputClass): Boolean }
+                    input InputClass {
+                        name: String
+                    }
+                """)
+                .resolvers(new Query())
+                .build()
+                .makeExecutableSchema()
+
         ExecutionContextBuilder.newExecutionContextBuilder()
                 .instrumentation(SimpleInstrumentation.INSTANCE)
                 .executionId(executionId)
                 .queryStrategy(executionStrategy)
                 .mutationStrategy(executionStrategy)
                 .subscriptionStrategy(executionStrategy)
+                .graphQLSchema(schema)
                 .build()
     }
 
@@ -251,4 +264,10 @@ class MethodFieldResolverDataFetcherSpec extends Specification {
 
     class ContextClass {
     }
+
+    static class Query implements GraphQLQueryResolver {
+        static boolean test(InputClass input) {
+            return input != null
+        }
+    }
 }

src/test/kotlin/graphql/kickstart/tools/EndToEndSpecHelper.kt

@@ -110,6 +110,7 @@ input ComplexInputTypeTwo {
 type Mutation {
     addItem(newItem: NewItemInput!): Item!
     echoFiles(fileParts: [Upload!]!): [String!]!
+    echoFile(input: FileInput): String
     saveUser(input: UserInput!): String
 }
 
@@ -127,6 +128,11 @@ extend input UserInput {
     password: String
 }
 
+input FileInput {
+    name: String                        
+    part: Upload                        
+}
+
 input ItemSearchInput {
     # The item name to look for
     name: String!
@@ -370,6 +376,7 @@ class ItemResolver : GraphQLResolver<Item> {
 
 class EchoFilesResolver : GraphQLMutationResolver {
     fun echoFiles(fileParts: List<Part>): List<String> = fileParts.map { String(it.inputStream.readBytes()) }
+    fun echoFile(input: FileInput): String? = input.part?.inputStream?.readBytes()?.let { String(it) }
 }
 
 interface ItemInterface {
@@ -388,6 +395,7 @@ data class NestedComplexMapItem(val item: UndiscoveredItem)
 data class Tag(val id: Int, val name: String)
 data class ItemSearchInput(val name: String)
 data class NewItemInput(val name: String, val type: Type)
+data class FileInput(var name: String = "", var part: Part? = null)
 data class ComplexNullable(val first: String, val second: String, val third: String)
 data class ComplexInputType(val first: String, val second: List<List<ComplexInputTypeTwo>?>?)
 data class ComplexInputTypeTwo(val first: String)

src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt

@@ -22,6 +22,7 @@ class MethodFieldResolverTest {
                         testValue(input: String): String
                         testOmitted(input: String): String
                         testNull(input: String): String
+                        testList(input: [String]): String
                     }
                     """
             )
@@ -30,6 +31,7 @@ class MethodFieldResolverTest {
                 fun testValue(input: Optional<String>) = input.toString()
                 fun testOmitted(input: Optional<String>) = input.toString()
                 fun testNull(input: Optional<String>) = input.toString()
+                fun testList(input: Optional<List<String>>) = input.toString()
             })
             .build()
             .makeExecutableSchema()
@@ -38,11 +40,13 @@ class MethodFieldResolverTest {
 
         val result = gql
             .execute(ExecutionInput.newExecutionInput()
-                .query("""
+                .query(
+                    """
                             query {
                                 testValue(input: "test-value")
                                 testOmitted
                                 testNull(input: null)
+                                testList(input: ["list", "value"])
                             }
                             """)
                 .context(Object())
@@ -51,7 +55,8 @@ class MethodFieldResolverTest {
         val expected = mapOf(
             "testValue" to "Optional[test-value]",
             "testOmitted" to "Optional.empty",
-            "testNull" to "Optional.empty"
+            "testNull" to "Optional.empty",
+            "testList" to "Optional[[list, value]]"
         )
 
         Assert.assertEquals(expected, result.getData())