Use LightDataFetcher where possible

Author: vojtapol

pom.xml

@@ -4,7 +4,7 @@
 
     <groupId>com.graphql-java-kickstart</groupId>
     <artifactId>graphql-java-tools</artifactId>
-    <version>13.1.2-SNAPSHOT</version>
+    <version>14.0.0-LOCAL</version>
     <packaging>jar</packaging>
 
     <name>GraphQL Java Tools</name>

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

@@ -29,13 +29,20 @@ internal abstract class FieldResolver(
     /**
      * Add source resolver depending on whether or not this is a resolver method
      */
-    protected fun getSourceResolver(): SourceResolver {
+    protected fun createSourceResolver(): SourceResolver {
         return if (this.search.source != null) {
-            { this.search.source }
+            SourceResolver { _, _ -> this.search.source }
         } else {
-            { environment ->
-                val source = environment.getSource<Any>()
-                    ?: throw ResolverError("Expected source object to not be null!")
+            SourceResolver { environment, sourceObject ->
+                val source = if (sourceObject != null) {
+                    // if source object is known, environment is null as an optimization (LightDataFetcher)
+                    sourceObject
+                } else {
+                    environment
+                        ?: throw ResolverError("Expected DataFetchingEnvironment to not be null!")
+                    environment.getSource<Any>()
+                        ?: throw ResolverError("Expected source object to not be null!")
+                }
 
                 if (!this.genericType.isAssignableFrom(source.javaClass)) {
                     throw ResolverError("Expected source object to be an instance of '${this.genericType.getRawClass().name}' but instead got '${source.javaClass.name}'")
@@ -47,4 +54,7 @@ internal abstract class FieldResolver(
     }
 }
 
-internal typealias SourceResolver = (DataFetchingEnvironment) -> Any
+fun interface SourceResolver {
+
+    fun resolve(environment: DataFetchingEnvironment?, source: Any?): Any
+}

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

@@ -8,6 +8,9 @@ import graphql.kickstart.tools.util.JavaType
 import graphql.language.FieldDefinition
 import graphql.schema.DataFetcher
 import graphql.schema.DataFetchingEnvironment
+import graphql.schema.GraphQLFieldDefinition
+import graphql.schema.LightDataFetcher
+import java.util.function.Supplier
 
 /**
  * @author Nick Weedon
@@ -37,7 +40,7 @@ internal class MapFieldResolver(
     }
 
     override fun createDataFetcher(): DataFetcher<*> {
-        return MapFieldResolverDataFetcher(getSourceResolver(), field.name)
+        return MapFieldResolverDataFetcher(createSourceResolver(), field.name)
     }
 
     override fun scanForMatches(): List<TypeClassMatcher.PotentialMatch> {
@@ -49,15 +52,18 @@ internal class MapFieldResolver(
 
 internal class MapFieldResolverDataFetcher(
     private val sourceResolver: SourceResolver,
-    private val key: String
-) : DataFetcher<Any> {
+    private val key: String,
+) : LightDataFetcher<Any> {
 
-    override fun get(environment: DataFetchingEnvironment): Any? {
-        val resolvedSourceObject = sourceResolver(environment)
-        if (resolvedSourceObject is Map<*, *>) {
-            return resolvedSourceObject[key]
+    override fun get(fieldDefinition: GraphQLFieldDefinition, sourceObject: Any, environmentSupplier: Supplier<DataFetchingEnvironment>): Any? {
+        if (sourceObject is Map<*, *>) {
+            return sourceObject[key]
         } else {
             throw RuntimeException("MapFieldResolver attempt to fetch a field from an object instance that was not a map")
         }
     }
+
+    override fun get(environment: DataFetchingEnvironment): Any? {
+        return get(environment.fieldDefinition, sourceResolver.resolve(environment, null), { environment })
+    }
 }

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

@@ -2,7 +2,6 @@ package graphql.kickstart.tools.resolver
 
 import com.fasterxml.jackson.core.type.TypeReference
 import graphql.GraphQLContext
-import graphql.TrivialDataFetcher
 import graphql.kickstart.tools.*
 import graphql.kickstart.tools.SchemaParserOptions.GenericWrapper
 import graphql.kickstart.tools.util.JavaType
@@ -12,12 +11,15 @@ import graphql.kickstart.tools.util.unwrap
 import graphql.language.*
 import graphql.schema.DataFetcher
 import graphql.schema.DataFetchingEnvironment
+import graphql.schema.GraphQLFieldDefinition
 import graphql.schema.GraphQLTypeUtil.isScalar
+import graphql.schema.LightDataFetcher
 import kotlinx.coroutines.future.future
 import org.slf4j.LoggerFactory
 import java.lang.reflect.InvocationTargetException
 import java.lang.reflect.Method
 import java.util.*
+import java.util.function.Supplier
 import kotlin.coroutines.intrinsics.suspendCoroutineUninterceptedOrReturn
 import kotlin.reflect.full.valueParameters
 import kotlin.reflect.jvm.javaType
@@ -122,9 +124,9 @@ internal class MethodFieldResolver(
         }
 
         return if (args.isEmpty() && isTrivialDataFetcher(this.method)) {
-            TrivialMethodFieldResolverDataFetcher(getSourceResolver(), this.method, args, options)
+            LightMethodFieldResolverDataFetcher(createSourceResolver(), this.method, options)
         } else {
-            MethodFieldResolverDataFetcher(getSourceResolver(), this.method, args, options)
+            MethodFieldResolverDataFetcher(createSourceResolver(), this.method, args, options)
         }
     }
 
@@ -189,69 +191,102 @@ internal class MethodFieldResolver(
     override fun toString() = "MethodFieldResolver{method=$method}"
 }
 
-internal open class MethodFieldResolverDataFetcher(
+internal class MethodFieldResolverDataFetcher(
     private val sourceResolver: SourceResolver,
-    method: Method,
+    private val method: Method,
     private val args: List<ArgumentPlaceholder>,
     private val options: SchemaParserOptions,
 ) : DataFetcher<Any> {
 
-    private val resolverMethod = method
-    private val isSuspendFunction = try {
-        method.kotlinFunction?.isSuspend == true
-    } catch (e: InternalError) {
-        false
-    }
+    private val isSuspendFunction = method.isSuspendFunction()
 
-    private class CompareGenericWrappers {
-        companion object : Comparator<GenericWrapper> {
-            override fun compare(w1: GenericWrapper, w2: GenericWrapper): Int = when {
-                w1.type.isAssignableFrom(w2.type) -> 1
-                else -> -1
+    override fun get(environment: DataFetchingEnvironment): Any? {
+        val source = sourceResolver.resolve(environment, null)
+        val args = this.args.map { it(environment) }.toTypedArray()
+
+        return if (isSuspendFunction) {
+            environment.coroutineScope().future(options.coroutineContextProvider.provide()) {
+                invokeSuspend(source, method, args)?.transformWithGenericWrapper(options.genericWrappers, { environment })
             }
+        } else {
+            invoke(method, source, args)?.transformWithGenericWrapper(options.genericWrappers, { environment })
         }
     }
 
-    override fun get(environment: DataFetchingEnvironment): Any? {
-        val source = sourceResolver(environment)
-        val args = this.args.map { it(environment) }.toTypedArray()
+    /**
+     * Function that returns the object used to fetch the data. It can be a DataFetcher or an entity.
+     */
+    @Suppress("unused")
+    fun getWrappedFetchingObject(environment: DataFetchingEnvironment): Any {
+        return sourceResolver.resolve(environment, null)
+    }
+}
+
+/**
+ * Similar to [MethodFieldResolverDataFetcher] but for light data fetchers which do not require the environment to be supplied unless suspend functions or
+ * generic wrappers are used.
+ */
+internal class LightMethodFieldResolverDataFetcher(
+    private val sourceResolver: SourceResolver,
+    private val method: Method,
+    private val options: SchemaParserOptions,
+) : LightDataFetcher<Any?> {
+
+    private val isSuspendFunction = method.isSuspendFunction()
+
+    override fun get(fieldDefinition: GraphQLFieldDefinition, sourceObject: Any, environmentSupplier: Supplier<DataFetchingEnvironment>): Any? {
+        val source = sourceResolver.resolve(null, sourceObject)
 
         return if (isSuspendFunction) {
-            environment.coroutineScope().future(options.coroutineContextProvider.provide()) {
-                invokeSuspend(source, resolverMethod, args)?.transformWithGenericWrapper(environment)
+            environmentSupplier.get().coroutineScope().future(options.coroutineContextProvider.provide()) {
+                invokeSuspend(source, method, emptyArray())?.transformWithGenericWrapper(options.genericWrappers, environmentSupplier)
             }
         } else {
-            invoke(resolverMethod, source, args)?.transformWithGenericWrapper(environment)
+            invoke(method, source, emptyArray())?.transformWithGenericWrapper(options.genericWrappers, environmentSupplier)
         }
     }
 
-    private fun Any.transformWithGenericWrapper(environment: DataFetchingEnvironment): Any? {
-        return options.genericWrappers
-            .asSequence()
-            .filter { it.type.isInstance(this) }
-            .sortedWith(CompareGenericWrappers)
-            .firstOrNull()
-            ?.transformer?.invoke(this, environment) ?: this
+    override fun get(environment: DataFetchingEnvironment): Any? {
+        return get(environment.fieldDefinition, sourceResolver.resolve(environment, null), { environment })
     }
 
     /**
-     * Function that returns the object used to fetch the data.
-     * It can be a DataFetcher or an entity.
+     * Function that returns the object used to fetch the data. It can be a DataFetcher or an entity.
      */
     @Suppress("unused")
-    open fun getWrappedFetchingObject(environment: DataFetchingEnvironment): Any {
-        return sourceResolver(environment)
+    fun getWrappedFetchingObject(environment: DataFetchingEnvironment): Any {
+        return sourceResolver.resolve(environment, null)
     }
 }
 
-// TODO use graphql.schema.LightDataFetcher
-internal class TrivialMethodFieldResolverDataFetcher(
-    sourceResolver: SourceResolver,
-    method: Method,
-    args: List<ArgumentPlaceholder>,
-    options: SchemaParserOptions,
-) : MethodFieldResolverDataFetcher(sourceResolver, method, args, options),
-    TrivialDataFetcher<Any> // just to mark it for tracing and optimizations
+private fun Any.transformWithGenericWrapper(
+    genericWrappers: List<GenericWrapper>,
+    environmentSupplier: Supplier<DataFetchingEnvironment>
+): Any? {
+    return genericWrappers
+        .asSequence()
+        .filter { it.type.isInstance(this) }
+        .sortedWith(CompareGenericWrappers)
+        .firstOrNull()
+        ?.transformer?.invoke(this, environmentSupplier.get()) ?: this
+}
+
+private class CompareGenericWrappers {
+    companion object : Comparator<GenericWrapper> {
+        override fun compare(w1: GenericWrapper, w2: GenericWrapper): Int = when {
+            w1.type.isAssignableFrom(w2.type) -> 1
+            else -> -1
+        }
+    }
+}
+
+private fun Method.isSuspendFunction(): Boolean {
+    return try {
+        this.kotlinFunction?.isSuspend == true
+    } catch (e: InternalError) {
+        false
+    }
+}
 
 private suspend inline fun invokeSuspend(target: Any, resolverMethod: Method, args: Array<Any?>): Any? {
     return suspendCoroutineUninterceptedOrReturn { continuation ->

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

@@ -6,7 +6,10 @@ import graphql.kickstart.tools.TypeClassMatcher
 import graphql.language.FieldDefinition
 import graphql.schema.DataFetcher
 import graphql.schema.DataFetchingEnvironment
+import graphql.schema.GraphQLFieldDefinition
+import graphql.schema.LightDataFetcher
 import java.lang.reflect.Field
+import java.util.function.Supplier
 
 /**
  * @author Andrew Potter
@@ -15,11 +18,11 @@ internal class PropertyFieldResolver(
     field: FieldDefinition,
     search: FieldResolverScanner.Search,
     options: SchemaParserOptions,
-    private val property: Field
+    private val property: Field,
 ) : FieldResolver(field, search, options, property.declaringClass) {
 
     override fun createDataFetcher(): DataFetcher<*> {
-        return PropertyFieldResolverDataFetcher(getSourceResolver(), property)
+        return PropertyFieldResolverDataFetcher(createSourceResolver(), property)
     }
 
     override fun scanForMatches(): List<TypeClassMatcher.PotentialMatch> {
@@ -28,7 +31,8 @@ internal class PropertyFieldResolver(
                 field.type,
                 property.genericType,
                 genericType,
-                SchemaClassScanner.FieldTypeReference(property.toString()))
+                SchemaClassScanner.FieldTypeReference(property.toString())
+            )
         )
     }
 
@@ -37,10 +41,14 @@ internal class PropertyFieldResolver(
 
 internal class PropertyFieldResolverDataFetcher(
     private val sourceResolver: SourceResolver,
-    private val field: Field
-) : DataFetcher<Any> {
+    private val field: Field,
+) : LightDataFetcher<Any> {
+
+    override fun get(fieldDefinition: GraphQLFieldDefinition, sourceObject: Any, environmentSupplier: Supplier<DataFetchingEnvironment>): Any? {
+        return field.get(sourceResolver.resolve(null, sourceObject))
+    }
 
     override fun get(environment: DataFetchingEnvironment): Any? {
-        return field.get(sourceResolver(environment))
+        return get(environment.fieldDefinition, sourceResolver.resolve(environment, null), { environment })
     }
 }

src/main/kotlin/graphql/kickstart/tools/util/Utils.kt

@@ -62,11 +62,11 @@ internal fun getDocumentation(node: AbstractNode<*>, options: SchemaParserOption
     }
 
 /**
- * Simple heuristic to check is a method is a trivial data fetcher.
+ * Simple heuristic to check if a method is a trivial data fetcher.
  *
  * Requirements are:
- * prefixed with get
- * must have zero parameters
+ * - prefixed with get
+ * - must have zero parameters
  */
 internal fun isTrivialDataFetcher(method: Method): Boolean {
     return (method.parameterCount == 0
@@ -80,4 +80,3 @@ private fun isBooleanGetter(method: Method) = (method.name.startsWith("is")
     || method.returnType == Boolean::class.java)
 
 internal fun String.snakeToCamelCase(): String = split("_").joinToString(separator = "") { it.replaceFirstChar(Char::titlecase) }
-

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

@@ -10,6 +10,7 @@ import graphql.schema.idl.SchemaDirectiveWiringEnvironment
 import org.junit.Test
 
 class DirectiveTest {
+
     @Test
     fun `should apply @uppercase directive on field`() {
         val schema = SchemaParser.newParser()

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

@@ -13,6 +13,8 @@ import graphql.language.TypeName
 import graphql.schema.DataFetcher
 import graphql.schema.DataFetchingEnvironment
 import graphql.schema.DataFetchingEnvironmentImpl
+import graphql.schema.GraphQLFieldDefinition
+import graphql.schema.GraphQLObjectType
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.Job
@@ -299,6 +301,12 @@ class MethodFieldResolverDataFetcherTest {
     private fun createEnvironment(source: Any = Object(), arguments: Map<String, Any> = emptyMap(), context: GraphQLContext? = null): DataFetchingEnvironment {
         return DataFetchingEnvironmentImpl.newDataFetchingEnvironment(buildExecutionContext())
             .source(source)
+            .fieldDefinition(
+                GraphQLFieldDefinition.newFieldDefinition()
+                    .name("ignored")
+                    .type(GraphQLObjectType.newObject().name("ignored").build())
+                    .build()
+            )
             .arguments(arguments)
             .graphQLContext(context)
             .build()

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

@@ -7,14 +7,16 @@ import graphql.GraphQL
 private val mapper = ObjectMapper()
 
 fun assertNoGraphQlErrors(gql: GraphQL, args: Map<String, Any> = mapOf(), context: Map<Any, Any> = mapOf(), closure: () -> String): Map<String, Any> {
-    val result = gql.execute(ExecutionInput.newExecutionInput()
-        .query(closure.invoke())
-        .graphQLContext(context)
-        .root(context)
-        .variables(args))
+    val result = gql.execute(
+        ExecutionInput.newExecutionInput()
+            .query(closure.invoke())
+            .graphQLContext(context)
+            .root(context)
+            .variables(args)
+    )
 
     if (result.errors.isNotEmpty()) {
-        throw AssertionError("GraphQL result contained errors!\n${result.errors.map { mapper.writeValueAsString(it) }.joinToString { "\n" }}")
+        throw AssertionError("GraphQL result contained errors!\n${result.errors.map { it.message }.joinToString("\n")}")
     }
 
     return result.getData() as Map<String, Any>