Remove support for @Batched

vojtapol · vojtapol · commit 9ed6ea11f1c5 · 2020-06-02T16:54:47.000-04:00
diff --git a/src/main/kotlin/graphql/kickstart/tools/FieldResolverScanner.kt b/src/main/kotlin/graphql/kickstart/tools/FieldResolverScanner.kt
@@ -111,13 +111,9 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) {
 
     private fun verifyMethodArguments(method: java.lang.reflect.Method, requiredCount: Int, search: Search): Boolean {
         val appropriateFirstParameter = if (search.requiredFirstParameterType != null) {
-            if (MethodFieldResolver.isBatched(method, search)) {
-                verifyBatchedMethodFirstArgument(method.genericParameterTypes.firstOrNull(), search.requiredFirstParameterType)
-            } else {
-                method.genericParameterTypes.firstOrNull()?.let {
-                    it == search.requiredFirstParameterType || method.declaringClass.typeParameters.contains(it)
-                } ?: false
-            }
+            method.genericParameterTypes.firstOrNull()?.let {
+                it == search.requiredFirstParameterType || method.declaringClass.typeParameters.contains(it)
+            } ?: false
         } else {
             true
         }
@@ -147,24 +143,6 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) {
         }
     }
 
-    private fun verifyBatchedMethodFirstArgument(firstType: JavaType?, requiredFirstParameterType: Class<*>?): Boolean {
-        if (firstType == null) {
-            return false
-        }
-
-        if (firstType !is ParameterizedType) {
-            return false
-        }
-
-        if (!TypeClassMatcher.isListType(firstType, GenericType(firstType, options))) {
-            return false
-        }
-
-        val typeArgument = firstType.actualTypeArguments.first() as? Class<*> ?: return false
-
-        return typeArgument == requiredFirstParameterType
-    }
-
     private fun findResolverProperty(field: FieldDefinition, search: Search) =
         FieldUtils.getAllFields(search.type.unwrap()).find { it.name == field.name }
 
@@ -207,7 +185,12 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) {
         return signatures
     }
 
-    data class Search(val type: JavaType, val resolverInfo: ResolverInfo, val source: Any?, val requiredFirstParameterType: Class<*>? = null, val allowBatched: Boolean = false)
+    data class Search(
+        val type: JavaType,
+        val resolverInfo: ResolverInfo,
+        val source: Any?,
+        val requiredFirstParameterType: Class<*>? = null
+    )
 }
 
 internal class FieldResolverError(msg: String) : RuntimeException(msg)
diff --git a/src/main/kotlin/graphql/kickstart/tools/MethodFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/MethodFieldResolver.kt
@@ -2,7 +2,6 @@ package graphql.kickstart.tools
 
 import com.fasterxml.jackson.core.type.TypeReference
 import graphql.TrivialDataFetcher
-import graphql.execution.batched.Batched
 import graphql.kickstart.tools.SchemaParserOptions.GenericWrapper
 import graphql.kickstart.tools.util.JavaType
 import graphql.kickstart.tools.util.coroutineScope
