Repository: graphql-java-kickstart/graphql-java-tools
Pull request #361 (2020-01-31 05:44:59)

fix unwrapGenericType to properly find generic type in arguments Map
## Problem
This PR fixes `GenericType$RelativeTo.unwrapGenericType` method adding the ability to correctly find generic types for a multi-level hierarchy class abstraction. Currently, the following exception occurs in case object is mapped with such case:

```
Exception in thread "main" java.lang.IllegalStateException: No type variable found for: com.mydomain.MyAbstractClass:T extends com.mydomain.MyTClass<?, ?>
	at com.coxautodev.graphql.tools.GenericType$RelativeTo.unwrapGenericType(GenericType.kt:126)
	at com.coxautodev.graphql.tools.GenericType$RelativeTo.unwrapGenericType(GenericType.kt:100)
	at com.coxautodev.graphql.tools.TypeClassMatcher.match(TypeClassMatcher.kt:34)
	at com.coxautodev.graphql.tools.TypeClassMatcher.match$default(TypeClassMatcher.kt:32)
	at com.coxautodev.graphql.tools.TypeClassMatcher.match(TypeClassMatcher.kt:80)
	at com.coxautodev.graphql.tools.TypeClassMatcher.match(TypeClassMatcher.kt:28)
	at com.coxautodev.graphql.tools.SchemaClassScanner.scanResolverInfoForPotentialMatches(SchemaClassScanner.kt:268)
	at com.coxautodev.graphql.tools.SchemaClassScanner.scanQueueItemForPotentialMatches(SchemaClassScanner.kt:251)
	at com.coxautodev.graphql.tools.SchemaClassScanner.scanQueue(SchemaClassScanner.kt:103)
	at com.coxautodev.graphql.tools.SchemaClassScanner.scanForClasses(SchemaClassScanner.kt:81)
	at com.coxautodev.graphql.tools.SchemaParserBuilder.scan(SchemaParserBuilder.kt:165)
	at com.coxautodev.graphql.tools.SchemaParserBuilder.build(SchemaParserBuilder.kt:206)
```

### Method
GenericType$RelativeTo.unwrapGenericType(GenericType.kt:126)

The existing logic, line 125, returns proper types in a Map, but fails to find `type` returning null instead.
`TypeUtils.determineTypeArguments(getRawClass(mostSpecificType), declaringType)[type]`

## Expected behavior
`unwrapGenericType` should be capable of finding generic types independently of number of class extentions. A > B > C
```
A<T> : B<T>
B<T> : C<T>
C<T>
```

## Solution
Filtering the map by matching type names instead of trying to get by type equality solves the problem.


Patch:
diff --git a/src/main/kotlin/graphql/kickstart/tools/GenericType.kt b/src/main/kotlin/graphql/kickstart/tools/GenericType.kt
index 60a5845..7771c5f 100644
--- a/src/main/kotlin/graphql/kickstart/tools/GenericType.kt
+++ b/src/main/kotlin/graphql/kickstart/tools/GenericType.kt
@@ -121,9 +121,17 @@ internal open class GenericType(protected val mostSpecificType: JavaType, protec
                     }
                 }
 
