Use LightDataFetcher where possible

Author: vojtapol

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

@@ -29,12 +29,15 @@ 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 }
+            // environment object is ignored and can be null in this case
+            SourceResolver { _ -> this.search.source }
         } else {
-            { environment ->
-                val source = environment.getSource<Any>()
+            SourceResolver { environment ->
+                val nonNullEnvironment = environment
+                    ?: throw ResolverError("Expected environment object to not be null!")
+                val source = nonNullEnvironment.getSource<Any>()
                     ?: throw ResolverError("Expected source object to not be null!")
 
                 if (!this.genericType.isAssignableFrom(source.javaClass)) {
@@ -47,4 +50,7 @@ internal abstract class FieldResolver(
     }
 }
 
-internal typealias SourceResolver = (DataFetchingEnvironment) -> Any
+fun interface SourceResolver {
+
+    fun resolve(environment: DataFetchingEnvironment?): 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? {
+        throw UnsupportedOperationException("This method should not be called as this is a LightDataFetcher.")
+    }
 }

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)
         }
     }
 
@@ -203,17 +205,8 @@ internal open class MethodFieldResolverDataFetcher(
         false
     }
 
-    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(environment)
+        val source = sourceResolver.resolve(environment)
         val args = this.args.map { it(environment) }.toTypedArray()
 
         return if (isSuspendFunction) {
@@ -240,18 +233,59 @@ internal open class MethodFieldResolverDataFetcher(
      */
     @Suppress("unused")
     open fun getWrappedFetchingObject(environment: DataFetchingEnvironment): Any {
-        return sourceResolver(environment)
+        return sourceResolver.resolve(environment)
     }
 }
 
-// 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
+/**
+ * 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 resolverMethod = method
+    private val isSuspendFunction = try {
+        method.kotlinFunction?.isSuspend == true
+    } catch (e: InternalError) {
+        false
+    }
+
+    override fun get(fieldDefinition: GraphQLFieldDefinition, sourceObject: Any, environmentSupplier: Supplier<DataFetchingEnvironment>): Any? {
+        return if (isSuspendFunction) {
+            environmentSupplier.get().coroutineScope().future(options.coroutineContextProvider.provide()) {
+                invokeSuspend(sourceObject, resolverMethod, emptyArray())?.transformWithGenericWrapper(environmentSupplier)
+            }
+        } else {
+            invoke(resolverMethod, sourceObject, emptyArray())?.transformWithGenericWrapper(environmentSupplier)
+        }
+    }
+
+    private fun Any.transformWithGenericWrapper(environment: Supplier<DataFetchingEnvironment>): Any? {
+        return options.genericWrappers
+            .asSequence()
+            .filter { it.type.isInstance(this) }
+            .sortedWith(CompareGenericWrappers)
+            .firstOrNull()
+            ?.transformer?.invoke(this, environment.get()) ?: this
+    }
+
+    override fun get(environment: DataFetchingEnvironment): Any? {
+        throw UnsupportedOperationException("This method should not be called as this is a LightDataFetcher.")
+    }
+}
+
+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 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))
+    }
 
     override fun get(environment: DataFetchingEnvironment): Any? {
-        return field.get(sourceResolver(environment))
+        throw UnsupportedOperationException("This method should not be called as this is a LightDataFetcher.")
     }
 }

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) }
-