@@ -32,19 +31,6 @@ internal class MethodFieldResolver(
     val method: Method
 ) : FieldResolver(field, search, options, search.type) {
 
-    companion object {
-        fun isBatched(method: Method, search: FieldResolverScanner.Search): Boolean {
-            if (method.getAnnotation(Batched::class.java) != null) {
-                if (!search.allowBatched) {
-                    throw ResolverError("The @Batched annotation is only allowed on non-root resolver methods, but it was found on ${search.type.unwrap().name}#${method.name}!")
-                }
-
-                return true
-            }
-            return false
-        }
-    }
-
     private val additionalLastArgument =
         try {
             method.kotlinFunction?.valueParameters?.size ?: method.parameterCount == (field.inputValueDefinitions.size + getIndexOffset() + 1)
@@ -53,17 +39,16 @@ internal class MethodFieldResolver(
         }
 
     override fun createDataFetcher(): DataFetcher<*> {
-        val batched = isBatched(method, search)
         val args = mutableListOf<ArgumentPlaceholder>()
         val mapper = options.objectMapperProvider.provide(field)
 
         // Add source argument if this is a resolver (but not a root resolver)
         if (this.search.requiredFirstParameterType != null) {
-            val expectedType = if (batched) Iterable::class.java else this.search.requiredFirstParameterType
+            val expectedType = this.search.requiredFirstParameterType
 
             args.add { environment ->
                 val source = environment.getSource<Any>()
-                if (!(expectedType.isAssignableFrom(source.javaClass))) {
+                if (!expectedType.isAssignableFrom(source.javaClass)) {
                     throw ResolverError("Source type (${source.javaClass.name}) is not expected type (${expectedType.name})!")
                 }
 
@@ -117,14 +102,10 @@ internal class MethodFieldResolver(
             }
         }
 
-        return if (batched) {
-            BatchedMethodFieldResolverDataFetcher(getSourceResolver(), this.method, args, options)
+        return if (args.isEmpty() && isTrivialDataFetcher(this.method)) {
+            TrivialMethodFieldResolverDataFetcher(getSourceResolver(), this.method, args, options)
         } else {
-            if (args.isEmpty() && isTrivialDataFetcher(this.method)) {
-                TrivialMethodFieldResolverDataFetcher(getSourceResolver(), this.method, args, options)
-            } else {
-                MethodFieldResolverDataFetcher(getSourceResolver(), this.method, args, options)
-            }
+            MethodFieldResolverDataFetcher(getSourceResolver(), this.method, args, options)
         }
     }
 
@@ -146,16 +127,15 @@ internal class MethodFieldResolver(
         }
 
     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 returnValueMatch = TypeClassMatcher.PotentialMatch.returnValue(field.type, unwrappedGenericType, genericType, SchemaClassScanner.ReturnValueReference(method), batched)
+        val returnValueMatch = TypeClassMatcher.PotentialMatch.returnValue(field.type, unwrappedGenericType, genericType, SchemaClassScanner.ReturnValueReference(method))
 
         return field.inputValueDefinitions.mapIndexed { i, inputDefinition ->
-            TypeClassMatcher.PotentialMatch.parameterType(inputDefinition.type, getJavaMethodParameterType(i)!!, genericType, SchemaClassScanner.MethodParameterReference(method, i), batched)
+            TypeClassMatcher.PotentialMatch.parameterType(inputDefinition.type, getJavaMethodParameterType(i)!!, genericType, SchemaClassScanner.MethodParameterReference(method, i))
         } + listOf(returnValueMatch)
     }
 
@@ -238,12 +218,13 @@ internal open class MethodFieldResolverDataFetcher(
     }
 }
 
-internal open class TrivialMethodFieldResolverDataFetcher(
+internal class TrivialMethodFieldResolverDataFetcher(
     sourceResolver: SourceResolver,
     method: Method,
     args: List<ArgumentPlaceholder>,
     options: SchemaParserOptions
-) : MethodFieldResolverDataFetcher(sourceResolver, method, args, options), TrivialDataFetcher<Any>
+) : MethodFieldResolverDataFetcher(sourceResolver, method, args, options),
+    TrivialDataFetcher<Any> // just to mark it for tracing and optimizations
 
 private suspend inline fun invokeSuspend(target: Any, resolverMethod: Method, args: Array<Any?>): Any? {
     return suspendCoroutineUninterceptedOrReturn { continuation ->
@@ -268,15 +249,5 @@ private inline fun invoke(method: Method, instance: Any, args: Array<Any?>): Any
     }
 }
 
-internal class BatchedMethodFieldResolverDataFetcher(
-    sourceResolver: SourceResolver,
-    method: Method,
-    args: List<ArgumentPlaceholder>,
-    options: SchemaParserOptions
-) : MethodFieldResolverDataFetcher(sourceResolver, method, args, options) {
-    @Batched
-    override fun get(environment: DataFetchingEnvironment) = super.get(environment)
-}
-
 internal typealias ArgumentPlaceholder = (DataFetchingEnvironment) -> Any?
 
diff --git a/src/main/kotlin/graphql/kickstart/tools/PropertyFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/PropertyFieldResolver.kt
@@ -25,8 +25,8 @@ internal class PropertyFieldResolver(
                 field.type,
                 property.genericType,
                 genericType,
-                SchemaClassScanner.FieldTypeReference(property.toString()),
-                false))
+                SchemaClassScanner.FieldTypeReference(property.toString()))
+        )
     }
 
     override fun toString() = "PropertyFieldResolver{property=$property}"
