Merge pull request #882 from graphql-java-kickstart/bugfix/880

Resolve type variables in resolver parameter types

Author: oryan-block

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

@@ -52,7 +52,7 @@ internal class GenericTypeResolver(private val containingType: JavaType, private
         }
     }
 
-    private fun resolveTypeVariables(type: JavaType, resolving: Set<TypeVariable<*>> = emptySet()): JavaType {
+    fun resolveTypeVariables(type: JavaType, resolving: Set<TypeVariable<*>> = emptySet()): JavaType {
         return when (type) {
             is ParameterizedType -> {
                 val actualTypeArguments = type.actualTypeArguments.map { resolveTypeVariables(it, resolving) }.toTypedArray()

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

@@ -74,7 +74,6 @@ internal class MethodFieldResolver(
         this.field.inputValueDefinitions.forEachIndexed { index, definition ->
 
             val parameterType = this.getMethodParameterType(index)
-                ?.apply { typeResolver.getRawClass(this) }
                 ?: throw ResolverError("Missing method type at position ${this.getJavaMethodParameterIndex(index)}, this is most likely a bug with graphql-java-tools")
 
             val isNonNull = definition.type is NonNullType
@@ -206,7 +205,7 @@ internal class MethodFieldResolver(
         val parameters = method.parameterTypes
 
         return if (parameters.size > methodIndex) {
-            method.genericParameterTypes[methodIndex]
+            typeResolver.resolveTypeVariables(method.genericParameterTypes[methodIndex])
         } else {
             null
         }

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

@@ -70,6 +70,48 @@ class GenericResolverTest {
 
     class CarResolver : FooGraphQLResolver<Car>()
 
+    @Test
+    fun `inherited methods taking a type variable of their resolver are resolved`() {
+        val schema = SchemaParser.newParser()
+            .schemaString(
+                """
+                type Query {
+                    generic(input: GreetingInput!): String!
+                    plain(input: GreetingInput!): String!
+                }
+
+                input GreetingInput {
+                    name: String!
+                }
+                """)
+            .resolvers(GreetingQuery())
+            .build()
+            .makeExecutableSchema()
+            .assertEveryFieldResolves()
+        val gql = GraphQL.newGraphQL(schema).build()
+
+        val data = assertNoGraphQlErrors(gql) {
+            """
+            query {
+                generic(input: { name: "a" })
+                plain(input: { name: "b" })
+            }
+            """
+        }
+
+        assertEquals(data, mapOf("generic" to "generic got GreetingInput(name=a)", "plain" to "plain got GreetingInput(name=b)"))
+    }
+
+    data class GreetingInput(val name: String)
+
+    abstract class InputResolver<I> : GraphQLQueryResolver {
+        fun generic(input: I): String = "generic got $input"
+    }
+
+    class GreetingQuery : InputResolver<GreetingInput>() {
+        fun plain(input: GreetingInput): String = "plain got $input"
+    }
+
     @Test
     fun `star projected resolvers are applied to parameterized data classes`() {
         val gql = GraphQL.newGraphQL(pageSchema(PageResolver())).build()

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

@@ -328,7 +328,7 @@ class UnmappedInputFieldsTest {
     }
 
     @Test
-    fun `arguments are matched to their parameters past sources, environments and continuations`() {
+    fun `arguments are matched to their parameters past sources, environments and continuations, and through type variables`() {
         val error = assertThrows(SchemaError::class.java) {
             SchemaParser.newParser()
                 .schemaString(
@@ -337,6 +337,7 @@ class UnmappedInputFieldsTest {
                         owner: Owner
                         withEnvironment(input: EnvironmentInput!): String
                         suspending(input: SuspendInput!): String
+                        generic(input: GenericInput!): String
                     }
 
                     type Owner {
@@ -354,6 +355,10 @@ class UnmappedInputFieldsTest {
                     input ChildInput {
                         name: String
                     }
+
+                    input GenericInput {
+                        name: String
+                    }
                     """)
                 .resolvers(object : GraphQLQueryResolver {
                     fun owner(): Owner = Owner()
@@ -362,7 +367,7 @@ class UnmappedInputFieldsTest {
                     suspend fun suspending(input: SuspendInput): String? = input.nmae
                 }, object : GraphQLResolver<Owner> {
                     fun child(owner: Owner, id: String, input: ChildInput): String? = input.nmae
-                })
+                }, GenericInputResolver())
                 .options(failOptions().build())
                 .build()
                 .makeExecutableSchema()
@@ -371,7 +376,8 @@ class UnmappedInputFieldsTest {
         assertEquals(error.message, listOf(
             unmapped("ChildInput.name", ChildInput::class.java, "argument 'input' of field 'Owner.child'", "nmae"),
             unmapped("EnvironmentInput.name", EnvironmentInput::class.java, "argument 'input' of field 'Query.withEnvironment'", "nmae"),
-            unmapped("SuspendInput.name", SuspendInput::class.java, "argument 'input' of field 'Query.suspending'", "nmae")
+            unmapped("SuspendInput.name", SuspendInput::class.java, "argument 'input' of field 'Query.suspending'", "nmae"),
+            unmapped("GenericInput.name", GenericInput::class.java, "argument 'input' of field 'Query.generic'", "nmae")
         ).joinToString("\n"))
     }
 
@@ -690,7 +696,6 @@ class UnmappedInputFieldsTest {
                     injected(input: InjectedInput!): String
                     builder(input: BuilderTypoInput!): String
                     unwrapped(input: KotlinUnwrappedInput!): String
-                    generic(input: GenericInput!): String
                 }
 
                 input InjectedInput {
@@ -705,27 +710,19 @@ class UnmappedInputFieldsTest {
                     name: String
                     street: String
                 }
-
-                input GenericInput {
-                    name: String
-                }
                 """)
             .resolvers(object : GraphQLQueryResolver {
                 fun injected(input: InjectedInput): String? = input.nmae
                 fun builder(input: BuilderTypoInput): String? = input.name
                 fun unwrapped(input: KotlinUnwrappedInput): String = "${input.name} ${input.address?.stret}"
-            }, GenericInputResolver())
+            })
             .options(failOptions()
                 .objectMapperConfigurer { mapper, _ -> mapper.setInjectableValues(InjectableValues.Std().addValue("tenant", "acme")) }
                 .build())
             .build()
             .makeExecutableSchema()
-            .assertEveryFieldResolves(except = setOf(
-                // the input classes misspell name on purpose
-                "Query.injected", "Query.builder",
-                // TODO query generic once the type variable of its inherited parameter is resolved (#880)
-                "Query.generic"
-            ))
+            // the input classes misspell name on purpose
+            .assertEveryFieldResolves(except = setOf("Query.injected", "Query.builder"))
 
         val result = GraphQL.newGraphQL(schema).build().execute(
             """