Repository: graphql-java-kickstart/graphql-java-tools
Pull request #337 (2019-11-16 19:00:40)

Add the TrivialDataFetcher interface to MethodFieldResolver so that i…
…t can be excluded from tracing


Patch:
diff --git a/pom.xml b/pom.xml
index b2d8ed8..200464d 100644
--- a/pom.xml
+++ b/pom.xml
@@ -4,7 +4,7 @@
 
     <groupId>com.graphql-java-kickstart</groupId>
     <artifactId>graphql-java-tools</artifactId>
-    <version>5.7.2-SNAPSHOT</version>
+    <version>5.6.2-SNAPSHOT</version>
     <packaging>jar</packaging>
 
     <name>GraphQL Java Tools</name>
@@ -16,7 +16,7 @@
         <java.version>1.8</java.version>
         <kotlin.version>1.3.31</kotlin.version>
         <kotlin-coroutines.version>1.2.1</kotlin-coroutines.version>
-        <jackson.version>2.10.0</jackson.version>
+        <jackson.version>2.9.9</jackson.version>
         <graphql-java.version>13.0</graphql-java.version>
 
         <maven.compiler.source>${java.version}</maven.compiler.source>
diff --git a/src/main/kotlin/com/coxautodev/graphql/tools/MethodFieldResolver.kt b/src/main/kotlin/com/coxautodev/graphql/tools/MethodFieldResolver.kt
index 89465f3..2e9e82a 100644
--- a/src/main/kotlin/com/coxautodev/graphql/tools/MethodFieldResolver.kt
+++ b/src/main/kotlin/com/coxautodev/graphql/tools/MethodFieldResolver.kt
@@ -3,20 +3,16 @@ package com.coxautodev.graphql.tools
 import com.coxautodev.graphql.tools.SchemaParserOptions.GenericWrapper
 import com.esotericsoftware.reflectasm.MethodAccess
 import com.fasterxml.jackson.core.type.TypeReference
+import graphql.TrivialDataFetcher
 import graphql.execution.batched.Batched
 import graphql.language.FieldDefinition
-import graphql.language.ListType
 import graphql.language.NonNullType
-import graphql.language.Type
-import graphql.language.TypeName
 import graphql.schema.DataFetcher
 import graphql.schema.DataFetchingEnvironment
-import graphql.schema.GraphQLTypeUtil.isScalar
 import kotlinx.coroutines.GlobalScope
 import kotlinx.coroutines.future.future
 import java.lang.reflect.Method
-import java.util.Comparator
-import java.util.Optional
+import java.util.*
 import kotlin.coroutines.intrinsics.suspendCoroutineUninterceptedOrReturn
 import kotlin.reflect.full.valueParameters
 import kotlin.reflect.jvm.javaType
@@ -94,12 +90,6 @@ internal class MethodFieldResolver(field: FieldDefinition, search: FieldResolver
                     }
                 }
 
-                if (value != null
-                        && genericParameterType.unwrap().isAssignableFrom(value.javaClass)
-                        && isScalarType(environment, definition.type, genericParameterType)) {
-                    return@add value
-                }
-
                 return@add mapper.convertValue(value, typeReference)
             }
         }
@@ -116,28 +106,18 @@ internal class MethodFieldResolver(field: FieldDefinition, search: FieldResolver
         return if (batched) {
             BatchedMethodFieldResolverDataFetcher(getSourceResolver(), this.method, args, options)
         } else {
-            MethodFieldResolverDataFetcher(getSourceResolver(), this.method, args, options)
+            if (args.isEmpty() && isTrivialDataFetcher(this.method)) {
+                TrivialMethodFieldResolverDataFetcher(getSourceResolver(), this.method, args, options)
+            } else {
+                MethodFieldResolverDataFetcher(getSourceResolver(), this.method, args, options)
+            }
+
         }
     }
 