diff --git a/src/main/kotlin/graphql/kickstart/tools/PropertyMapResolver.kt b/src/main/kotlin/graphql/kickstart/tools/PropertyMapResolver.kt
@@ -38,7 +38,7 @@ internal class PropertyMapResolver(
     }
 
     override fun scanForMatches(): List<TypeClassMatcher.PotentialMatch> {
-        return listOf(TypeClassMatcher.PotentialMatch.returnValue(field.type, mapGenericValue, genericType, SchemaClassScanner.FieldTypeReference(field.name), false))
+        return listOf(TypeClassMatcher.PotentialMatch.returnValue(field.type, mapGenericValue, genericType, SchemaClassScanner.FieldTypeReference(field.name)))
     }
 
     override fun toString() = "PropertyMapResolverDataFetcher{key=${field.name}}"
diff --git a/src/main/kotlin/graphql/kickstart/tools/ResolverInfo.kt b/src/main/kotlin/graphql/kickstart/tools/ResolverInfo.kt
@@ -41,7 +41,7 @@ internal class NormalResolverInfo(
 
     override fun getFieldSearches(): List<FieldResolverScanner.Search> {
         return listOf(
-            FieldResolverScanner.Search(resolverType, this, resolver, dataClassType, true),
+            FieldResolverScanner.Search(resolverType, this, resolver, dataClassType),
             FieldResolverScanner.Search(dataClassType, this, null)
         )
     }
@@ -66,7 +66,7 @@ internal class MultiResolverInfo(val resolverInfoList: List<NormalResolverInfo>)
     override fun getFieldSearches(): List<FieldResolverScanner.Search> {
         return resolverInfoList
             .asSequence()
-            .map { FieldResolverScanner.Search(it.resolverType, this, it.resolver, dataClassType, true) }
+            .map { FieldResolverScanner.Search(it.resolverType, this, it.resolver, dataClassType) }
             .plus(FieldResolverScanner.Search(dataClassType, this, null))
             .toList()
     }
diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt
@@ -113,9 +113,11 @@ internal class SchemaClassScanner(
     }
 
     private fun validateAndCreateResult(rootTypeHolder: RootTypesHolder): ScannedSchemaObjects {
-        initialDictionary.filter { !it.value.accessed }.forEach {
-            log.warn("Dictionary mapping was provided but never used, and can be safely deleted: \"${it.key}\" -> ${it.value.get().name}")
-        }
+        initialDictionary
+            .filter { !it.value.accessed }
+            .forEach {
+                log.warn("Dictionary mapping was provided but never used, and can be safely deleted: \"${it.key}\" -> ${it.value.get().name}")
+            }
 
         val observedDefinitions = dictionary.keys.toSet() + unvalidatedTypes
 
@@ -125,35 +127,37 @@ internal class SchemaClassScanner(
         // Union types can also be excluded, as their possible types are resolved recursively later
         val dictionary = try {
             BiMap.unmodifiableBiMap(BiMap.create<TypeDefinition<*>, JavaType>().also {
-                dictionary.filter {
-                    it.value.javaType != null
-                        && it.value.typeClass() != java.lang.Object::class.java
-                        && !java.util.Map::class.java.isAssignableFrom(it.value.typeClass())
-                        && it.key !is InputObjectTypeDefinition
-                        && it.key !is UnionTypeDefinition
-                }.mapValuesTo(it) { it.value.javaType }
+                dictionary
+                    .filter {
+                        it.value.javaType != null
+                            && it.value.typeClass() != java.lang.Object::class.java
+                            && !java.util.Map::class.java.isAssignableFrom(it.value.typeClass())
+                            && it.key !is InputObjectTypeDefinition
+                            && it.key !is UnionTypeDefinition
+                    }.mapValuesTo(it) { it.value.javaType }
             })
         } catch (t: Throwable) {
             throw SchemaClassScannerError("Error creating bimap of type => class", t)
         }
         val scalarDefinitions = observedDefinitions.filterIsInstance<ScalarTypeDefinition>()
 
         // Ensure all scalar definitions have implementations and add the definition to those.
-        val scalars = scalarDefinitions.filter {
-            // Filter for any defined scalars OR scalars that aren't defined but also aren't standard
-            scalars.containsKey(it.name) || !ScalarInfo.STANDARD_SCALAR_DEFINITIONS.containsKey(it.name)
-        }.map { definition ->
-            val provided = scalars[definition.name]
-                ?: throw SchemaClassScannerError("Expected a user-defined GraphQL scalar type with name '${definition.name}' but found none!")
-            GraphQLScalarType.newScalar()
-                .name(provided.name)
-                .description(
-                    if (definition.description != null) definition.description.content
-                    else SchemaParser.getDocumentation(definition) ?: provided.description)
-                .coercing(provided.coercing)
-                .definition(definition)
-                .build()
-        }.associateBy { it.name!! }
+        val scalars = scalarDefinitions
+            .filter {
+                // Filter for any defined scalars OR scalars that aren't defined but also aren't standard
+                scalars.containsKey(it.name) || !ScalarInfo.STANDARD_SCALAR_DEFINITIONS.containsKey(it.name)
+            }.map { definition ->
+                val provided = scalars[definition.name]
+                    ?: throw SchemaClassScannerError("Expected a user-defined GraphQL scalar type with name '${definition.name}' but found none!")
+                GraphQLScalarType.newScalar()
+                    .name(provided.name)
+                    .description(
+                        if (definition.description != null) definition.description.content
+                        else SchemaParser.getDocumentation(definition) ?: provided.description)
+                    .coercing(provided.coercing)
+                    .definition(definition)
+                    .build()
+            }.associateBy { it.name!! }
 
         val unusedDefinitions = (definitionsByName.values - observedDefinitions).toSet()
         unusedDefinitions
@@ -343,8 +347,7 @@ internal class SchemaClassScanner(
                                 inputValueDefinition.type,
                                 inputValueJavaType,
                                 GenericType(javaType, options).relativeToType(inputValueJavaType),
-                                InputObjectReference(inputValueDefinition),
-                                false
+                                InputObjectReference(inputValueDefinition)
                             )))
                         } else {
                             var mappingAdvice = "Try adding it manually to the dictionary"
diff --git a/src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt b/src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt
@@ -20,11 +20,7 @@ internal class TypeClassMatcher(private val definitionsByName: Map<String, TypeD
     private fun error(potentialMatch: PotentialMatch, msg: String) = SchemaClassScannerError("Unable to match type definition (${potentialMatch.graphQLType}) with java type (${potentialMatch.javaType}): $msg")
 
     fun match(potentialMatch: PotentialMatch): Match {
-        return if (potentialMatch.batched) {
-            match(stripBatchedType(potentialMatch)) // stripBatchedType sets 'batched' to false
-        } else {
-            match(potentialMatch, potentialMatch.graphQLType, potentialMatch.javaType, true)
-        }
+        return match(potentialMatch, potentialMatch.graphQLType, potentialMatch.javaType, true)
     }
 
     private fun match(potentialMatch: PotentialMatch, graphQLType: GraphQLLangType, javaType: JavaType, root: Boolean = false): Match {
@@ -101,34 +97,30 @@ internal class TypeClassMatcher(private val definitionsByName: Map<String, TypeD
 
     private fun isListType(realType: ParameterizedType, potentialMatch: PotentialMatch) = isListType(realType, potentialMatch.generic)
 
-    private fun stripBatchedType(potentialMatch: PotentialMatch): PotentialMatch {
-        return if (potentialMatch.location == Location.PARAMETER_TYPE) {
-            potentialMatch.copy(javaType = potentialMatch.javaType, batched = false)
-        } else {
-            val realType = potentialMatch.generic.unwrapGenericType(potentialMatch.javaType)
-            if (realType is ParameterizedType && isListType(realType, potentialMatch)) {
-                potentialMatch.copy(javaType = realType.actualTypeArguments.first(), batched = false)
-            } else {
-                throw error(potentialMatch, "Method was marked as @Batched but ${potentialMatch.location.prettyName} was not a list!")
-            }
-        }
-    }
-
     internal interface Match
+
     internal data class ScalarMatch(val type: ScalarTypeDefinition) : Match
+
     internal data class ValidMatch(val type: TypeDefinition<*>, val javaType: JavaType, val reference: SchemaClassScanner.Reference) : Match
+
     internal enum class Location(val prettyName: String) {
         RETURN_TYPE("return type"),
         PARAMETER_TYPE("parameter"),
     }
 
-    internal data class PotentialMatch(val graphQLType: GraphQLLangType, val javaType: JavaType, val generic: GenericType.RelativeTo, val reference: SchemaClassScanner.Reference, val location: Location, val batched: Boolean) {
+    internal data class PotentialMatch(
+        val graphQLType: GraphQLLangType,
+        val javaType: JavaType,
+        val generic: GenericType.RelativeTo,
+        val reference: SchemaClassScanner.Reference,
+        val location: Location
+    ) {
         companion object {
-            fun returnValue(graphQLType: GraphQLLangType, javaType: JavaType, generic: GenericType.RelativeTo, reference: SchemaClassScanner.Reference, batched: Boolean) =
-                PotentialMatch(graphQLType, javaType, generic, reference, Location.RETURN_TYPE, batched)
+            fun returnValue(graphQLType: GraphQLLangType, javaType: JavaType, generic: GenericType.RelativeTo, reference: SchemaClassScanner.Reference) =
+                PotentialMatch(graphQLType, javaType, generic, reference, Location.RETURN_TYPE)
 
-            fun parameterType(graphQLType: GraphQLLangType, javaType: JavaType, generic: GenericType.RelativeTo, reference: SchemaClassScanner.Reference, batched: Boolean) =
-                PotentialMatch(graphQLType, javaType, generic, reference, Location.PARAMETER_TYPE, batched)
+            fun parameterType(graphQLType: GraphQLLangType, javaType: JavaType, generic: GenericType.RelativeTo, reference: SchemaClassScanner.Reference) =
+                PotentialMatch(graphQLType, javaType, generic, reference, Location.PARAMETER_TYPE)
         }
     }
 }
diff --git a/src/test/groovy/graphql/kickstart/tools/EndToEndSpec.groovy b/src/test/groovy/graphql/kickstart/tools/EndToEndSpec.groovy
diff --git a/src/test/groovy/graphql/kickstart/tools/SchemaClassScannerSpec.groovy b/src/test/groovy/graphql/kickstart/tools/SchemaClassScannerSpec.groovy
diff --git a/src/test/kotlin/graphql/kickstart/tools/EndToEndSpecHelper.kt b/src/test/kotlin/graphql/kickstart/tools/EndToEndSpecHelper.kt

Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,7 @@ internal class PropertyMapResolver(`
`38`	`38`	`}`
`39`	`39`
`40`	`40`	`override fun scanForMatches(): List<TypeClassMatcher.PotentialMatch> {`
`41`		`- return listOf(TypeClassMatcher.PotentialMatch.returnValue(field.type, mapGenericValue, genericType, SchemaClassScanner.FieldTypeReference(field.name), false))`
	`41`	`+ return listOf(TypeClassMatcher.PotentialMatch.returnValue(field.type, mapGenericValue, genericType, SchemaClassScanner.FieldTypeReference(field.name)))`
`42`	`42`	`}`
`43`	`43`
`44`	`44`	`override fun toString() = "PropertyMapResolverDataFetcher{key=${field.name}}"`
Original file line number	Diff line number	Diff line change
`@@ -41,7 +41,7 @@ internal class NormalResolverInfo(`
`41`	`41`
`42`	`42`	`override fun getFieldSearches(): List<FieldResolverScanner.Search> {`
`43`	`43`	`return listOf(`
`44`		`- FieldResolverScanner.Search(resolverType, this, resolver, dataClassType, true),`
	`44`	`+ FieldResolverScanner.Search(resolverType, this, resolver, dataClassType),`
`45`	`45`	`FieldResolverScanner.Search(dataClassType, this, null)`
`46`	`46`	`)`
`47`	`47`	`}`
`@@ -66,7 +66,7 @@ internal class MultiResolverInfo(val resolverInfoList: List<NormalResolverInfo>)`
`66`	`66`	`override fun getFieldSearches(): List<FieldResolverScanner.Search> {`
`67`	`67`	`return resolverInfoList`
`68`	`68`	`.asSequence()`
`69`		`- .map { FieldResolverScanner.Search(it.resolverType, this, it.resolver, dataClassType, true) }`
	`69`	`+ .map { FieldResolverScanner.Search(it.resolverType, this, it.resolver, dataClassType) }`
`70`	`70`	`.plus(FieldResolverScanner.Search(dataClassType, this, null))`
`71`	`71`	`.toList()`
`72`	`72`	`}`