Merge branch 'master' into bugfix/446

Author: oryan-block

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

@@ -65,14 +65,16 @@ internal class TypeClassMatcher(private val definitionsByName: Map<String, TypeD
                 match(potentialMatch, graphQLType.type, realType)
             }
 
-            is ListType -> {
-                if (realType is ParameterizedType && isListType(realType, potentialMatch)) {
+            is ListType -> when {
+                realType is ParameterizedType && isListType(realType, potentialMatch) ->
                     match(potentialMatch, graphQLType.type, realType.actualTypeArguments.first())
-                } else if (realType is Class<*> && realType.isArray) {
+                realType is Class<*> && realType.isArray ->
                     match(potentialMatch, graphQLType.type, realType.componentType)
-                } else {
+                // e.g. a property map value: the element type is unknown, so match it as Object too
+                realType == Object::class.java ->
+                    match(potentialMatch, graphQLType.type, realType)
+                else ->
                     throw error(potentialMatch, "Java class is not a List or generic type information was lost: $realType")
-                }
             }
 
             is TypeName -> {

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

@@ -29,6 +29,8 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) {
 
     private val allowedLastArgumentTypes = listOfNotNull(DataFetchingEnvironment::class.java, GraphQLContext::class.java, options.contextClass)
 
+    private val methodsByNameCache = mutableMapOf<Pair<Class<out Any>, Boolean>, Map<String, List<Method>>>()
+
     fun findFieldResolver(field: FieldDefinition, resolverInfo: ResolverInfo): FieldResolver {
         val searches = resolverInfo.getFieldSearches()
 
@@ -86,29 +88,34 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) {
     }
 
     private fun findResolverMethod(field: FieldDefinition, search: Search): Method? {
-        val methods = getAllMethods(search)
+        val methodsByName = getMethodsByName(search)
         val argumentCount = field.inputValueDefinitions.size + if (search.requiredFirstParameterType != null) 1 else 0
         val name = field.name
+        val capitalizedName = name.replaceFirstChar(Char::titlecase)
 
         // Check for the following one by one:
         //   1. Method with exact field name
         //   2. Method that returns a boolean with "is" style getter
         //   3. Method with "get" style getter
         //   4. Method with "getField" style getter
         //   5. Method with "get" style getter with the field name converted from snake_case to camelCased. ex: key_ops -> getKeyOps()
-        return methods.find {
-            it.name == name && verifyMethodArguments(it, argumentCount, search)
-        } ?: methods.find {
-            (isBoolean(field.type) && it.name == "is${name.replaceFirstChar(Char::titlecase)}") && verifyMethodArguments(it, argumentCount, search)
-        } ?: methods.find {
-            it.name == "get${name.replaceFirstChar(Char::titlecase)}" && verifyMethodArguments(it, argumentCount, search)
-        } ?: methods.find {
-            it.name == "getField${name.replaceFirstChar(Char::titlecase)}" && verifyMethodArguments(it, argumentCount, search)
-        } ?: methods.find {
-            it.name == "get${name.snakeToCamelCase()}" && verifyMethodArguments(it, argumentCount, search)
+        return listOfNotNull(
+            name,
+            if (isBoolean(field.type)) "is$capitalizedName" else null,
+            "get$capitalizedName",
+            "getField$capitalizedName",
+            "get${name.snakeToCamelCase()}"
+        ).firstNotNullOfOrNull { methodName ->
+            methodsByName[methodName]?.find { verifyMethodArguments(it, argumentCount, search) }
         }
     }
 
+    // root fields are searched on every root resolver, so each class's methods are only indexed once
+    private fun getMethodsByName(search: Search): Map<String, List<Method>> =
+        methodsByNameCache.getOrPut(search.type.unwrap() to search.isSubscription) {
+            getAllMethods(search).groupBy { it.name }
+        }
+
     private fun getAllMethods(search: Search): List<Method> {
         val type = search.type.unwrap()
         val declaredMethods = type.declaredNonProxyMethods

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

@@ -15,6 +15,8 @@ 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
 import kotlinx.coroutines.future.future
 import org.apache.commons.lang3.reflect.TypeUtils
 import org.reactivestreams.Publisher
@@ -211,7 +213,10 @@ internal class MethodFieldResolverDataFetcher(
         val args = this.args.map { it(environment) }.toTypedArray()
 
         return if (isSuspendFunction) {
-            environment.coroutineScope().future(options.coroutineContextProvider.provide()) {
+            // start undispatched so DataLoader loads are queued before graphql-java dispatches them,
+            // which runs the block even if the context is already cancelled, hence ensureActive
+            environment.coroutineScope().future(options.coroutineContextProvider.provide(), CoroutineStart.UNDISPATCHED) {
+                ensureActive()
                 invokeSuspend(source, method, args)?.transformWithGenericWrapper(options.genericWrappers) { environment }
             }
         } else {

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

@@ -98,6 +98,19 @@ class FieldResolverScannerTest {
         Locale.setDefault(default)
     }
 
+    @Test
+    fun `scanner finds field resolver methods in priority order`() {
+        val resolverInfo = RootResolverInfo(listOf(PriorityQuery()), options)
+
+        fun methodName(name: String, type: String) =
+            (scanner.findFieldResolver(FieldDefinition(name, TypeName(type)), resolverInfo) as MethodFieldResolver).method.name
+
+        assertEquals(methodName("name", "String"), "name")
+        assertEquals(methodName("active", "Boolean"), "isActive")
+        assertEquals(methodName("enabled", "String"), "getEnabled")
+        assertEquals(methodName("count", "Int"), "getFieldCount")
+    }
+
     class RootQuery1 : GraphQLQueryResolver {
         fun field1() {}
     }
@@ -118,6 +131,18 @@ class FieldResolverScannerTest {
         fun getId(): HullType = HullType()
     }
 
+    class PriorityQuery : GraphQLQueryResolver {
+        fun name(): String = "name"
+        fun getName(): String = "name"
+        fun getFieldName(): String = "name"
+        fun active(flag: Boolean): Boolean = flag
+        fun isActive(): Boolean = true
+        fun getActive(): Boolean = true
+        fun isEnabled(): Boolean = true
+        fun getEnabled(): String = "enabled"
+        fun getFieldCount(): Int = 1
+    }
+
     class HullType
 
     open class ParentPropertyQuery {

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

@@ -21,6 +21,7 @@ import kotlinx.coroutines.channels.ReceiveChannel
 import org.junit.Test
 import org.reactivestreams.Publisher
 import org.reactivestreams.tck.TestEnvironment
+import java.util.concurrent.CancellationException
 import java.util.concurrent.CompletableFuture
 import kotlin.coroutines.coroutineContext
 
@@ -57,6 +58,33 @@ class MethodFieldResolverDataFetcherTest {
         }
     }
 
+    @Test
+    fun `data fetcher does not invoke suspend function if coroutineContext defined by options is cancelled`() {
+        // setup
+        val cancelledClass = CancelledClass()
+
+        val resolver = createFetcher("active", cancelledClass, options = cancelledClass.options)
+
+        // expect
+        val future = resolver.get(createEnvironment(DataClass())) as CompletableFuture<*>
+        assert(runCatching { future.get() }.exceptionOrNull() is CancellationException)
+        assert(!cancelledClass.invoked)
+    }
+
+    class CancelledClass : GraphQLResolver<DataClass> {
+        var invoked = false
+
+        val options = SchemaParserOptions.Builder()
+            .coroutineContext(Dispatchers.Default + Job().apply { cancel() })
+            .build()
+
+        @Suppress("UNUSED_PARAMETER")
+        suspend fun isActive(data: DataClass): Boolean {
+            invoked = true
+            return true
+        }
+    }
+
     @ExperimentalCoroutinesApi
     @Test
     fun `canceling subscription Publisher also cancels underlying Kotlin coroutine channel`() {

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

@@ -0,0 +1,129 @@
+package graphql.kickstart.tools
+
+import graphql.GraphQL
+import org.junit.Assert.assertThrows
+import org.junit.Test
+
+class PropertyMapListTest {
+
+    @Test
+    fun `property maps should support list fields`() {
+        val schema = SchemaParser.newParser()
+            .schemaString(
+                """
+                type Query {
+                    greeting: Greeting!
+                }
+
+                type Greeting {
+                    name: String!
+                    tags: [String!]!
+                    matrix: [[Int]]
+                    key: Key
+                    keys: [Key!]
+                }
+
+                type Key {
+                    a: String
+                    b: [String]
+                }
+                """)
+            .resolvers(object : GraphQLQueryResolver {
+                fun greeting(): Map<String, Any> = mapOf(
+                    "name" to "hello",
+                    "tags" to listOf("x", "y"),
+                    "matrix" to listOf(listOf(1, 2), listOf(3)),
+                    "key" to hashMapOf("a" to "one", "b" to listOf("ss")),
+                    "keys" to listOf(hashMapOf("a" to "two", "b" to listOf("tt")))
+                )
+            })
+            .dictionary("Key", HashMap::class)
+            .build()
+            .makeExecutableSchema()
+        val gql = GraphQL.newGraphQL(schema).build()
+
+        val data = assertNoGraphQlErrors(gql) {
+            """
+            {
+                greeting {
+                    name
+                    tags
+                    matrix
+                    key { a b }
+                    keys { a b }
+                }
+            }
+            """
+        }
+
+        assertEquals(data["greeting"], mapOf(
+            "name" to "hello",
+            "tags" to listOf("x", "y"),
+            "matrix" to listOf(listOf(1, 2), listOf(3)),
+            "key" to mapOf("a" to "one", "b" to listOf("ss")),
+            "keys" to listOf(mapOf("a" to "two", "b" to listOf("tt")))
+        ))
+    }
+
+    @Test
+    fun `property maps with typed list values should support list fields`() {
+        val schema = SchemaParser.newParser()
+            .schemaString(
+                """
+                type Query {
+                    tags: Tags!
+                }
+
+                type Tags {
+                    names: [String!]!
+                }
+                """)
+            .resolvers(object : GraphQLQueryResolver {
+                fun tags(): TagsMap = TagsMap().apply { put("names", listOf("x", "y")) }
+            })
+            .build()
+            .makeExecutableSchema()
+        val gql = GraphQL.newGraphQL(schema).build()
+
+        val data = assertNoGraphQlErrors(gql) {
+            """
+            {
+                tags {
+                    names
+                }
+            }
+            """
+        }
+
+        assertEquals(data["tags"], mapOf("names" to listOf("x", "y")))
+    }
+
+    @Test
+    fun `property maps with object list fields should require a dictionary entry`() {
+        val error = assertThrows(SchemaClassScannerError::class.java) {
+            SchemaParser.newParser()
+                .schemaString(
+                    """
+                    type Query {
+                        greeting: Greeting!
+                    }
+
+                    type Greeting {
+                        keys: [Key!]
+                    }
+
+                    type Key {
+                        a: String
+                    }
+                    """)
+                .resolvers(object : GraphQLQueryResolver {
+                    fun greeting(): Map<String, Any> = mapOf()
+                })
+                .build()
+        }
+
+        assertEquals(error.message, "The GraphQL schema type 'Key' maps to a field of type java.lang.Object however there is no matching entry for this type in the type dictionary. You may need to add this type to the dictionary before building the schema.")
+    }
+
+    class TagsMap : HashMap<String, List<String>>()
+}

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

@@ -0,0 +1,81 @@
+package graphql.kickstart.tools
+
+import graphql.ExecutionInput
+import graphql.GraphQL
+import graphql.schema.DataFetchingEnvironment
+import kotlinx.coroutines.future.await
+import org.dataloader.DataLoaderFactory
+import org.dataloader.DataLoaderRegistry
+import org.junit.Test
+import java.util.concurrent.CompletableFuture
+import java.util.concurrent.TimeUnit
+
+class SuspendFunctionDataLoaderTest {
+
+    private val schema = SchemaParser.newParser()
+        .schemaString(
+            """
+            type Query {
+                user(id: Int!): User!
+                users(ids: [Int!]!): [User!]!
+            }
+
+            type User {
+                id: Int!
+                friend: User!
+            }
+            """)
+        .resolvers(Query(), UserResolver())
+        .build()
+        .makeExecutableSchema()
+    private val gql = GraphQL.newGraphQL(schema).build()
+
+    @Test
+    fun `root suspend function can await a data loader`() {
+        repeat(20) {
+            val data = execute("{ users(ids: [1, 2, 3]) { id } }")
+
+            assertEquals(data, mapOf("users" to listOf(mapOf("id" to 1), mapOf("id" to 2), mapOf("id" to 3))))
+        }
+    }
+
+    @Test
+    fun `nested suspend function can await a data loader`() {
+        repeat(20) {
+            val data = execute("{ a: user(id: 1) { friend { id } } b: user(id: 2) { friend { id } } }")
+
+            assertEquals(data, mapOf(
+                "a" to mapOf("friend" to mapOf("id" to 2)),
+                "b" to mapOf("friend" to mapOf("id" to 3))
+            ))
+        }
+    }
+
+    private fun execute(query: String): Any? {
+        val userLoader = DataLoaderFactory.newDataLoader<Int, User> { ids ->
+            CompletableFuture.supplyAsync { ids.map { User(it) } }
+        }
+        val registry = DataLoaderRegistry.newRegistry().register("user", userLoader).build()
+
+        // a resolver that awaits a load before it is dispatched hangs, so don't wait forever
+        val result = gql.executeAsync(ExecutionInput.newExecutionInput(query).dataLoaderRegistry(registry))
+            .get(5, TimeUnit.SECONDS)
+
+        assert(result.errors.isEmpty()) { result.errors.joinToString { it.message } }
+        return result.getData<Any>()
+    }
+
+    class Query : GraphQLQueryResolver {
+        fun user(id: Int): User = User(id)
+
+        suspend fun users(ids: List<Int>, env: DataFetchingEnvironment): List<User> =
+            env.getDataLoader<Int, User>("user")!!.loadMany(ids).await()
+    }
+
+    class UserResolver : GraphQLResolver<User> {
+        suspend fun friend(user: User, env: DataFetchingEnvironment): User =
+            env.getDataLoader<Int, User>("user")!!.load(user.id + 1).await()
+    }
+
+    data class User(val id: Int)
+}