Resolve generic superclass methods (fix #163)

Author: oliemansm

src/main/kotlin/com/coxautodev/graphql/tools/FieldResolverScanner.kt

@@ -9,6 +9,7 @@ import org.apache.commons.lang3.reflect.FieldUtils
 import org.slf4j.LoggerFactory
 import java.lang.reflect.Modifier
 import java.lang.reflect.ParameterizedType
+import java.lang.reflect.TypeVariable
 
 /**
  * @author Andrew Potter
@@ -22,11 +23,13 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) {
 
         fun getAllMethods(type: Class<*>) =
                 (type.methods.toList() + ClassUtils.getAllSuperclasses(type).flatMap { it.methods.toList() })
-                    .filter { !it.isSynthetic }
-                    .filter { !Modifier.isPrivate(it.modifiers) }
-                    // discard any methods that are coming off the root of the class hierarchy
-                    // to avoid issues with duplicate method declarations
-                    .filter { it.declaringClass != Object::class.java }
+                        .asSequence()
+                        .filter { !it.isSynthetic }
+                        .filter { !Modifier.isPrivate(it.modifiers) }
+                        // discard any methods that are coming off the root of the class hierarchy
+                        // to avoid issues with duplicate method declarations
+                        .filter { it.declaringClass != Object::class.java }
+                        .toList()
     }
 
     fun findFieldResolver(field: FieldDefinition, resolverInfo: ResolverInfo): FieldResolver {
@@ -96,10 +99,12 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) {
 
     private fun verifyMethodArguments(method: java.lang.reflect.Method, requiredCount: Int, search: Search): Boolean {
         val appropriateFirstParameter = if (search.requiredFirstParameterType != null) {
-            if(MethodFieldResolver.isBatched(method, search)) {
+            if (MethodFieldResolver.isBatched(method, search)) {
                 verifyBatchedMethodFirstArgument(method.genericParameterTypes.firstOrNull(), search.requiredFirstParameterType)
             } else {
-                method.parameterTypes.firstOrNull() == search.requiredFirstParameterType
+                method.genericParameterTypes.firstOrNull()?.let {
+                    it == search.requiredFirstParameterType || method.declaringClass.typeParameters.contains(it)
+                } ?: false
             }
         } else {
             true
@@ -110,15 +115,15 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) {
     }
 
     private fun verifyBatchedMethodFirstArgument(firstType: JavaType?, requiredFirstParameterType: Class<*>?): Boolean {
-        if(firstType == null) {
+        if (firstType == null) {
             return false
         }
 
-        if(firstType !is ParameterizedType) {
+        if (firstType !is ParameterizedType) {
             return false
         }
 
-        if(!TypeClassMatcher.isListType(firstType, GenericType(firstType, options))) {
+        if (!TypeClassMatcher.isListType(firstType, GenericType(firstType, options))) {
             return false
         }
 
@@ -128,7 +133,7 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) {
     }
 
     private fun findResolverProperty(field: FieldDefinition, search: Search) =
-        FieldUtils.getAllFields(search.type).find { it.name == field.name }
+            FieldUtils.getAllFields(search.type).find { it.name == field.name }
 
     private fun getMissingFieldMessage(field: FieldDefinition, searches: List<Search>, scannedProperties: Boolean): String {
         val signatures = mutableListOf("")
@@ -138,7 +143,8 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) {
             signatures.addAll(getMissingMethodSignatures(field, search, isBoolean, scannedProperties))
         }
 
-        val sourceLocation = if (field.sourceLocation != null) "${field.sourceLocation.sourceName}:${field.sourceLocation.line}" else "<unknown>"
+        val sourceName = if (field.sourceLocation != null && field.sourceLocation.sourceName != null) field.sourceLocation.sourceName else "<unknown>"
+        val sourceLocation = if (field.sourceLocation != null) "$sourceName:${field.sourceLocation.line}" else "<unknown>"
         return "No method${if (scannedProperties) " or field" else ""} found as defined in $sourceLocation with any of the following signatures (with or without one of $allowedLastArgumentTypes as the last argument), in priority order:\n${signatures.joinToString("\n  ")}"
     }
 
@@ -171,4 +177,4 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) {
     data class Search(val type: Class<*>, val resolverInfo: ResolverInfo, val source: Any?, val requiredFirstParameterType: Class<*>? = null, val allowBatched: Boolean = false)
 }
 
-class FieldResolverError(msg: String): RuntimeException(msg)
+class FieldResolverError(msg: String) : RuntimeException(msg)

src/main/kotlin/com/coxautodev/graphql/tools/ResolverInfo.kt

@@ -7,27 +7,27 @@ internal abstract class ResolverInfo {
     abstract fun getFieldSearches(): List<FieldResolverScanner.Search>
 
     fun getRealResolverClass(resolver: GraphQLResolver<*>, options: SchemaParserOptions) =
-        options.proxyHandlers.find { it.canHandle(resolver) }?.getTargetClass(resolver) ?: resolver.javaClass
+            options.proxyHandlers.find { it.canHandle(resolver) }?.getTargetClass(resolver) ?: resolver.javaClass
 }
 
-internal class NormalResolverInfo(val resolver: GraphQLResolver<*>, private val options: SchemaParserOptions): ResolverInfo() {
+internal class NormalResolverInfo(val resolver: GraphQLResolver<*>, private val options: SchemaParserOptions) : ResolverInfo() {
     val resolverType = getRealResolverClass(resolver, options)
     val dataClassType = findDataClass()
 
     private fun findDataClass(): Class<*> {
         // Grab the parent interface with type GraphQLResolver from our resolver and get its first type argument.
         val interfaceType = GenericType(resolverType, options).getGenericInterface(GraphQLResolver::class.java)
-        if(interfaceType == null || interfaceType !is ParameterizedType) {
+        if (interfaceType == null || interfaceType !is ParameterizedType) {
             error("${GraphQLResolver::class.java.simpleName} interface was not parameterized for: ${resolverType.name}")
         }
 
         val type = TypeUtils.determineTypeArguments(resolverType, interfaceType)[GraphQLResolver::class.java.typeParameters[0]]
 
-        if(type == null || type !is Class<*>) {
-            throw ResolverError("Unable to determine data class for resolver '${resolverType.name}' from generic interface!  This is most likely a bug with graphql-java-tools.")
+        if (type == null || type !is Class<*>) {
+            throw ResolverError("Unable to determine data class for resolver '${resolverType.name}' from generic interface! This is most likely a bug with graphql-java-tools.")
         }
 
-        if(type == Void::class.java) {
+        if (type == Void::class.java) {
             throw ResolverError("Resolvers may not have ${Void::class.java.name} as their type, use a real type or use a root resolver interface.")
         }
 
@@ -36,13 +36,13 @@ internal class NormalResolverInfo(val resolver: GraphQLResolver<*>, private val
 
     override fun getFieldSearches(): List<FieldResolverScanner.Search> {
         return listOf(
-            FieldResolverScanner.Search(resolverType, this, resolver, dataClassType, true),
-            FieldResolverScanner.Search(dataClassType, this, null)
+                FieldResolverScanner.Search(resolverType, this, resolver, dataClassType, true),
+                FieldResolverScanner.Search(dataClassType, this, null)
         )
     }
 }
 
-internal class MultiResolverInfo(val resolverInfoList: List<NormalResolverInfo>): ResolverInfo() {
+internal class MultiResolverInfo(val resolverInfoList: List<NormalResolverInfo>) : ResolverInfo() {
     private val dataClassType = findDataClass()
 
     /**
@@ -51,7 +51,7 @@ internal class MultiResolverInfo(val resolverInfoList: List<NormalResolverInfo>)
     private fun findDataClass(): Class<*> {
         val dataClass = resolverInfoList.map { it.dataClassType }.distinct().singleOrNull()
 
-        if(dataClass == null) {
+        if (dataClass == null) {
             throw ResolverError("Resolvers may not use the same type.")
         } else {
             return dataClass
@@ -65,17 +65,17 @@ internal class MultiResolverInfo(val resolverInfoList: List<NormalResolverInfo>)
     }
 }
 
-internal class RootResolverInfo(val resolvers: List<GraphQLRootResolver>, private val options: SchemaParserOptions): ResolverInfo() {
+internal class RootResolverInfo(val resolvers: List<GraphQLRootResolver>, private val options: SchemaParserOptions) : ResolverInfo() {
     override fun getFieldSearches() =
-        resolvers.map { FieldResolverScanner.Search(getRealResolverClass(it, options), this, it) }
+            resolvers.map { FieldResolverScanner.Search(getRealResolverClass(it, options), this, it) }
 }
 
-internal class DataClassResolverInfo(private val dataClass: Class<*>): ResolverInfo() {
+internal class DataClassResolverInfo(private val dataClass: Class<*>) : ResolverInfo() {
     override fun getFieldSearches() =
-        listOf(FieldResolverScanner.Search(dataClass, this, null))
+            listOf(FieldResolverScanner.Search(dataClass, this, null))
 }
 
-internal class MissingResolverInfo: ResolverInfo() {
+internal class MissingResolverInfo : ResolverInfo() {
     override fun getFieldSearches(): List<FieldResolverScanner.Search> = listOf()
 }
 

src/main/kotlin/com/coxautodev/graphql/tools/SchemaClassScanner.kt

@@ -216,10 +216,10 @@ internal class SchemaClassScanner(initialDictionary: BiMap<String, Class<*>>, al
         val resolverInfoList = this.resolverInfos.filter { it.dataClassType == item.clazz }
         val resolverInfo: ResolverInfo
 
-        if (resolverInfoList.size > 1) {
-            resolverInfo = MultiResolverInfo(resolverInfoList)
+        resolverInfo = if (resolverInfoList.size > 1) {
+            MultiResolverInfo(resolverInfoList)
         } else {
-            resolverInfo = resolverInfosByDataClass[item.clazz] ?: DataClassResolverInfo(item.clazz)
+            resolverInfosByDataClass[item.clazz] ?: DataClassResolverInfo(item.clazz)
         }
 
         scanResolverInfoForPotentialMatches(item.type, resolverInfo)
@@ -229,7 +229,7 @@ internal class SchemaClassScanner(initialDictionary: BiMap<String, Class<*>>, al
         type.getExtendedFieldDefinitions(extensionDefinitions).forEach { field ->
             val fieldResolver = fieldResolverScanner.findFieldResolver(field, resolverInfo)
 
-            fieldResolversByType.getOrPut(type, { mutableMapOf() })[fieldResolver.field] = fieldResolver
+            fieldResolversByType.getOrPut(type) { mutableMapOf() }[fieldResolver.field] = fieldResolver
             fieldResolver.scanForMatches().forEach { potentialMatch ->
                 handleFoundType(typeClassMatcher.match(potentialMatch))
             }

src/test/groovy/com/coxautodev/graphql/tools/GenericResolverSpec.groovy

@@ -0,0 +1,46 @@
+package com.coxautodev.graphql.tools
+
+import graphql.schema.GraphQLSchema
+import spock.lang.Specification
+
+class GenericResolverSpec extends Specification {
+
+    def "methods from generic resolvers are resolved"() {
+        when:
+            GraphQLSchema schema = SchemaParser.newParser().schemaString('''\
+                        type Query {
+                            bar: Bar!
+                        }
+                        
+                        type Bar {
+                            value: String
+                        }
+                        ''')
+                    .resolvers(new QueryResolver(), new BarResolver())
+                    .build()
+                    .makeExecutableSchema()
+
+        then:
+            schema != null
+    }
+
+}
+
+class QueryResolver implements GraphQLQueryResolver {
+    Bar getBar() {
+        return new Bar()
+    }
+}
+
+class Bar {
+}
+
+abstract class FooResolver<T> implements GraphQLResolver<T> {
+    String getValue(T foo) {
+        return "value"
+    }
+}
+
+class BarResolver extends FooResolver<Bar> implements GraphQLResolver<Bar> {
+
+}

src/test/groovy/com/coxautodev/graphql/tools/SchemaParserSpec.groovy

@@ -34,7 +34,9 @@ class SchemaParserSpec extends Specification {
     def "builder doesn't throw FileNotFound exception when file is present"() {
         when:
             SchemaParser.newParser().file("test.graphqls")
-                    .resolvers(new GraphQLQueryResolver() { String getId() { "1" }})
+                    .resolvers(new GraphQLQueryResolver() {
+                String getId() { "1" }
+            })
                     .build()
 
         then:
@@ -317,15 +319,15 @@ class SchemaParserSpec extends Specification {
                     }
                 '''.stripIndent())
                     .resolvers(new GraphQLMutationResolver() {
-                        boolean save(SaveInput input) { false }
+                boolean save(SaveInput input) { false }
 
-                        class SaveInput {
-                            EnumType type;
-                        }
+                class SaveInput {
+                    EnumType type;
+                }
 
-                    }, new GraphQLQueryResolver() {
-                        boolean test() { false }
-                    })
+            }, new GraphQLQueryResolver() {
+                boolean test() { false }
+            })
                     .dictionary(EnumType.class)
                     .build()
                     .makeExecutableSchema()
@@ -364,6 +366,7 @@ class SchemaParserSpec extends Specification {
         then:
             noExceptionThrown()
     }
+
 }
 
 enum EnumType { TEST }