-        private fun unwrapGenericType(declaringType: ParameterizedType, type: TypeVariable<*>) =
-                unwrapGenericType(TypeUtils.determineTypeArguments(getRawClass(mostSpecificType), declaringType)[type]
-                        ?: error("No type variable found for: ${TypeUtils.toLongString(type)}"))
+        private fun unwrapGenericType(declaringType: ParameterizedType, type: TypeVariable<*>): JavaType {
+            val rawClass = getRawClass(mostSpecificType)
+            val arguments = TypeUtils.determineTypeArguments(rawClass, declaringType)
+            val matchingType = arguments
+                    .filter { it.key.name == type.name }
+                    .values
+                    .firstOrNull()
+                    ?: error("No type variable found for: ${TypeUtils.toLongString(type)}")
+
+            return unwrapGenericType(matchingType)
+        }
 
         private fun replaceTypeVariable(type: JavaType): JavaType {
             return when (type) {


Test patch:
diff --git a/src/test/groovy/graphql/kickstart/tools/SchemaParserSpec.groovy b/src/test/groovy/graphql/kickstart/tools/SchemaParserSpec.groovy
index 78e486e..f98e20f 100644
--- a/src/test/groovy/graphql/kickstart/tools/SchemaParserSpec.groovy
+++ b/src/test/groovy/graphql/kickstart/tools/SchemaParserSpec.groovy
@@ -33,7 +33,7 @@ class SchemaParserSpec extends Specification {
 
     def "builder doesn't throw FileNotFound exception when file is present"() {
         when:
-            SchemaParser.newParser().file("test.graphqls")
+            SchemaParser.newParser().file("Test.graphqls")
                     .resolvers(new GraphQLQueryResolver() {
                 String getId() { "1" }
             })
@@ -286,7 +286,7 @@ class SchemaParserSpec extends Specification {
     def "parser should include source location for field definition when loaded from single classpath file"() {
         when:
             GraphQLSchema schema = SchemaParser.newParser()
-                    .file("test.graphqls")
+                    .file("Test.graphqls")
                     .resolvers(new QueryWithIdResolver())
                     .build()
                     .makeExecutableSchema()
@@ -298,7 +298,7 @@ class SchemaParserSpec extends Specification {
             sourceLocation != null
             sourceLocation.line == 2
             sourceLocation.column == 5
-            sourceLocation.sourceName == "test.graphqls"
+            sourceLocation.sourceName == "Test.graphqls"
     }
 
     def "support enum types if only used as input type"() {
diff --git a/src/test/kotlin/graphql/kickstart/tools/DeepGenericsHierarchyTest.kt b/src/test/kotlin/graphql/kickstart/tools/DeepGenericsHierarchyTest.kt
new file mode 100644
index 0000000..72a6e35
--- /dev/null
+++ b/src/test/kotlin/graphql/kickstart/tools/DeepGenericsHierarchyTest.kt
@@ -0,0 +1,55 @@
+package graphql.kickstart.tools
+
+import graphql.ExecutionInput
+import graphql.GraphQL
+import org.junit.Test
+
+class PlaceTest {
+
+    @Test
+    fun shouldHandleGenericsDeepHierarchy() {
+        val schema = SchemaParser.newParser()
+                .file("Place.graphqls")
+                .resolvers(PlaceQuery())
+                .build().makeExecutableSchema()
+
+        val graphql = GraphQL.newGraphQL(schema).build()
+        val query = "query { places1 { id } places2 { id } }"
+        val executionInput = ExecutionInput.newExecutionInput().query(query).build()
+        val result = graphql.execute(executionInput)
+
+        assert(result.getData<Map<String, List<*>>>()["places1"]?.size == 3)
+        assert(result.getData<Map<String, List<*>>>()["places2"]?.size == 2)
+    }
+}
+
+private class PlaceQuery : GraphQLQueryResolver {
+
+    fun places1(): List<Place1> = listOf(Place1("1"), Place1("2"), Place1("3"))
+
+    fun places2(): List<Place2> = listOf(Place2("4"), Place2("5"))
+}
+
+private abstract class Entity(val id: String? = null)
+
+private abstract class OtherPlace<R : Review<*>>(id: String? = null) : Place<R>(id) {
+    val other: String? = null
+}
+
+private abstract class Place<R : Review<*>>(id: String? = null) : Entity(id) {
+    val name: String? = null
+    val reviews: MutableSet<R>? = null
+}
+
+private class Place1(id: String? = null) : OtherPlace<Review1>(id)
+
+private class Place2(id: String? = null) : OtherPlace<Review2>(id)
+
+private abstract class Review<T : Entity>(id: String? = null) : Entity(id) {
+    val rating: Int? = null
+    val content: T? = null
+}
+
+private class Review1(id: String? = null) : Review<Place1>(id)
+
+private class Review2(id: String? = null) : Review<Place2>(id)
diff --git a/src/test/kotlin/graphql/kickstart/tools/UtilsTest.kt b/src/test/kotlin/graphql/kickstart/tools/UtilsTest.kt
index 38f9cc6..5294300 100644
--- a/src/test/kotlin/graphql/kickstart/tools/UtilsTest.kt
+++ b/src/test/kotlin/graphql/kickstart/tools/UtilsTest.kt
@@ -24,7 +24,7 @@ class UtilsTest {
     }
 
     @Test
-    fun `isTrivialDataFetcher`() {
+    fun isTrivialDataFetcher() {
         val clazz = Bean::class.java
 
         Assert.assertTrue(isTrivialDataFetcher(clazz.getMethod("getterValid")))
diff --git a/src/test/resources/Place.graphqls b/src/test/resources/Place.graphqls
new file mode 100644
index 0000000..dc9c7c6
--- /dev/null
+++ b/src/test/resources/Place.graphqls
@@ -0,0 +1,51 @@
+type Query {
+  places1: [Place1!]
+  places2: [Place2!]
+}
+
+interface Entity {
+  id: ID!
+}
+
+interface Place {
+  name: String
+  reviews: [Review!]
+}
+
+interface OtherPlace {
+  name: String
+  other: String
+  reviews: [Review!]
+}
+
+type Place1 implements Entity, Place, OtherPlace {
+  id: ID!
+  name: String
+  other: String
+  reviews: [Review1!]
+}
+
+type Place2 implements Entity, Place, OtherPlace {
+  id: ID!
+  name: String
+  other: String
+  reviews: [Review2!]
+}
+
+interface Review {
+  id: ID!
+  rating: Int
+  content: Entity
+}
+
+type Review1 implements Review {
+  id: ID!
+  rating: Int
+  content: Place1
+}
+
+type Review2 implements Review {
+  id: ID!
+  rating: Int
+  content: Place2
+}