-    private fun isScalarType(environment: DataFetchingEnvironment, type: Type<*>, genericParameterType: JavaType): Boolean =
-            when (type) {
-                is ListType ->
-                    List::class.java.isAssignableFrom(this.genericType.getRawClass(genericParameterType))
-                        && isScalarType(environment, type.type, this.genericType.unwrapGenericType(genericParameterType))
-                is TypeName ->
-                    environment.graphQLSchema?.getType(type.name)?.let { isScalar(it) } ?: false
-                else ->
-                    false
-            }
-
     override fun scanForMatches(): List<TypeClassMatcher.PotentialMatch> {
         val batched = isBatched(method, search)
-        val unwrappedGenericType = genericType.unwrapGenericType(try {
-            method.kotlinFunction?.returnType?.javaType ?: method.genericReturnType
-        } catch (e: InternalError) {
-            method.genericReturnType
-        })
+        val unwrappedGenericType = genericType.unwrapGenericType(try { method.kotlinFunction?.returnType?.javaType ?: method.genericReturnType } catch (e: InternalError) { method.genericReturnType })
         val returnValueMatch = TypeClassMatcher.PotentialMatch.returnValue(field.type, unwrappedGenericType, genericType, SchemaClassScanner.ReturnValueReference(method), batched)
 
         return field.inputValueDefinitions.mapIndexed { i, inputDefinition ->
@@ -174,11 +154,7 @@ open class MethodFieldResolverDataFetcher(private val sourceResolver: SourceReso
     // Convert to reflactasm reflection
     private val methodAccess = MethodAccess.get(method.declaringClass)!!
     private val methodIndex = methodAccess.getIndex(method.name, *method.parameterTypes)
-    private val isSuspendFunction = try {
-        method.kotlinFunction?.isSuspend == true
-    } catch (e: InternalError) {
-        false
-    }
+    private val isSuspendFunction = try {method.kotlinFunction?.isSuspend == true } catch (e: InternalError) { false }
 
     private class CompareGenericWrappers {
         companion object : Comparator<GenericWrapper> {
@@ -221,6 +197,10 @@ open class MethodFieldResolverDataFetcher(private val sourceResolver: SourceReso
     }
 }
 
+open class TrivialMethodFieldResolverDataFetcher(private val sourceResolver: SourceResolver, method: Method, private val args: List<ArgumentPlaceholder>, private val options: SchemaParserOptions) : MethodFieldResolverDataFetcher(sourceResolver, method, args, options), TrivialDataFetcher<Any> {
+
+}
+
 private suspend inline fun MethodAccess.invokeSuspend(target: Any, methodIndex: Int, args: Array<Any?>): Any? {
     return suspendCoroutineUninterceptedOrReturn { continuation ->
         invoke(target, methodIndex, *args + continuation)
diff --git a/src/main/kotlin/com/coxautodev/graphql/tools/ResolverInfo.kt b/src/main/kotlin/com/coxautodev/graphql/tools/ResolverInfo.kt
index 2efe18d..6a8917c 100644
--- a/src/main/kotlin/com/coxautodev/graphql/tools/ResolverInfo.kt
+++ b/src/main/kotlin/com/coxautodev/graphql/tools/ResolverInfo.kt
@@ -19,8 +19,13 @@ internal class NormalResolverInfo(val resolver: GraphQLResolver<*>, private val
     override val dataClassType = findDataClass()
 
     private fun findDataClass(): Class<out Any> {
+        // 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) {
+            error("${GraphQLResolver::class.java.simpleName} interface was not parameterized for: ${resolverType.name}")
+        }
 
-        val type = TypeUtils.getTypeArguments(resolverType, GraphQLResolver::class.java)[GraphQLResolver::class.java.typeParameters[0]]
+        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.")
diff --git a/src/main/kotlin/com/coxautodev/graphql/tools/SchemaParser.kt b/src/main/kotlin/com/coxautodev/graphql/tools/SchemaParser.kt
index 8521205..37a67c9 100644
--- a/src/main/kotlin/com/coxautodev/graphql/tools/SchemaParser.kt
+++ b/src/main/kotlin/com/coxautodev/graphql/tools/SchemaParser.kt
@@ -126,9 +126,7 @@ class SchemaParser internal constructor(scanResult: ScannedSchemaObjects, privat
         val subscription = objects.find { it.name == subscriptionName }
                 ?: if (rootInfo.isSubscriptionRequired()) throw SchemaError("Expected a Subscription object with name '$subscriptionName' but found none!") else null
 
-        val additionalObjects = objects.filter { o -> o != query && o != subscription && o != mutation }
-
-        return SchemaObjects(query, mutation, subscription, (additionalObjects + inputObjects + enums + interfaces + unions).toSet(), codeRegistryBuilder)
+        return SchemaObjects(query, mutation, subscription, (objects + inputObjects + enums + interfaces + unions).toSet(), codeRegistryBuilder)
     }
 
     /**
diff --git a/src/main/kotlin/com/coxautodev/graphql/tools/Utils.kt b/src/main/kotlin/com/coxautodev/graphql/tools/Utils.kt
index ab51f3b..2efd256 100644
--- a/src/main/kotlin/com/coxautodev/graphql/tools/Utils.kt
+++ b/src/main/kotlin/com/coxautodev/graphql/tools/Utils.kt
@@ -6,6 +6,7 @@ import graphql.language.NonNullType
 import graphql.language.ObjectTypeDefinition
 import graphql.language.ObjectTypeExtensionDefinition
 import graphql.language.Type
+import java.lang.reflect.Method
 import java.lang.reflect.ParameterizedType
 
 /**
@@ -33,3 +34,21 @@ internal fun JavaType.unwrap(): Class<out Any> =
         } else {
             this as Class<*>
         }
+
+/**
+ * Simple heuristic to check is a method is a trivial data fetcher.
+ *
+ * Requirements are:
+ * prefixed with get
+ * must have zero parameters
+ */
+internal fun isTrivialDataFetcher(method: Method): Boolean {
+    return (method.parameterCount == 0
+            && (
+            method.name.startsWith("get")
+                    || isBooleanGetter(method)))
+}
+
+private fun isBooleanGetter(method: Method) = (method.name.startsWith("is")
+        && (method.returnType == java.lang.Boolean::class.java)
+        || method.returnType == Boolean::class.java)
\ No newline at end of file


Test patch:
diff --git a/src/test/groovy/com/coxautodev/graphql/tools/GenericResolverSpec.groovy b/src/test/groovy/com/coxautodev/graphql/tools/GenericResolverSpec.groovy
index 6b43a70..ca643a0 100644
--- a/src/test/groovy/com/coxautodev/graphql/tools/GenericResolverSpec.groovy
+++ b/src/test/groovy/com/coxautodev/graphql/tools/GenericResolverSpec.groovy
@@ -16,7 +16,7 @@ class GenericResolverSpec extends Specification {
                             value: String
                         }
                         ''')
-                    .resolvers(new QueryResolver1(), new BarResolver())
+                    .resolvers(new QueryResolver(), new BarResolver())
                     .build()
                     .makeExecutableSchema()
 
@@ -24,7 +24,7 @@ class GenericResolverSpec extends Specification {
             noExceptionThrown()
     }
 
-    class QueryResolver1 implements GraphQLQueryResolver {
+    class QueryResolver implements GraphQLQueryResolver {
         Bar getBar() {
             return new Bar()
         }
@@ -33,7 +33,7 @@ class GenericResolverSpec extends Specification {
     class Bar {
     }
 
-    abstract class FooResolver<T> implements GraphQLResolver<T> {
+    abstract class FooResolver<T> {
         String getValue(T foo) {
             return "value"
         }
@@ -42,43 +42,4 @@ class GenericResolverSpec extends Specification {
     class BarResolver extends FooResolver<Bar> implements GraphQLResolver<Bar> {
 
     }
-
-
-    def "methods from generic inherited resolvers are resolved"() {
-        when:
-        SchemaParser.newParser().schemaString('''\
-                        type Query {
-                            car: Car!
-                        }
-                        type Car {
-                            value: String
-                        }
-                        ''')
-                .resolvers(new QueryResolver2(), new CarResolver())
-                .build()
-                .makeExecutableSchema()
-
-        then:
-        noExceptionThrown()
-    }
-
-
-    class QueryResolver2 implements GraphQLQueryResolver {
-        Car getCar() {
-            return new Car()
-        }
-    }
-
-    abstract class FooGraphQLResolver<T> implements GraphQLResolver<T> {
-        String getValue(T foo) {
-            return "value"
-        }
-    }
-
-    class Car {
-    }
-
-    class CarResolver extends FooGraphQLResolver<Car> {
-
-    }
 }
diff --git a/src/test/java/com/coxautodev/graphql/tools/UtilsTestTrivialDataFetcherBean.java b/src/test/java/com/coxautodev/graphql/tools/UtilsTestTrivialDataFetcherBean.java
new file mode 100644
index 0000000..969f9da
--- /dev/null
+++ b/src/test/java/com/coxautodev/graphql/tools/UtilsTestTrivialDataFetcherBean.java
@@ -0,0 +1,8 @@
+package com.coxautodev.graphql.tools;
+
+public class UtilsTestTrivialDataFetcherBean {
+
+    public boolean isBooleanPrimitive() {
+        return false;
+    }
+}
diff --git a/src/test/kotlin/com/coxautodev/graphql/tools/MethodFieldResolverTest.kt b/src/test/kotlin/com/coxautodev/graphql/tools/MethodFieldResolverTest.kt
deleted file mode 100644
index b10074a..0000000
--- a/src/test/kotlin/com/coxautodev/graphql/tools/MethodFieldResolverTest.kt
+++ /dev/null
@@ -1,112 +0,0 @@
-package com.coxautodev.graphql.tools
-
-import graphql.ExecutionInput
-import graphql.GraphQL
-import graphql.language.StringValue
-import graphql.schema.Coercing
-import graphql.schema.GraphQLScalarType
-import org.junit.Assert
-import org.junit.Test
-
-class MethodFieldResolverTest {
-
-    @Test
-    fun `should handle scalar types as method input argument`() {
-        val schema = SchemaParser.newParser()
-                .schemaString("""
-                    scalar CustomScalar
-                    type Query {
-                        test(input: CustomScalar): Int
-                    }
-                    """.trimIndent()
-                )
-                .scalars(customScalarType)
-                .resolvers(object : GraphQLQueryResolver {
-                    fun test(scalar: CustomScalar) = scalar.value.length
-                })
-                .build()
-                .makeExecutableSchema()
-
-        val gql = GraphQL.newGraphQL(schema).build()
-
-        val result = gql
-                .execute(ExecutionInput.newExecutionInput()
-                        .query("""
-                            query Test(${"$"}input: CustomScalar) {
-                                test(input: ${"$"}input)
-                            }
-                            """.trimIndent())
-                        .variables(mapOf("input" to "FooBar"))
-                        .context(Object())
-                        .root(Object()))
-
-        Assert.assertEquals(6, result.getData<Map<String, Any>>()["test"])
-    }
-
-    @Test
-    fun `should handle lists of scalar types`() {
-        val schema = SchemaParser.newParser()
-                .schemaString("""
-                    scalar CustomScalar
-                    type Query {
-                        test(input: [CustomScalar]): Int
-                    }
-                    """.trimIndent()
-                )
-                .scalars(customScalarType)
-                .resolvers(object : GraphQLQueryResolver {
-                    fun test(scalars: List<CustomScalar>) = scalars.map { it.value.length }.sum()
-                })
-                .build()
-                .makeExecutableSchema()
-
-        val gql = GraphQL.newGraphQL(schema).build()
-
-        val result = gql
-                .execute(ExecutionInput.newExecutionInput()
-                        .query("""
-                            query Test(${"$"}input: [CustomScalar]) {
-                                test(input: ${"$"}input)
-                            }
-                            """.trimIndent())
-                        .variables(mapOf("input" to listOf("Foo", "Bar")))
-                        .context(Object())
-                        .root(Object()))
-
-        Assert.assertEquals(6, result.getData<Map<String, Any>>()["test"])
-    }
-
-    /**
-     * Custom Scalar Class type that doesn't work with Jackson serialization/deserialization
-     */
-    class CustomScalar private constructor(private val internalValue: String) {
-        val value get() = internalValue
-
-        companion object {
-            fun of(input: Any?) = when (input) {
-                is String -> CustomScalar(input)
-                else -> null
-            }
-        }
-    }
-
-    private val customScalarType: GraphQLScalarType = GraphQLScalarType.newScalar()
-            .name("CustomScalar")
-            .description("customScalar")
-            .coercing(object : Coercing<CustomScalar, String> {
-
-                override fun parseValue(input: Any?) = CustomScalar.of(input)
-
-                override fun parseLiteral(input: Any?) = when (input) {
-                    is StringValue -> CustomScalar.of(input.value)
-                    else -> null
-                }
-
-                override fun serialize(dataFetcherResult: Any?) = when (dataFetcherResult) {
-                    is CustomScalar -> dataFetcherResult.value
-                    else -> null
-                }
-            })
-            .build()
-
-}
diff --git a/src/test/kotlin/com/coxautodev/graphql/tools/UtilsTest.kt b/src/test/kotlin/com/coxautodev/graphql/tools/UtilsTest.kt
new file mode 100644
index 0000000..a72bdae
--- /dev/null
+++ b/src/test/kotlin/com/coxautodev/graphql/tools/UtilsTest.kt
@@ -0,0 +1,40 @@
+package com.coxautodev.graphql.tools
+
+import org.junit.Assert
+import org.junit.Test
+
+class UtilsTest {
+
+    @Suppress("unused")
+    class Bean {
+        fun getterValid(): String = ""
+
+        fun getterWithArgument(@Suppress("UNUSED_PARAMETER") arg1: String): String = ""
+
+        internal fun getterInternal(): String = ""
+
+        fun notAGetter(): String = ""
+
+        fun isString(): String = ""
+
+        @Suppress("PLATFORM_CLASS_MAPPED_TO_KOTLIN")
+        fun isJavaBoolean(): java.lang.Boolean = java.lang.Boolean(false)
+
+        fun isKotlinBoolean(): Boolean = false
+    }
+
+    @Test
+    fun `isTrivialDataFetcher`() {
+        val clazz = Bean::class.java
+
+        Assert.assertTrue(isTrivialDataFetcher(clazz.getMethod("getterValid")))
+        Assert.assertFalse(isTrivialDataFetcher(clazz.getMethod("getterWithArgument", String::class.java)))
+        Assert.assertFalse(isTrivialDataFetcher(clazz.getMethod("notAGetter")))
+
+        Assert.assertFalse(isTrivialDataFetcher(clazz.getMethod("isString")))
+        Assert.assertTrue(isTrivialDataFetcher(clazz.getMethod("isJavaBoolean")))
+        Assert.assertTrue(isTrivialDataFetcher(clazz.getMethod("isKotlinBoolean")))
+
+        Assert.assertTrue(isTrivialDataFetcher(UtilsTestTrivialDataFetcherBean::class.java.getMethod("isBooleanPrimitive")))
+    }
+}