From 5f68c41ea8b9f2f075a3b14d4c5fa69946fdaefe Mon Sep 17 00:00:00 2001 From: Oryan Date: Thu, 5 Nov 2026 21:47:20 -0500 Subject: [PATCH 01/10] Build one resolver plan per field and delete the resolver hierarchy ResolverPlan.kt replaces FieldResolverScanner, FieldResolver and the method, property, Map and missing resolvers. Its nested Finder picks the member behind a field, and the plan reads that member's signature once: the argument types, the extra parameters, the source and the return shape. The scan, the data fetcher, the proxy and parameter checks and UnmappedInputFieldChecker read the plan instead of the Method. Member variants are private to the file and only expose a raw read. The plan builds every data fetcher and unwraps every value through TypeShape. The resolver kinds and the extra parameters are enums whose entries the finder and the binding iterate, and the subscription Publisher check and the argument Optional and concrete scalar checks move next to the type walk in TypeShape.kt. SourceResolver keeps its public interface in its own file. ResolverOwnershipTest keeps member reflection in ResolverPlan.kt, container dispatch in TypeShape.kt and TokenBuffer conversion in ArgumentConverter.kt. Messages, lookup order and laziness are unchanged. --- .../graphql/kickstart/tools/ResolverInfo.kt | 34 +- .../kickstart/tools/ScannedSchemaObjects.kt | 4 +- .../kickstart/tools/SchemaClassScanner.kt | 84 +-- .../graphql/kickstart/tools/SchemaParser.kt | 7 +- .../tools/UnmappedInputFieldChecker.kt | 13 +- .../kickstart/tools/resolver/FieldResolver.kt | 49 -- .../tools/resolver/FieldResolverScanner.kt | 292 -------- .../tools/resolver/MapFieldResolver.kt | 76 -- .../tools/resolver/MethodFieldResolver.kt | 253 ------- .../tools/resolver/MissingFieldResolver.kt | 17 - .../tools/resolver/PropertyFieldResolver.kt | 50 -- .../kickstart/tools/resolver/ResolverPlan.kt | 670 ++++++++++++++++++ .../tools/resolver/SourceResolver.kt | 8 + .../kickstart/tools/resolver/TypeShape.kt | 45 +- .../graphql/kickstart/tools/util/Utils.kt | 24 - .../MethodFieldResolverDataFetcherTest.kt | 4 +- .../tools/ResolverCompatibilityTest.kt | 56 ++ .../kickstart/tools/ResolverOwnershipTest.kt | 176 +++++ ...lverScannerTest.kt => ResolverPlanTest.kt} | 72 +- .../kickstart/tools/TypeClassMatcherTest.kt | 6 +- 20 files changed, 1040 insertions(+), 900 deletions(-) delete mode 100644 src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolver.kt delete mode 100644 src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt delete mode 100644 src/main/kotlin/graphql/kickstart/tools/resolver/MapFieldResolver.kt delete mode 100644 src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt delete mode 100644 src/main/kotlin/graphql/kickstart/tools/resolver/MissingFieldResolver.kt delete mode 100644 src/main/kotlin/graphql/kickstart/tools/resolver/PropertyFieldResolver.kt create mode 100644 src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt create mode 100644 src/main/kotlin/graphql/kickstart/tools/resolver/SourceResolver.kt create mode 100644 src/test/kotlin/graphql/kickstart/tools/ResolverOwnershipTest.kt rename src/test/kotlin/graphql/kickstart/tools/{FieldResolverScannerTest.kt => ResolverPlanTest.kt} (58%) diff --git a/src/main/kotlin/graphql/kickstart/tools/ResolverInfo.kt b/src/main/kotlin/graphql/kickstart/tools/ResolverInfo.kt index 638bdfb8..b9fba675 100644 --- a/src/main/kotlin/graphql/kickstart/tools/ResolverInfo.kt +++ b/src/main/kotlin/graphql/kickstart/tools/ResolverInfo.kt @@ -1,6 +1,5 @@ package graphql.kickstart.tools -import graphql.kickstart.tools.resolver.FieldResolverScanner import graphql.kickstart.tools.util.GraphQLRootResolver import graphql.kickstart.tools.util.JavaType import graphql.kickstart.tools.util.eraseUnboundedWildcards @@ -8,7 +7,7 @@ import org.apache.commons.lang3.reflect.TypeUtils import java.lang.reflect.ParameterizedType internal abstract class ResolverInfo { - abstract fun getFieldSearches(): List + abstract fun getFieldSearches(): List fun getRealResolverClass(resolver: GraphQLResolver<*>, options: SchemaParserOptions) = options.proxyHandlers.find { it.canHandle(resolver) } @@ -43,10 +42,10 @@ internal class NormalResolverInfo( return type } - override fun getFieldSearches(): List { + override fun getFieldSearches(): List { return listOf( - FieldResolverScanner.Search(resolverType, this, resolver, dataClassType), - FieldResolverScanner.Search(dataClassType, this, null) + ResolverSearch(resolverType, this, resolver, dataClassType), + ResolverSearch(dataClassType, this, null) ) } } @@ -60,11 +59,11 @@ internal class MultiResolverInfo( private val dataClass: JavaType ) : ResolverInfo() { - override fun getFieldSearches(): List { + override fun getFieldSearches(): List { return resolverInfoList .asSequence() - .map { FieldResolverScanner.Search(it.resolverType, this, it.resolver, it.dataClassType) } - .plus(FieldResolverScanner.Search(dataClass, this, null)) + .map { ResolverSearch(it.resolverType, this, it.resolver, it.dataClassType) } + .plus(ResolverSearch(dataClass, this, null)) .toList() } } @@ -75,16 +74,29 @@ internal class RootResolverInfo( val isSubscription: Boolean = false ) : ResolverInfo() { override fun getFieldSearches() = - resolvers.map { FieldResolverScanner.Search(getRealResolverClass(it, options), this, it) } + resolvers.map { ResolverSearch(getRealResolverClass(it, options), this, it) } } internal class DataClassResolverInfo(private val dataClass: JavaType) : ResolverInfo() { override fun getFieldSearches() = - listOf(FieldResolverScanner.Search(dataClass, this, null)) + listOf(ResolverSearch(dataClass, this, null)) } internal class MissingResolverInfo : ResolverInfo() { - override fun getFieldSearches(): List = listOf() + override fun getFieldSearches(): List = listOf() +} + +internal data class ResolverSearch( + val type: JavaType, + val resolverInfo: ResolverInfo, + val source: Any?, + val requiredFirstParameterType: Class<*>? = null +) { + val isSubscription get() = resolverInfo is RootResolverInfo && resolverInfo.isSubscription + + val firstArgumentIndex get() = if (requiredFirstParameterType != null) 1 else 0 + + fun withoutSourceParameter() = copy(requiredFirstParameterType = null) } internal class ResolverError(message: String, cause: Throwable? = null) : RuntimeException(message, cause) diff --git a/src/main/kotlin/graphql/kickstart/tools/ScannedSchemaObjects.kt b/src/main/kotlin/graphql/kickstart/tools/ScannedSchemaObjects.kt index a17009b1..133851a5 100644 --- a/src/main/kotlin/graphql/kickstart/tools/ScannedSchemaObjects.kt +++ b/src/main/kotlin/graphql/kickstart/tools/ScannedSchemaObjects.kt @@ -1,6 +1,6 @@ package graphql.kickstart.tools -import graphql.kickstart.tools.resolver.FieldResolver +import graphql.kickstart.tools.resolver.ResolverPlan import graphql.kickstart.tools.util.JavaType import graphql.language.FieldDefinition import graphql.schema.GraphQLScalarType @@ -13,7 +13,7 @@ internal data class ScannedSchemaObjects( val definitions: MergedDefinitions, val types: Set>, val customScalars: CustomScalarMap, - val fieldResolversByType: Map>, + val resolverPlansByType: Map>, val unusedTypes: Set>, val directiveOnlyEnums: Set ) diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt index db08b8fb..dfbe38b4 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt @@ -5,21 +5,13 @@ import graphql.kickstart.tools.ScanDictionary.FieldTypeReference import graphql.kickstart.tools.ScanDictionary.InitialDictionaryEntry import graphql.kickstart.tools.ScanDictionary.UnionMemberReference import graphql.kickstart.tools.ScanDictionary.UnusedTypeReference -import graphql.kickstart.tools.resolver.FieldResolver import graphql.kickstart.tools.resolver.FieldResolverError -import graphql.kickstart.tools.resolver.FieldResolverScanner -import graphql.kickstart.tools.resolver.MethodFieldResolver -import graphql.kickstart.tools.resolver.PropertyFieldResolver +import graphql.kickstart.tools.resolver.ResolverPlan import graphql.kickstart.tools.util.* import graphql.language.* import graphql.schema.idl.ScalarInfo -import graphql.schema.idl.TypeUtil import org.apache.commons.lang3.ClassUtils -import org.apache.commons.lang3.reflect.TypeUtils import org.slf4j.LoggerFactory -import java.lang.reflect.Field -import java.lang.reflect.Member -import java.lang.reflect.Modifier /** * @author Andrew Potter @@ -46,15 +38,15 @@ internal class SchemaClassScanner( private val typesByName = definitions.byName private val objectTypesByName = definitions.objectsWithRepeats.associateBy { it.name } - private val fieldResolverScanner = FieldResolverScanner(options, definitions.scalarNames) + private val resolverPlans = ResolverPlan.Finder(options, definitions.scalarNames) private val typeClassMatcher = TypeClassMatcher(definitions) private val unvalidatedTypes = mutableSetOf>() private val queue = linkedSetOf() private val dictionary = ScanDictionary(options, log, typesByName, typeClassMatcher, this.initialDictionary, ::enqueue, ::handleFoundScalarType) private val sealedSubclassMatcher = SealedSubclassMatcher(dictionary) - private val fieldResolversByType = mutableMapOf>() - private val notCallableOnResolver = mutableListOf() + private val resolverPlansByType = mutableMapOf>() + private val notCallableOnResolver = mutableListOf() init { initialDictionary.forEach { (name, clazz) -> @@ -270,17 +262,17 @@ internal class SchemaClassScanner( log.warn("Schema type was defined but can never be accessed, and can be safely deleted: ${definition.name}") } - val fieldResolvers = fieldResolversByType.flatMap { entry -> entry.value.map { it.value } } - val observedNormalResolverInfos = fieldResolvers.map { it.resolverInfo }.filterIsInstance().toSet() - val observedMultiResolverInfos = fieldResolvers.map { it.resolverInfo }.filterIsInstance().flatMap { it.resolverInfoList }.toSet() + val resolverPlans = resolverPlansByType.flatMap { entry -> entry.value.map { it.value } } + val observedNormalResolverInfos = resolverPlans.map { it.resolverInfo }.filterIsInstance().toSet() + val observedMultiResolverInfos = resolverPlans.map { it.resolverInfo }.filterIsInstance().flatMap { it.resolverInfoList }.toSet() (resolverInfos - observedNormalResolverInfos - observedMultiResolverInfos).forEach { resolverInfo -> log.warn("Resolver was provided but no methods on it were used in data fetchers, and can be safely deleted: ${resolverInfo.resolver}") } - validateRootResolversWereUsed(rootTypeHolder.query, fieldResolvers) - validateRootResolversWereUsed(rootTypeHolder.mutation, fieldResolvers) - validateRootResolversWereUsed(rootTypeHolder.subscription, fieldResolvers) + validateRootResolversWereUsed(rootTypeHolder.query, resolverPlans) + validateRootResolversWereUsed(rootTypeHolder.mutation, resolverPlans) + validateRootResolversWereUsed(rootTypeHolder.subscription, resolverPlans) // The enums in unvalidatedTypes come from directives without a class, so they can only be built when no field uses them. // A field hands the enum's values to user code even when the scanner found no class through it, e.g. an input field bound @@ -288,15 +280,15 @@ internal class SchemaClassScanner( val typeNamesUsedByFields = typeNamesUsedByFields(observedDefinitions) val directiveOnlyEnums = unvalidatedTypes.filterIsInstance().filter { it.name !in typeNamesUsedByFields }.toSet() - return ScannedSchemaObjects(dictionary, definitions, observedDefinitions, scalars, fieldResolversByType.toMap(), unusedDefinitions, directiveOnlyEnums) + return ScannedSchemaObjects(dictionary, definitions, observedDefinitions, scalars, resolverPlansByType.toMap(), unusedDefinitions, directiveOnlyEnums) } - private fun validateRootResolversWereUsed(rootType: RootType?, fieldResolvers: List) { + private fun validateRootResolversWereUsed(rootType: RootType?, resolverPlans: List) { if (rootType == null) { return } - val observedRootTypes = fieldResolvers.asSequence().filter { it.resolverInfo is RootResolverInfo && it.resolverInfo == rootType.resolverInfo }.map { it.search.type }.toSet() + val observedRootTypes = resolverPlans.asSequence().filter { it.resolverInfo is RootResolverInfo && it.resolverInfo == rootType.resolverInfo }.map { it.search.type }.toSet() rootType.resolvers.forEach { resolver -> if (rootType.resolverInfo.getRealResolverClass(resolver, options) !in observedRootTypes) { log.warn("Root ${rootType.name} resolver was provided but no methods on it were used in data fetchers for GraphQL type '${rootType.type.name}'! Either remove the ${rootType.resolverInterface.name} interface from the resolver or remove the resolver entirely: $resolver") @@ -424,36 +416,18 @@ internal class SchemaClassScanner( private fun scanResolverInfoForPotentialMatches(type: MergedType.Object, resolverInfo: ResolverInfo) { type.fields.forEach { field -> - val fieldResolver = fieldResolverScanner.findFieldResolver(field, resolverInfo) - collectIfNotCallableOnResolver(type, fieldResolver) + val plan = resolverPlans.find(field, resolverInfo) + plan.notCallableOnResolver(type.name)?.let { notCallableOnResolver += it } - fieldResolversByType.getOrPut(type) { mutableMapOf() }[fieldResolver.field] = fieldResolver + resolverPlansByType.getOrPut(type) { mutableMapOf() }[plan.field] = plan - fieldResolver.scanForMatches().forEach { shape -> + plan.typeShapes().forEach { shape -> dictionary.handleFoundType(typeClassMatcher.match(shape)) } - requireParameterForEachArgument(type, fieldResolver) + plan.requireParameterForEachArgument(type.name) } } - private fun collectIfNotCallableOnResolver(type: MergedType.Object, fieldResolver: FieldResolver) { - val source = fieldResolver.search.source ?: return - val member: Member = when (fieldResolver) { - is MethodFieldResolver -> fieldResolver.method - is PropertyFieldResolver -> fieldResolver.property - else -> return - } - if (Modifier.isStatic(member.modifiers) || member.declaringClass.isInstance(source)) { - return - } - - notCallableOnResolver += NotCallableOnResolver( - "Field '${type.name}.${fieldResolver.field.name}' resolves to ${member.declaringClass.name}.${member.name}, but the resolver is a " + - "${source.javaClass.name}, which isn't a ${member.declaringClass.name}, so every query of the field will fail.", - member is Field - ) - } - private fun throwIfNotCallableOnResolver() { if (notCallableOnResolver.isEmpty()) { return @@ -466,26 +440,6 @@ internal class SchemaClassScanner( ) } - private fun requireParameterForEachArgument(type: MergedType.Object, fieldResolver: FieldResolver) { - if (fieldResolver !is MethodFieldResolver) { - return - } - - fieldResolver.field.inputValueDefinitions.forEachIndexed { index, argument -> - val parameter = fieldResolver.getMethodParameterType(index)?.let { TypeUtils.getRawType(it, fieldResolver.search.type) } ?: return@forEachIndexed - if (!fieldResolverScanner.isContextParameter(parameter, argument)) { - return@forEachIndexed - } - - val method = fieldResolver.method - throw FieldResolverError( - "Field '${type.name}.${fieldResolver.field.name}' has no parameter for its argument '${argument.name}: " + - "${TypeUtil.simplePrint(argument.type)}' in ${method.declaringClass.name}.${method.name}, so the ${parameter.simpleName} " + - "parameter gets the argument instead. Add a parameter for each argument, in order, before the ${parameter.simpleName} parameter." - ) - } - } - private fun handleFoundScalarType(type: MergedType.Scalar) { unvalidatedTypes.add(type) } @@ -496,8 +450,6 @@ internal class SchemaClassScanner( private data class QueueItem(val type: MergedType.Object, val clazz: JavaType) - private data class NotCallableOnResolver(val message: String, val isField: Boolean) - private class RootTypesHolder( options: SchemaParserOptions, rootInfo: RootTypeInfo, diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt index 501a41bc..5f941d24 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt @@ -6,7 +6,6 @@ import graphql.introspection.Introspection.DirectiveLocation.ENUM_VALUE import graphql.introspection.Introspection.DirectiveLocation.FIELD_DEFINITION import graphql.introspection.Introspection.DirectiveLocation.INPUT_FIELD_DEFINITION import graphql.kickstart.tools.directive.DirectiveWiringHelper -import graphql.kickstart.tools.resolver.MethodFieldResolver import graphql.kickstart.tools.util.describe import graphql.kickstart.tools.util.getDocumentation import graphql.kickstart.tools.util.unwrap @@ -39,7 +38,7 @@ class SchemaParser internal constructor( private val types = scanResult.types private val customScalars = scanResult.customScalars private val rootInfo = definitions.rootInfo - private val fieldResolversByType = scanResult.fieldResolversByType + private val resolverPlansByType = scanResult.resolverPlansByType private val unusedDefinitions = definitions.definitionsOf(scanResult.unusedTypes) private val directiveOnlyEnums = scanResult.directiveOnlyEnums @@ -120,7 +119,7 @@ class SchemaParser internal constructor( val wiredInputObjects = wiredObjects.dictionary.filterIsInstance().associateBy { it.name } val checker = UnmappedInputFieldChecker(wiredInputObjects, options.unmappedInputFields) objectDefinitions.forEach { type -> - fieldResolversByType[type].orEmpty().values.filterIsInstance().forEach { checker.check(type.name, it) } + resolverPlansByType[type].orEmpty().values.forEach { checker.check(type.name, it) } } checker.report() } @@ -168,7 +167,7 @@ class SchemaParser internal constructor( createField(field, fieldDefinition, inputObjects) codeRegistryBuilder.dataFetcher( FieldCoordinates.coordinates(objectDefinition.name, fieldDefinition.name), - fieldResolversByType[objectDefinition]?.get(fieldDefinition)?.createDataFetcher() + resolverPlansByType[objectDefinition]?.get(fieldDefinition)?.dataFetcher() ?: throw SchemaError("No resolver method found for object type '${objectDefinition.name}' and field '${fieldDefinition.name}', this is most likely a bug with graphql-java-tools") ) diff --git a/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt b/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt index dcee5d8b..c1f374e6 100644 --- a/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt +++ b/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt @@ -1,6 +1,6 @@ package graphql.kickstart.tools -import graphql.kickstart.tools.resolver.MethodFieldResolver +import graphql.kickstart.tools.resolver.ResolverPlan import graphql.kickstart.tools.util.unwrap import graphql.schema.GraphQLInputObjectField import graphql.schema.GraphQLInputObjectType @@ -32,14 +32,13 @@ internal class UnmappedInputFieldChecker( private val messages = linkedMapOf() - fun check(objectType: String, resolver: MethodFieldResolver) { - resolver.field.inputValueDefinitions.forEachIndexed { index, argument -> - val inputObject = inputObjects[argument.type.unwrap().name] ?: return@forEachIndexed - val usage = "argument '${argument.name}' of field '$objectType.${resolver.field.name}'" + fun check(objectType: String, plan: ResolverPlan) { + plan.argumentTargets.forEach { (argument, parameterType) -> + val inputObject = inputObjects[argument.type.unwrap().name] ?: return@forEach + val usage = "argument '${argument.name}' of field '$objectType.${plan.field.name}'" try { - val parameterType = resolver.getMethodParameterType(index) ?: return@forEachIndexed - val reads = StrictReads(resolver.objectMapper) + val reads = StrictReads(plan.objectMapper) val deserializer = reads.context.findRootValueDeserializer(reads.mapper.typeFactory.constructType(parameterType)) checkFields(reads, deserializer, null, inputObject, usage) } catch (e: VirtualMachineError) { diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolver.kt deleted file mode 100644 index 22a93e03..00000000 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolver.kt +++ /dev/null @@ -1,49 +0,0 @@ -package graphql.kickstart.tools.resolver - -import graphql.kickstart.tools.* -import graphql.language.FieldDefinition -import graphql.schema.DataFetcher -import graphql.schema.DataFetchingEnvironment -import org.apache.commons.lang3.reflect.TypeUtils - -/** - * @author Andrew Potter - */ -internal abstract class FieldResolver( - val field: FieldDefinition, - val search: FieldResolverScanner.Search, - val options: SchemaParserOptions -) { - val resolverInfo: ResolverInfo = search.resolverInfo - - abstract fun scanForMatches(): Sequence - - abstract fun createDataFetcher(): DataFetcher<*> - - /** - * Add source resolver depending on whether or not this is a resolver method - */ - protected fun createSourceResolver(): SourceResolver { - return if (this.search.source != null) { - SourceResolver { _, _ -> this.search.source } - } else { - SourceResolver { environment, sourceObject -> - // if source object is known, environment is null as an optimization (LightDataFetcher) - val source = sourceObject - ?: environment?.getSource() - ?: throw ResolverError("Expected DataFetchingEnvironment and source object to not be null!") - - if (!TypeUtils.isAssignable(source.javaClass, this.search.type)) { - throw ResolverError("Expected source object to be an instance of '${TypeUtils.getRawType(this.search.type, this.search.type).name}' but instead got '${source.javaClass.name}'") - } - - source - } - } - } -} - -fun interface SourceResolver { - - fun resolve(environment: DataFetchingEnvironment?, source: Any?): Any -} diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt deleted file mode 100644 index 561096b6..00000000 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt +++ /dev/null @@ -1,292 +0,0 @@ -package graphql.kickstart.tools.resolver - -import graphql.GraphQLContext -import graphql.Scalars -import graphql.kickstart.tools.ResolverInfo -import graphql.kickstart.tools.RootResolverInfo -import graphql.kickstart.tools.SchemaParserOptions -import graphql.kickstart.tools.util.* -import graphql.language.FieldDefinition -import graphql.language.InputValueDefinition -import graphql.schema.DataFetchingEnvironment -import graphql.schema.idl.ScalarInfo -import kotlinx.coroutines.channels.ReceiveChannel -import org.apache.commons.lang3.ClassUtils -import org.apache.commons.lang3.reflect.FieldUtils -import org.apache.commons.lang3.reflect.TypeUtils -import org.reactivestreams.Publisher -import org.slf4j.LoggerFactory -import java.lang.reflect.* -import kotlin.reflect.full.extensionReceiverParameter -import kotlin.reflect.jvm.kotlinFunction - -/** - * @author Andrew Potter - */ -internal class FieldResolverScanner(val options: SchemaParserOptions, private val scalarNames: Set) { - - private val log = LoggerFactory.getLogger(javaClass) - - private val allowedTrailingParameterTypes = listOfNotNull(DataFetchingEnvironment::class.java, GraphQLContext::class.java, options.contextClass) - - private val methodsByNameCache = mutableMapOf, Boolean>, Map>>() - - fun findFieldResolver(field: FieldDefinition, resolverInfo: ResolverInfo): FieldResolver { - val searches = resolverInfo.getFieldSearches() - - val scanProperties = field.inputValueDefinitions.isEmpty() - val found = searches.mapNotNull { search -> findFieldResolver(field, search, scanProperties) } - - if (resolverInfo is RootResolverInfo && found.size > 1) { - throw FieldResolverError("Found more than one matching resolver for field '$field': $found") - } - - return found.firstOrNull() - ?: missingFieldResolver(field) - ?: findSourcelessFieldResolver(field, searches) - ?: throw FieldResolverError(getMissingFieldMessage(field, searches, scanProperties)) - } - - private fun findFieldResolver(field: FieldDefinition, search: Search, scanProperties: Boolean): FieldResolver? { - val method = findResolverMethod(field, search) - if (method != null) { - return MethodFieldResolver(field, search, options, method.apply(trySetAccessible(field, search.type)), scalarNames) - } - - if (scanProperties) { - val property = findResolverProperty(field, search) - if (property != null) { - return PropertyFieldResolver(field, search, options, property.apply(trySetAccessible(field, search.type))) - } - } - - if (Map::class.java.isAssignableFrom(search.type.unwrap())) { - return MapFieldResolver(field, search, options, search.type.unwrap()) - } - - return null - } - - private fun trySetAccessible(field: FieldDefinition, type: JavaType): AccessibleObject.() -> Unit = { - try { - isAccessible = true - } catch (e: RuntimeException) { - log.warn("Unable to make field ${type.unwrap().name}#${field.name} accessible. " + - "Be sure to provide a resolver or open the enclosing module if possible.") - } - } - - private fun missingFieldResolver(field: FieldDefinition): FieldResolver? { - return if (options.allowUnimplementedResolvers || options.missingResolverDataFetcherProvider != null) { - if (options.allowUnimplementedResolvers) { - log.warn("Missing resolver for field: $field") - } - - MissingFieldResolver(field, options) - } else { - null - } - } - - private fun findSourcelessFieldResolver(field: FieldDefinition, searches: List): FieldResolver? = - searches.firstNotNullOfOrNull { search -> - val sourceType = search.requiredFirstParameterType ?: return@firstNotNullOfOrNull null - val sourceless = search.withoutSourceParameter() - findMethodByName(field, sourceless) { isSourcelessMethod(it, field, sourceless, sourceType) }?.let { method -> - MethodFieldResolver(field, sourceless, options, method.apply(trySetAccessible(field, search.type)), scalarNames) - } - } - - private fun isSourcelessMethod(method: Method, field: FieldDefinition, search: Search, sourceType: Class<*>): Boolean { - val arguments = field.inputValueDefinitions - val parameterTypes = method.parameterTypes - - return method.parameterCountWithoutContinuation() > arguments.size && - arguments.indices.none { isContextParameter(parameterTypes[it], arguments[it]) } && - (arguments.isEmpty() || !couldTakeSource(parameterTypes[0], sourceType)) && - verifyMethodArguments(method, arguments.size, search) - } - - private fun couldTakeSource(parameterType: Class<*>, sourceType: Class<*>) = - sourceType.isAssignableFrom(parameterType) || parameterType.isAssignableFrom(sourceType) - - // the context class can also be an argument's type (e.g. a Map for an input object), so only spec scalars count for it - fun isContextParameter(parameter: Class<*>, argument: InputValueDefinition): Boolean = - parameter in allowedTrailingParameterTypes && - (parameter != options.contextClass || argument.type.unwrap().name in ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS) - - private fun findResolverMethod(field: FieldDefinition, search: Search): Method? { - val argumentCount = field.inputValueDefinitions.size + search.firstArgumentIndex - return findMethodByName(field, search) { verifyMethodArguments(it, argumentCount, search) } - } - - private inline fun findMethodByName(field: FieldDefinition, search: Search, matches: (Method) -> Boolean): Method? { - val methodsByName = getMethodsByName(search) - 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 listOfNotNull( - name, - if (isBoolean(field.type)) "is$capitalizedName" else null, - "get$capitalizedName", - "getField$capitalizedName", - "get${name.snakeToCamelCase()}" - ).firstNotNullOfOrNull { methodName -> - methodsByName[methodName]?.find { matches(it) } - } - } - - // root fields are searched on every root resolver, so each class's methods are only indexed once - private fun getMethodsByName(search: Search): Map> = - methodsByNameCache.getOrPut(search.type.unwrap() to search.isSubscription) { - getAllMethods(search).groupBy { it.name } - } - - private fun getAllMethods(search: Search): List { - val type = search.type.unwrap() - val declaredMethods = type.declaredNonProxyMethods - val superClassesMethods = ClassUtils.getAllSuperclasses(type).flatMap { it.methods.toList() } - val interfacesMethods = ClassUtils.getAllInterfaces(type).flatMap { it.methods.toList() } - - return (declaredMethods + superClassesMethods + interfacesMethods) - .asSequence() - .filter { !it.isSynthetic } - .filter { !Modifier.isPrivate(it.modifiers) } - // discard any methods that are coming off the root of the class hierarchy - // to avoid issues with duplicate method declarations - .filter { it.declaringClass != Any::class.java } - // subscription resolvers must return a publisher - .filter { !search.isSubscription || resolverMethodReturnsPublisher(it) } - .toList() - } - - private fun resolverMethodReturnsPublisher(method: Method) = - // suspend functions and unbounded type variables are erased to Object, so the actual return type is unknown here - method.returnType == Any::class.java - || Publisher::class.java.isAssignableFrom(method.returnType) - || resolverMethodReturnsPublisherFuture(method) - || receiveChannelToPublisherWrapper(method) - - private fun resolverMethodReturnsPublisherFuture(method: Method) = - method.genericReturnType.futureValueType() - ?.let { if (it is WildcardType) it.upperBounds.first() else it } - ?.let { TypeUtils.isAssignable(it, Publisher::class.java) } == true - - private fun receiveChannelToPublisherWrapper(method: Method) = - ReceiveChannel::class.java.isAssignableFrom(method.returnType) - && options.genericWrappers.any { wrapper -> - val isReceiveChannelWrapper = wrapper.type == method.returnType - // lambdas are erased to return Object, so this only checks that the transformer can return a Publisher - val hasPublisherTransformer = wrapper - .transformer.javaClass - .declaredMethods - .filter { it.name == "invoke" } - .any { it.returnType.isAssignableFrom(Publisher::class.java) } - isReceiveChannelWrapper && hasPublisherTransformer - } - - private fun isBoolean(type: GraphQLLangType) = type.unwrap().name == Scalars.GraphQLBoolean.name - - private fun verifyMethodArguments(method: Method, requiredCount: Int, search: Search): Boolean { - val appropriateFirstParameter = if (search.requiredFirstParameterType != null) { - method.genericParameterTypes.firstOrNull()?.let { - it.eraseUnboundedWildcards() == search.requiredFirstParameterType || method.declaringClass.typeParameters.contains(it) - } ?: false - } else { - // an extension receiver can only take the source object - !isExtensionFunction(method) - } - - val methodParameterCount = method.parameterCountWithoutContinuation() - - val correctParameterCount = methodParameterCount >= requiredCount && - method.parameterTypes.slice(requiredCount until methodParameterCount).all { it in allowedTrailingParameterTypes } - return correctParameterCount && appropriateFirstParameter - } - - private fun isExtensionFunction(method: Method): Boolean { - return try { - method.kotlinFunction?.extensionReceiverParameter != null - } catch (e: InternalError) { - false - } - } - - private fun findResolverProperty(field: FieldDefinition, search: Search) = - FieldUtils.getAllFields(search.type.unwrap()).find { it.name == field.name } - - private fun getMissingFieldMessage(field: FieldDefinition, searches: List, scannedProperties: Boolean): String { - val signatures = mutableListOf("") - val isBoolean = isBoolean(field.type) - var isSubscription = false - - searches.forEach { search -> - signatures.addAll(getMissingMethodSignatures(field, search, isBoolean, scannedProperties)) - isSubscription = isSubscription || search.isSubscription - } - searches.filter { it.requiredFirstParameterType != null }.forEach { search -> - signatures.addAll(getMissingMethodSignatures(field, search.withoutSourceParameter(), isBoolean, false, "")) - } - - val sourceName = field.sourceLocation?.sourceName ?: "" - val sourceLocation = field.sourceLocation?.let { "$sourceName:${it.line}" } ?: "" - - return "No method${if (scannedProperties) " or field" else ""} found as defined in schema $sourceLocation with any of the following signatures " + - "(optionally followed by any of $allowedTrailingParameterTypes), in priority order:\n${signatures.joinToString("\n ")}" + - if (isSubscription) "\n\nNote that a Subscription data fetcher must return a Publisher of events" else "" - } - - private fun getMissingMethodSignatures( - field: FieldDefinition, - search: Search, - isBoolean: Boolean, - scannedProperties: Boolean, - trailingParameter: String? = null - ): List { - val baseType = search.type.unwrap() - val signatures = mutableListOf() - val args = mutableListOf() - val sep = ", " - - if (search.requiredFirstParameterType != null) { - args.add(search.requiredFirstParameterType.name) - } - - args.addAll(field.inputValueDefinitions.map { "~${it.name}" }) - trailingParameter?.let(args::add) - - val argString = args.joinToString(sep) - - signatures.add("${baseType.name}.${field.name}($argString)") - if (isBoolean) { - signatures.add("${baseType.name}.is${field.name.replaceFirstChar(Char::titlecase)}($argString)") - } - signatures.add("${baseType.name}.get${field.name.replaceFirstChar(Char::titlecase)}($argString)") - if (scannedProperties) { - signatures.add("${baseType.name}.${field.name}") - } - - return signatures - } - - data class Search( - val type: JavaType, - val resolverInfo: ResolverInfo, - val source: Any?, - val requiredFirstParameterType: Class<*>? = null - ) { - val isSubscription get() = resolverInfo is RootResolverInfo && resolverInfo.isSubscription - - val firstArgumentIndex get() = if (requiredFirstParameterType != null) 1 else 0 - - fun withoutSourceParameter() = copy(requiredFirstParameterType = null) - } -} - -internal class FieldResolverError(msg: String) : RuntimeException(msg) diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/MapFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/MapFieldResolver.kt deleted file mode 100644 index fa33e15e..00000000 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/MapFieldResolver.kt +++ /dev/null @@ -1,76 +0,0 @@ -package graphql.kickstart.tools.resolver - -import com.fasterxml.classmate.TypeResolver -import graphql.kickstart.tools.ScanDictionary -import graphql.kickstart.tools.SchemaParserOptions -import graphql.kickstart.tools.util.JavaType -import graphql.kickstart.tools.util.unwrap -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 - * - * The MapFieldResolver implements the Map (i.e. property map) specific portion of the logic within the GraphQL PropertyDataFetcher class. - */ -internal class MapFieldResolver( - field: FieldDefinition, - search: FieldResolverScanner.Search, - options: SchemaParserOptions, - mapClass: JavaType -) : FieldResolver(field, search, options) { - - private val mapGenericValue: JavaType = getMapGenericType(mapClass) - - /** - * Takes a type which implements Map and tries to find the - * value type of that map. For some reason, mapClass is losing - * its generics somewhere along the way and is always a raw - * type. - */ - private fun getMapGenericType(mapClass: JavaType): JavaType { - val resolvedType = TypeResolver().resolve(mapClass) - val typeParameters = resolvedType.typeParametersFor(Map::class.java) - - return typeParameters.elementAtOrElse(1) { Any::class.java } - } - - private val returnShape by lazy { - TypeShape.returnType(field.type, mapGenericValue, search.isSubscription, search.type, options, ScanDictionary.FieldTypeReference(field.name)) - } - - private val hasTypedValues get() = resolveTypeVariables(mapGenericValue, search.type) != Any::class.java - - override fun createDataFetcher(): DataFetcher<*> { - val unwrapper = if (hasTypedValues) TypeShape.Unwrapper(returnShape, ResolverKind.MAP_VALUE, "${search.type.unwrap().name}.${field.name}") else null - return MapFieldResolverDataFetcher(createSourceResolver(), field.name, unwrapper) - } - - override fun scanForMatches(): Sequence = sequenceOf(returnShape) - - override fun toString() = "MapFieldResolver{key=${field.name}}" -} - -internal class MapFieldResolverDataFetcher( - private val sourceResolver: SourceResolver, - private val key: String, - private val unwrapper: TypeShape.Unwrapper? -) : LightDataFetcher { - - override fun get(fieldDefinition: GraphQLFieldDefinition, sourceObject: Any?, environmentSupplier: Supplier): Any? { - if (sourceObject is Map<*, *>) { - val value = sourceObject[key] - return if (value == null || unwrapper == null) value else unwrapper.unwrap(value, environmentSupplier) - } 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 } - } -} diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt deleted file mode 100644 index 24ec4c19..00000000 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt +++ /dev/null @@ -1,253 +0,0 @@ -package graphql.kickstart.tools.resolver - -import graphql.GraphQLContext -import graphql.kickstart.tools.* -import graphql.kickstart.tools.util.JavaType -import graphql.kickstart.tools.util.coroutineScope -import graphql.kickstart.tools.util.isSuspendFunction -import graphql.kickstart.tools.util.parameterCountWithoutContinuation -import graphql.kickstart.tools.util.typeArgument -import graphql.kickstart.tools.util.unwrap -import graphql.language.* -import graphql.schema.DataFetcher -import graphql.schema.DataFetchingEnvironment -import graphql.schema.GraphQLFieldDefinition -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 tools.jackson.databind.ObjectMapper -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.jvm.javaType -import kotlin.reflect.jvm.kotlinFunction - -/** - * @author Andrew Potter - */ -internal class MethodFieldResolver( - field: FieldDefinition, - search: FieldResolverScanner.Search, - options: SchemaParserOptions, - val method: Method, - private val scalarNames: Set -) : FieldResolver(field, search, options) { - - private val isSuspendFunction = method.isSuspendFunction() - private val numberOfParameters = method.parameterCountWithoutContinuation() - - internal val objectMapper: ObjectMapper by lazy { options.objectMapperProvider.provide(field) } - - override fun createDataFetcher(): DataFetcher<*> { - val args = mutableListOf() - val mapper = objectMapper - - if (this.search.requiredFirstParameterType != null) { - val expectedType = this.search.requiredFirstParameterType - - args.add { environment -> - val source = environment.getSource() - ?: throw ResolverError("Expected source object to not be null!") - if (!expectedType.isAssignableFrom(source.javaClass)) { - throw ResolverError("Source type (${source.javaClass.name}) is not expected type (${expectedType.name})!") - } - - source - } - } - - // Add an argument for each argument defined in the GraphQL schema - this.field.inputValueDefinitions.forEachIndexed { index, definition -> - - val parameterType = this.getMethodParameterType(index) - ?: throw ResolverError("Missing method type at position ${this.getJavaMethodParameterIndex(index)}, this is most likely a bug with graphql-java-tools") - - val isNonNull = definition.type is NonNullType - val isOptional = TypeUtils.getRawType(parameterType, search.type) == Optional::class.java - val argumentConverter = ArgumentConverter(mapper, parameterType) - val parameterClass = parameterType.unwrap() - val isConcreteScalar = isConcreteScalarType(definition.type, parameterType) - - args.add { environment -> - val argumentPresent = environment.arguments.containsKey(definition.name) - if (!argumentPresent && isNonNull) { - throw ResolverError("Missing required argument with name '${definition.name}', this is most likely a bug with graphql-java-tools") - } - - val value = environment.arguments[definition.name] - - if (value == null && isOptional) { - if (options.inputArgumentOptionalDetectOmission && !environment.containsArgument(definition.name)) { - return@add null - } - return@add Optional.empty() - } - - if (value == null || !isConcreteScalar || !parameterClass.isInstance(value)) { - return@add argumentConverter.convert(value) - } - - return@add value - } - } - - // Add DataFetchingEnvironment/Context arguments, stopping before the Continuation parameter of a suspend function - val firstTrailingParameter = field.inputValueDefinitions.size + search.firstArgumentIndex - method.parameterTypes.slice(firstTrailingParameter until numberOfParameters).forEach { parameterType -> - when (parameterType) { - GraphQLContext::class.java -> args.add { environment -> environment.graphQlContext } - - options.contextClass -> args.add { environment -> - environment.graphQlContext.get(parameterType) ?: throw ResolverError( - "Resolver method '${method.declaringClass.name}.${method.name}' has a parameter of the context class " + - "'${parameterType.name}', but the GraphQLContext has no value for it. Put it in the GraphQLContext under " + - "the class key when building the ExecutionInput, for example " + - "graphQLContext(mapOf(${parameterType.simpleName}::class.java to context))." - ) - } - - else -> args.add { environment -> environment } - } - } - - val unwrapper = TypeShape.Unwrapper(returnShape, ResolverKind.METHOD, "${method.declaringClass.name}.${method.name}", isSuspendFunction) - - return if (numberOfParameters > 0 || isSuspendFunction) { - // requires arguments and environment or is a suspend function - MethodFieldResolverDataFetcher(createSourceResolver(), this.method, args, options, isSuspendFunction, unwrapper) - } else { - // if there are no parameters an optimized version of the data fetcher can be used - LightMethodFieldResolverDataFetcher(createSourceResolver(), this.method, unwrapper) - } - } - - /** - * A concrete scalar type is a scalar type where values always coerce to the same Java type. The ID scalar type is not concrete - * because values can be coerced to multiple different Java types (eg. String, Long, UUID). All values of a non-concrete scalar - * type must be converted to the target method parameter type. - */ - private fun isConcreteScalarType(type: Type<*>, genericParameterType: JavaType): Boolean { - return when (type) { - is ListType -> genericParameterType.typeArgument(List::class.java) - ?.let { isConcreteScalarType(type.type, it) } ?: false - - is TypeName -> type.name in scalarNames && type.name != "ID" - is NonNullType -> isConcreteScalarType(type.type, genericParameterType) - else -> false - } - } - - // each shape is walked only when the scanner gets to it, so an earlier match's dictionary error is reported first - override fun scanForMatches(): Sequence = sequence { - field.inputValueDefinitions.forEachIndexed { i, inputDefinition -> - yield(TypeShape.parameterType(inputDefinition.type, getMethodParameterType(i)!!, search.type, options, ScanDictionary.MethodParameterReference(method, i))) - } - yield(returnShape) - } - - private val returnShape by lazy { - val declaredType = try { - method.kotlinFunction?.returnType?.javaType ?: method.genericReturnType - } catch (e: InternalError) { - method.genericReturnType - } - TypeShape.returnType(field.type, resolveTypeVariables(declaredType, search.type), search.isSubscription, search.type, options, ScanDictionary.ReturnValueReference(method)) - } - - private fun getJavaMethodParameterIndex(index: Int) = index + search.firstArgumentIndex - - internal fun getMethodParameterType(index: Int): JavaType? { - val methodIndex = getJavaMethodParameterIndex(index) - val parameters = method.parameterTypes - - return if (parameters.size > methodIndex) { - resolveTypeVariables(method.genericParameterTypes[methodIndex], search.type) - } else { - null - } - } - - override fun toString() = "MethodFieldResolver{method=$method}" -} - -internal class MethodFieldResolverDataFetcher( - private val sourceResolver: SourceResolver, - private val method: Method, - private val args: List, - private val options: SchemaParserOptions, - private val isSuspendFunction: Boolean, - private val unwrapper: TypeShape.Unwrapper -) : DataFetcher { - - override fun get(environment: DataFetchingEnvironment): Any? { - val source = sourceResolver.resolve(environment, null) - val args = this.args.map { it(environment) }.toTypedArray() - - return if (isSuspendFunction) { - // 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)?.let { unwrapper.unwrapSuspending(it) { environment } } - } - } else { - invoke(method, source, args)?.let { unwrapper.unwrap(it) { environment } } - } - } - - /** - * 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 generic wrappers are used. - */ -internal class LightMethodFieldResolverDataFetcher( - private val sourceResolver: SourceResolver, - private val method: Method, - private val unwrapper: TypeShape.Unwrapper -) : LightDataFetcher { - - override fun get(fieldDefinition: GraphQLFieldDefinition?, sourceObject: Any?, environmentSupplier: Supplier): Any? { - val source = sourceResolver.resolve(null, sourceObject) - - return invoke(method, source, emptyArray())?.let { unwrapper.unwrap(it, environmentSupplier) } - } - - 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. - */ - @Suppress("unused") - fun getWrappedFetchingObject(environment: DataFetchingEnvironment): Any { - return sourceResolver.resolve(environment, null) - } -} - -private suspend inline fun invokeSuspend(target: Any, resolverMethod: Method, args: Array): Any? { - return suspendCoroutineUninterceptedOrReturn { continuation -> - invoke(resolverMethod, target, args + continuation) - } -} - -private fun invoke(method: Method, instance: Any, args: Array): Any? { - try { - return method.invoke(instance, *args) - } catch (e: InvocationTargetException) { - throw e.cause ?: RuntimeException("Unknown error occurred while invoking resolver method") - } -} - -internal typealias ArgumentPlaceholder = (DataFetchingEnvironment) -> Any? diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/MissingFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/MissingFieldResolver.kt deleted file mode 100644 index 98f6f00a..00000000 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/MissingFieldResolver.kt +++ /dev/null @@ -1,17 +0,0 @@ -package graphql.kickstart.tools.resolver - -import graphql.kickstart.tools.MissingResolverInfo -import graphql.kickstart.tools.SchemaParserOptions -import graphql.language.FieldDefinition -import graphql.schema.DataFetcher - -internal class MissingFieldResolver( - field: FieldDefinition, - options: SchemaParserOptions -) : FieldResolver(field, FieldResolverScanner.Search(Any::class.java, MissingResolverInfo(), null), options) { - - private val missingResolverDataFetcherProvider: MissingResolverDataFetcherProvider = options.missingResolverDataFetcherProvider - ?: MissingResolverDataFetcherProvider { _, _ -> DataFetcher { TODO("Schema resolver not implemented") } } - override fun scanForMatches(): Sequence = emptySequence() - override fun createDataFetcher(): DataFetcher<*> = missingResolverDataFetcherProvider.createDataFetcher(field, options) -} diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/PropertyFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/PropertyFieldResolver.kt deleted file mode 100644 index 112ead6c..00000000 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/PropertyFieldResolver.kt +++ /dev/null @@ -1,50 +0,0 @@ -package graphql.kickstart.tools.resolver - -import graphql.kickstart.tools.ScanDictionary -import graphql.kickstart.tools.SchemaParserOptions -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 - */ -internal class PropertyFieldResolver( - field: FieldDefinition, - search: FieldResolverScanner.Search, - options: SchemaParserOptions, - val property: Field -) : FieldResolver(field, search, options) { - - private val returnShape by lazy { - TypeShape.returnType(field.type, property.genericType, search.isSubscription, search.type, options, ScanDictionary.FieldTypeReference(property.toString())) - } - - override fun createDataFetcher(): DataFetcher<*> { - val unwrapper = TypeShape.Unwrapper(returnShape, ResolverKind.FIELD, "${property.declaringClass.name}.${property.name}") - return PropertyFieldResolverDataFetcher(createSourceResolver(), property, unwrapper) - } - - override fun scanForMatches(): Sequence = sequenceOf(returnShape) - - override fun toString() = "PropertyFieldResolver{property=$property}" -} - -internal class PropertyFieldResolverDataFetcher( - private val sourceResolver: SourceResolver, - private val field: Field, - private val unwrapper: TypeShape.Unwrapper -) : LightDataFetcher { - - override fun get(fieldDefinition: GraphQLFieldDefinition, sourceObject: Any?, environmentSupplier: Supplier): Any? { - return field.get(sourceResolver.resolve(null, sourceObject))?.let { unwrapper.unwrap(it, environmentSupplier) } - } - - override fun get(environment: DataFetchingEnvironment): Any? { - return get(environment.fieldDefinition, sourceResolver.resolve(environment, null)) { environment } - } -} diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt new file mode 100644 index 00000000..df227b3f --- /dev/null +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt @@ -0,0 +1,670 @@ +package graphql.kickstart.tools.resolver + +import com.fasterxml.classmate.TypeResolver +import graphql.GraphQLContext +import graphql.Scalars +import graphql.kickstart.tools.MissingResolverInfo +import graphql.kickstart.tools.ResolverError +import graphql.kickstart.tools.ResolverInfo +import graphql.kickstart.tools.ResolverSearch +import graphql.kickstart.tools.RootResolverInfo +import graphql.kickstart.tools.ScanDictionary +import graphql.kickstart.tools.SchemaParserOptions +import graphql.kickstart.tools.util.GraphQLLangType +import graphql.kickstart.tools.util.JavaType +import graphql.kickstart.tools.util.coroutineScope +import graphql.kickstart.tools.util.eraseUnboundedWildcards +import graphql.kickstart.tools.util.snakeToCamelCase +import graphql.kickstart.tools.util.unwrap +import graphql.language.FieldDefinition +import graphql.language.InputValueDefinition +import graphql.language.NonNullType +import graphql.schema.DataFetcher +import graphql.schema.DataFetchingEnvironment +import graphql.schema.GraphQLFieldDefinition +import graphql.schema.LightDataFetcher +import graphql.schema.idl.ScalarInfo +import graphql.schema.idl.TypeUtil +import kotlinx.coroutines.CoroutineStart +import kotlinx.coroutines.ensureActive +import kotlinx.coroutines.future.future +import org.apache.commons.lang3.ClassUtils +import org.apache.commons.lang3.reflect.FieldUtils +import org.apache.commons.lang3.reflect.TypeUtils +import org.slf4j.LoggerFactory +import tools.jackson.databind.ObjectMapper +import java.lang.reflect.AccessibleObject +import java.lang.reflect.Field +import java.lang.reflect.InvocationTargetException +import java.lang.reflect.Method +import java.lang.reflect.Modifier +import java.lang.reflect.Proxy +import java.util.* +import java.util.function.Supplier +import kotlin.coroutines.intrinsics.suspendCoroutineUninterceptedOrReturn +import kotlin.reflect.full.extensionReceiverParameter +import kotlin.reflect.jvm.javaType +import kotlin.reflect.jvm.kotlinFunction + +/** + * How one object field is resolved: the Java member behind it, the type shapes the scan matches, and its data fetcher. Every + * fact about the member's signature is read here once, so the scan, the data fetcher and the checks after the build can't + * disagree about it. + */ +internal class ResolverPlan private constructor( + val field: FieldDefinition, + val search: ResolverSearch, + private val options: SchemaParserOptions, + // null when the field has no resolver and the options allow that + private val member: Member? +) { + val resolverInfo: ResolverInfo get() = search.resolverInfo + + // provided at parse time, once per method field, so the user's provider doesn't run for a schema that fails to scan + val objectMapper: ObjectMapper by lazy { options.objectMapperProvider.provide(field) } + + val argumentTargets: List get() = (member as? MethodMember)?.arguments.orEmpty().map { ArgumentTarget(it.definition, it.type) } + + private val returnShape by lazy { + member?.let { TypeShape.returnType(field.type, it.declaredValueType, search.isSubscription, search.type, options, it.reference) } + } + + // each shape is walked only when the scanner gets to it, so an earlier match's dictionary error is reported first + fun typeShapes(): Sequence = sequence { + (member as? MethodMember)?.arguments?.forEach { yield(it.shape) } + returnShape?.let { yield(it) } + } + + fun dataFetcher(): DataFetcher<*> { + val member = member + ?: return (options.missingResolverDataFetcherProvider ?: notImplemented).createDataFetcher(field, options) + val source = sourceResolver() + return when (member) { + is MethodMember -> { + val slots = member.slots(objectMapper) + val unwrapper = TypeShape.Unwrapper(returnShape!!, ResolverKind.METHOD, member.name, member.isSuspend) + if (slots.isNotEmpty() || member.isSuspend) Fetcher(source, member, slots, options, unwrapper) else LightFetcher(source, source, member, unwrapper) + } + + is PropertyMember -> LightFetcher(source, source, member, TypeShape.Unwrapper(returnShape!!, ResolverKind.FIELD, member.name)) + + is MapMember -> { + val unwrapper = if (member.hasTypedValues) TypeShape.Unwrapper(returnShape!!, ResolverKind.MAP_VALUE, member.name) else null + LightFetcher(source, MapMember.sourceResolver, member, unwrapper) + } + } + } + + private fun sourceResolver(): SourceResolver { + val resolver = search.source + if (resolver != null) { + return SourceResolver { _, _ -> resolver } + } + + return SourceResolver { environment, sourceObject -> + // if source object is known, environment is null as an optimization (LightDataFetcher) + val source = sourceObject + ?: environment?.getSource() + ?: throw ResolverError("Expected DataFetchingEnvironment and source object to not be null!") + + if (!TypeUtils.isAssignable(source.javaClass, search.type)) { + throw ResolverError("Expected source object to be an instance of '${TypeUtils.getRawType(search.type, search.type).name}' but instead got '${source.javaClass.name}'") + } + + source + } + } + + fun notCallableOnResolver(objectType: String): NotCallableOnResolver? { + val source = search.source ?: return null + val javaMember = member?.javaMember ?: return null + if (Modifier.isStatic(javaMember.modifiers) || javaMember.declaringClass.isInstance(source)) { + return null + } + + return NotCallableOnResolver( + "Field '$objectType.${field.name}' resolves to ${javaMember.declaringClass.name}.${javaMember.name}, but the resolver is a " + + "${source.javaClass.name}, which isn't a ${javaMember.declaringClass.name}, so every query of the field will fail.", + javaMember is Field + ) + } + + fun requireParameterForEachArgument(objectType: String) { + val member = member as? MethodMember ?: return + val method = member.method + member.arguments.forEach { argument -> + val parameter = TypeUtils.getRawType(argument.type, search.type) ?: return@forEach + if (!ExtraParameter.takesArgumentSlot(parameter, argument.definition, options)) { + return@forEach + } + + throw FieldResolverError( + "Field '$objectType.${field.name}' has no parameter for its argument '${argument.definition.name}: " + + "${TypeUtil.simplePrint(argument.definition.type)}' in ${method.declaringClass.name}.${method.name}, so the ${parameter.simpleName} " + + "parameter gets the argument instead. Add a parameter for each argument, in order, before the ${parameter.simpleName} parameter." + ) + } + } + + override fun toString() = member.toString() + + data class ArgumentTarget(val definition: InputValueDefinition, val type: JavaType) + + data class NotCallableOnResolver(val message: String, val isField: Boolean) + + /** + * Finds the member behind each field. Root fields are searched on every root resolver, so it indexes each class's methods + * only once. + */ + class Finder(private val options: SchemaParserOptions, private val scalarNames: Set) { + + private val log = LoggerFactory.getLogger(javaClass) + + private val methodsByNameCache = mutableMapOf, Boolean>, Map>>() + + fun find(field: FieldDefinition, resolverInfo: ResolverInfo): ResolverPlan { + val searches = resolverInfo.getFieldSearches() + + val scanProperties = field.inputValueDefinitions.isEmpty() + val found = searches.mapNotNull { search -> findMember(field, search, scanProperties)?.let { plan(field, search, it) } } + + if (resolverInfo is RootResolverInfo && found.size > 1) { + throw FieldResolverError("Found more than one matching resolver for field '$field': $found") + } + + return found.firstOrNull() + ?: missingResolver(field) + ?: findSourcelessResolver(field, searches) + ?: throw FieldResolverError(getMissingFieldMessage(field, searches, scanProperties)) + } + + private fun plan(field: FieldDefinition, search: ResolverSearch, member: Member?) = ResolverPlan(field, search, options, member) + + private fun findMember(field: FieldDefinition, search: ResolverSearch, scanProperties: Boolean): Member? = + ResolverKind.entries.firstNotNullOfOrNull { kind -> + when (kind) { + ResolverKind.METHOD -> findResolverMethod(field, search) + ?.let { MethodMember(field, search, it.accessible(field, search.type), options, scalarNames) } + + ResolverKind.FIELD -> if (scanProperties) { + findResolverProperty(field, search)?.let { PropertyMember(it.accessible(field, search.type)) } + } else { + null + } + + ResolverKind.MAP_VALUE -> if (Map::class.java.isAssignableFrom(search.type.unwrap())) MapMember(field, search) else null + } + } + + private fun T.accessible(field: FieldDefinition, type: JavaType): T = apply { + try { + isAccessible = true + } catch (e: RuntimeException) { + log.warn("Unable to make field ${type.unwrap().name}#${field.name} accessible. " + + "Be sure to provide a resolver or open the enclosing module if possible.") + } + } + + private fun missingResolver(field: FieldDefinition): ResolverPlan? { + return if (options.allowUnimplementedResolvers || options.missingResolverDataFetcherProvider != null) { + if (options.allowUnimplementedResolvers) { + log.warn("Missing resolver for field: $field") + } + + plan(field, ResolverSearch(Any::class.java, MissingResolverInfo(), null), null) + } else { + null + } + } + + private fun findSourcelessResolver(field: FieldDefinition, searches: List): ResolverPlan? = + searches.firstNotNullOfOrNull { search -> + val sourceType = search.requiredFirstParameterType ?: return@firstNotNullOfOrNull null + val sourceless = search.withoutSourceParameter() + findMethodByName(field, sourceless) { isSourcelessMethod(it, field, sourceless, sourceType) }?.let { method -> + plan(field, sourceless, MethodMember(field, sourceless, method.accessible(field, search.type), options, scalarNames)) + } + } + + private fun isSourcelessMethod(method: Method, field: FieldDefinition, search: ResolverSearch, sourceType: Class<*>): Boolean { + val arguments = field.inputValueDefinitions + val parameterTypes = method.parameterTypes + + return method.parameterCountWithoutContinuation() > arguments.size && + arguments.indices.none { ExtraParameter.takesArgumentSlot(parameterTypes[it], arguments[it], options) } && + (arguments.isEmpty() || !couldTakeSource(parameterTypes[0], sourceType)) && + verifyMethodArguments(method, arguments.size, search) + } + + private fun couldTakeSource(parameterType: Class<*>, sourceType: Class<*>) = + sourceType.isAssignableFrom(parameterType) || parameterType.isAssignableFrom(sourceType) + + private fun findResolverMethod(field: FieldDefinition, search: ResolverSearch): Method? { + val argumentCount = field.inputValueDefinitions.size + search.firstArgumentIndex + return findMethodByName(field, search) { verifyMethodArguments(it, argumentCount, search) } + } + + private inline fun findMethodByName(field: FieldDefinition, search: ResolverSearch, matches: (Method) -> Boolean): Method? { + val methodsByName = getMethodsByName(search) + 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 listOfNotNull( + name, + if (isBoolean(field.type)) "is$capitalizedName" else null, + "get$capitalizedName", + "getField$capitalizedName", + "get${name.snakeToCamelCase()}" + ).firstNotNullOfOrNull { methodName -> + methodsByName[methodName]?.find { matches(it) } + } + } + + private fun getMethodsByName(search: ResolverSearch): Map> = + methodsByNameCache.getOrPut(search.type.unwrap() to search.isSubscription) { + getAllMethods(search).groupBy { it.name } + } + + private fun getAllMethods(search: ResolverSearch): List { + val type = search.type.unwrap() + val declaredMethods = if (Proxy.isProxyClass(type)) emptyList() else type.declaredMethods.toList() + val superClassesMethods = ClassUtils.getAllSuperclasses(type).flatMap { it.methods.toList() } + val interfacesMethods = ClassUtils.getAllInterfaces(type).flatMap { it.methods.toList() } + + return (declaredMethods + superClassesMethods + interfacesMethods) + .asSequence() + .filter { !it.isSynthetic } + .filter { !Modifier.isPrivate(it.modifiers) } + // discard any methods that are coming off the root of the class hierarchy + // to avoid issues with duplicate method declarations + .filter { it.declaringClass != Any::class.java } + .filter { !search.isSubscription || returnsPublisher(it.returnType, it.genericReturnType, options.genericWrappers) } + .toList() + } + + private fun isBoolean(type: GraphQLLangType) = type.unwrap().name == Scalars.GraphQLBoolean.name + + private fun verifyMethodArguments(method: Method, requiredCount: Int, search: ResolverSearch): Boolean { + val appropriateFirstParameter = if (search.requiredFirstParameterType != null) { + method.genericParameterTypes.firstOrNull()?.let { + it.eraseUnboundedWildcards() == search.requiredFirstParameterType || method.declaringClass.typeParameters.contains(it) + } ?: false + } else { + // an extension receiver can only take the source object + !isExtensionFunction(method) + } + + val methodParameterCount = method.parameterCountWithoutContinuation() + + val correctParameterCount = methodParameterCount >= requiredCount && + method.parameterTypes.slice(requiredCount until methodParameterCount).all { ExtraParameter.of(it, options) != null } + return correctParameterCount && appropriateFirstParameter + } + + private fun isExtensionFunction(method: Method): Boolean { + return try { + method.kotlinFunction?.extensionReceiverParameter != null + } catch (e: InternalError) { + false + } + } + + private fun findResolverProperty(field: FieldDefinition, search: ResolverSearch) = + FieldUtils.getAllFields(search.type.unwrap()).find { it.name == field.name } + + private fun getMissingFieldMessage(field: FieldDefinition, searches: List, scannedProperties: Boolean): String { + val signatures = mutableListOf("") + val isBoolean = isBoolean(field.type) + var isSubscription = false + + searches.forEach { search -> + signatures.addAll(getMissingMethodSignatures(field, search, isBoolean, scannedProperties)) + isSubscription = isSubscription || search.isSubscription + } + searches.filter { it.requiredFirstParameterType != null }.forEach { search -> + signatures.addAll(getMissingMethodSignatures(field, search.withoutSourceParameter(), isBoolean, false, "")) + } + + val sourceName = field.sourceLocation?.sourceName ?: "" + val sourceLocation = field.sourceLocation?.let { "$sourceName:${it.line}" } ?: "" + + return "No method${if (scannedProperties) " or field" else ""} found as defined in schema $sourceLocation with any of the following signatures " + + "(optionally followed by any of ${ExtraParameter.types(options)}), in priority order:\n${signatures.joinToString("\n ")}" + + if (isSubscription) "\n\nNote that a Subscription data fetcher must return a Publisher of events" else "" + } + + private fun getMissingMethodSignatures( + field: FieldDefinition, + search: ResolverSearch, + isBoolean: Boolean, + scannedProperties: Boolean, + trailingParameter: String? = null + ): List { + val baseType = search.type.unwrap() + val signatures = mutableListOf() + val args = mutableListOf() + val sep = ", " + + if (search.requiredFirstParameterType != null) { + args.add(search.requiredFirstParameterType.name) + } + + args.addAll(field.inputValueDefinitions.map { "~${it.name}" }) + trailingParameter?.let(args::add) + + val argString = args.joinToString(sep) + + signatures.add("${baseType.name}.${field.name}($argString)") + if (isBoolean) { + signatures.add("${baseType.name}.is${field.name.replaceFirstChar(Char::titlecase)}($argString)") + } + signatures.add("${baseType.name}.get${field.name.replaceFirstChar(Char::titlecase)}($argString)") + if (scannedProperties) { + signatures.add("${baseType.name}.${field.name}") + } + + return signatures + } + } + + private companion object { + val notImplemented = MissingResolverDataFetcherProvider { _, _ -> DataFetcher { TODO("Schema resolver not implemented") } } + } +} + +/** + * The kinds of Java member a field can resolve to. The [ResolverPlan.Finder] looks for them in this order. + */ +internal enum class ResolverKind(val noun: String, val holdCompletionStage: String) { + METHOD("method", "Return a CompletionStage, such as a CompletableFuture, from the method or its generic wrapper transformer instead."), + FIELD("field", "Assign a CompletionStage, such as a CompletableFuture, to the field, or return one from its generic wrapper transformer instead."), + MAP_VALUE("map value", "Put a CompletionStage, such as a CompletableFuture, in the map, or return one from its generic wrapper transformer instead.") +} + +/** + * The parameters a resolver method may have after its arguments, listed in the order the missing-resolver error names them. + */ +internal enum class ExtraParameter(private val bindingPrecedence: Int) { + ENVIRONMENT(bindingPrecedence = 2) { + override fun type(options: SchemaParserOptions): Class<*> = DataFetchingEnvironment::class.java + override fun takesArgumentSlotOf(argument: InputValueDefinition) = true + override fun bind(parameterType: Class<*>, method: Method): (DataFetchingEnvironment) -> Any? = { environment -> environment } + }, + GRAPHQL_CONTEXT(bindingPrecedence = 0) { + override fun type(options: SchemaParserOptions): Class<*> = GraphQLContext::class.java + override fun takesArgumentSlotOf(argument: InputValueDefinition) = true + override fun bind(parameterType: Class<*>, method: Method): (DataFetchingEnvironment) -> Any? = { environment -> environment.graphQlContext } + }, + CONTEXT_CLASS(bindingPrecedence = 1) { + override fun type(options: SchemaParserOptions): Class<*>? = options.contextClass + + // the context class can also be an argument's type (e.g. a Map for an input object), so only spec scalars count for it + override fun takesArgumentSlotOf(argument: InputValueDefinition) = + argument.type.unwrap().name in ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS + + override fun bind(parameterType: Class<*>, method: Method): (DataFetchingEnvironment) -> Any? = { environment -> + environment.graphQlContext.get(parameterType) ?: throw ResolverError( + "Resolver method '${method.declaringClass.name}.${method.name}' has a parameter of the context class " + + "'${parameterType.name}', but the GraphQLContext has no value for it. Put it in the GraphQLContext under " + + "the class key when building the ExecutionInput, for example " + + "graphQLContext(mapOf(${parameterType.simpleName}::class.java to context))." + ) + } + }; + + abstract fun type(options: SchemaParserOptions): Class<*>? + + abstract fun takesArgumentSlotOf(argument: InputValueDefinition): Boolean + + abstract fun bind(parameterType: Class<*>, method: Method): (DataFetchingEnvironment) -> Any? + + companion object { + private val byBindingPrecedence = entries.sortedBy { it.bindingPrecedence } + + fun types(options: SchemaParserOptions): List> = entries.mapNotNull { it.type(options) } + + // a class can be the type of more than one entry, e.g. a context class of DataFetchingEnvironment + fun of(parameterType: Class<*>, options: SchemaParserOptions): ExtraParameter? = + byBindingPrecedence.find { it.type(options) == parameterType } + + // whether a parameter of this type in an argument's position is an extra parameter rather than the argument's + fun takesArgumentSlot(parameterType: Class<*>, argument: InputValueDefinition, options: SchemaParserOptions): Boolean { + val matching = entries.filter { it.type(options) == parameterType } + return matching.isNotEmpty() && matching.all { it.takesArgumentSlotOf(argument) } + } + } +} + +internal class FieldResolverError(msg: String) : RuntimeException(msg) + +private sealed class Member { + abstract val name: String + abstract val declaredValueType: JavaType + abstract val reference: ScanDictionary.Reference + abstract val javaMember: java.lang.reflect.Member? + + abstract fun read(target: Any, arguments: Array): Any? +} + +private class MethodMember( + field: FieldDefinition, + private val search: ResolverSearch, + val method: Method, + private val options: SchemaParserOptions, + private val scalarNames: Set +) : Member() { + val isSuspend = method.isSuspendFunction() + private val parameterCount = if (isSuspend) method.parameterCount - 1 else method.parameterCount + + override val name get() = "${method.declaringClass.name}.${method.name}" + override val javaMember get() = method + override val reference get() = ScanDictionary.ReturnValueReference(method) + + override val declaredValueType: JavaType + get() { + val declaredType = try { + method.kotlinFunction?.returnType?.javaType ?: method.genericReturnType + } catch (e: InternalError) { + method.genericReturnType + } + return resolveTypeVariables(declaredType, search.type) + } + + val arguments: List = field.inputValueDefinitions.mapIndexed { index, definition -> + val type = resolveTypeVariables(method.genericParameterTypes[index + search.firstArgumentIndex], search.type) + Argument(definition, type, lazy { TypeShape.parameterType(definition.type, type, search.type, options, ScanDictionary.MethodParameterReference(method, index)) }) + } + + class Argument(val definition: InputValueDefinition, val type: JavaType, private val lazyShape: Lazy) { + val shape by lazyShape + } + + /** + * What to pass for each parameter in the method's order, leaving out the Continuation of a suspend function. + */ + fun slots(mapper: ObjectMapper): List<(DataFetchingEnvironment) -> Any?> { + val slots = mutableListOf<(DataFetchingEnvironment) -> Any?>() + + search.requiredFirstParameterType?.let { expectedType -> + slots.add { environment -> + val source = environment.getSource() + ?: throw ResolverError("Expected source object to not be null!") + if (!expectedType.isAssignableFrom(source.javaClass)) { + throw ResolverError("Source type (${source.javaClass.name}) is not expected type (${expectedType.name})!") + } + + source + } + } + + arguments.forEach { slots.add(argumentSlot(it, mapper)) } + + method.parameterTypes.slice(slots.size until parameterCount).forEach { parameterType -> + slots.add(ExtraParameter.of(parameterType, options)!!.bind(parameterType, method)) + } + + return slots + } + + private fun argumentSlot(argument: Argument, mapper: ObjectMapper): (DataFetchingEnvironment) -> Any? { + val definition = argument.definition + val isNonNull = definition.type is NonNullType + val isOptional = isOptionalParameter(argument.type, search.type) + val argumentConverter = ArgumentConverter(mapper, argument.type) + val parameterClass = argument.type.unwrap() + val isConcreteScalar = isConcreteScalarType(definition.type, argument.type, scalarNames) + + return slot@{ environment -> + val argumentPresent = environment.arguments.containsKey(definition.name) + if (!argumentPresent && isNonNull) { + throw ResolverError("Missing required argument with name '${definition.name}', this is most likely a bug with graphql-java-tools") + } + + val value = environment.arguments[definition.name] + + if (value == null && isOptional) { + if (options.inputArgumentOptionalDetectOmission && !environment.containsArgument(definition.name)) { + return@slot null + } + return@slot Optional.empty() + } + + if (value == null || !isConcreteScalar || !parameterClass.isInstance(value)) { + return@slot argumentConverter.convert(value) + } + + return@slot value + } + } + + override fun read(target: Any, arguments: Array): Any? = invoke(method, target, arguments) + + suspend fun readSuspending(target: Any, arguments: Array): Any? = + suspendCoroutineUninterceptedOrReturn { continuation -> + arguments[arguments.lastIndex] = continuation + invoke(method, target, arguments) + } + + override fun toString() = "MethodFieldResolver{method=$method}" +} + +private class PropertyMember(private val property: Field) : Member() { + override val name get() = "${property.declaringClass.name}.${property.name}" + override val javaMember get() = property + override val declaredValueType: JavaType get() = property.genericType + override val reference get() = ScanDictionary.FieldTypeReference(property.toString()) + + override fun read(target: Any, arguments: Array): Any? = property.get(target) + + override fun toString() = "PropertyFieldResolver{property=$property}" +} + +private class MapMember(field: FieldDefinition, private val search: ResolverSearch) : Member() { + private val key = field.name + + // Classmate keeps the value type a Map subclass fixes, e.g. a class extending HashMap + override val declaredValueType: JavaType = + TypeResolver().resolve(search.type.unwrap()).typeParametersFor(Map::class.java).elementAtOrElse(1) { Any::class.java } + + val hasTypedValues get() = resolveTypeVariables(declaredValueType, search.type) != Any::class.java + + override val name get() = "${search.type.unwrap().name}.$key" + override val javaMember: java.lang.reflect.Member? get() = null + override val reference get() = ScanDictionary.FieldTypeReference(key) + + override fun read(target: Any, arguments: Array): Any? = (target as Map<*, *>)[key] + + override fun toString() = "MapFieldResolver{key=$key}" + + companion object { + // the light data fetcher reads any Map it's given, without checking its class against the search type + val sourceResolver = SourceResolver { _, source -> + source as? Map<*, *> ?: throw RuntimeException("MapFieldResolver attempt to fetch a field from an object instance that was not a map") + } + } +} + +private val noArguments = emptyArray() + +private class Fetcher( + private val source: SourceResolver, + private val member: MethodMember, + private val slots: List<(DataFetchingEnvironment) -> Any?>, + private val options: SchemaParserOptions, + private val unwrapper: TypeShape.Unwrapper +) : DataFetcher { + + override fun get(environment: DataFetchingEnvironment): Any? { + val target = source.resolve(environment, null) + val arguments = arrayOfNulls(if (member.isSuspend) slots.size + 1 else slots.size) + slots.forEachIndexed { index, slot -> arguments[index] = slot(environment) } + + return if (member.isSuspend) { + // 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() + member.readSuspending(target, arguments)?.let { unwrapper.unwrapSuspending(it) { environment } } + } + } else { + member.read(target, arguments)?.let { unwrapper.unwrap(it) { environment } } + } + } + + /** + * 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 = source.resolve(environment, null) +} + +/** + * For a member without parameters, which needs the environment only if a generic wrapper's transformer asks for it. + */ +private class LightFetcher( + private val source: SourceResolver, + private val lightSource: SourceResolver, + private val member: Member, + private val unwrapper: TypeShape.Unwrapper? +) : LightDataFetcher { + + override fun get(fieldDefinition: GraphQLFieldDefinition?, sourceObject: Any?, environmentSupplier: Supplier): Any? { + val value = member.read(lightSource.resolve(null, sourceObject), noArguments) + return if (value == null || unwrapper == null) value else unwrapper.unwrap(value, environmentSupplier) + } + + override fun get(environment: DataFetchingEnvironment): Any? = + get(environment.fieldDefinition, source.resolve(environment, null)) { environment } + + /** + * 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 = source.resolve(environment, null) +} + +private fun invoke(method: Method, instance: Any, arguments: Array): Any? { + try { + return method.invoke(instance, *arguments) + } catch (e: InvocationTargetException) { + throw e.cause ?: RuntimeException("Unknown error occurred while invoking resolver method") + } +} + +private fun Method.isSuspendFunction(): Boolean { + return try { + this.kotlinFunction?.isSuspend == true + } catch (e: InternalError) { + false + } +} + +// the trailing Continuation of a suspend function isn't a resolver argument +private fun Method.parameterCountWithoutContinuation(): Int = + if (isSuspendFunction()) parameterCount - 1 else parameterCount diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/SourceResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/SourceResolver.kt new file mode 100644 index 00000000..90dd4676 --- /dev/null +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/SourceResolver.kt @@ -0,0 +1,8 @@ +package graphql.kickstart.tools.resolver + +import graphql.schema.DataFetchingEnvironment + +fun interface SourceResolver { + + fun resolve(environment: DataFetchingEnvironment?, source: Any?): Any +} diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt index 609d7ed4..acbb83da 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt @@ -16,8 +16,10 @@ import graphql.kickstart.tools.util.isSubtypeOf import graphql.kickstart.tools.util.typeArgument import graphql.language.ListType import graphql.language.NonNullType +import graphql.language.TypeName import graphql.schema.DataFetchingEnvironment import graphql.schema.idl.TypeUtil +import kotlinx.coroutines.channels.ReceiveChannel import kotlinx.coroutines.future.asDeferred import org.apache.commons.lang3.reflect.TypeUtils import org.reactivestreams.Publisher @@ -259,6 +261,43 @@ private fun JavaType.asPublisher(): JavaType = ?.let { TypeUtils.parameterize(Publisher::class.java, it) } ?: this +// picks the subscription method before its return type is walked, so it reads the erased type, where suspend functions and +// unbounded type variables are Object and the actual return type is unknown +internal fun returnsPublisher(erasedType: Class<*>, genericType: JavaType, wrappers: List): Boolean = + erasedType == Any::class.java + || Publisher::class.java.isAssignableFrom(erasedType) + || returnsPublisherFuture(genericType) + || ReceiveChannel::class.java.isAssignableFrom(erasedType) && wrappers.any { it.type == erasedType && it.canTransformToPublisher() } + +private fun returnsPublisherFuture(genericType: JavaType) = + genericType.futureValueType() + ?.let { if (it is WildcardType) it.upperBounds.first() else it } + ?.let { TypeUtils.isAssignable(it, Publisher::class.java) } == true + +// lambdas are erased to return Object, so this only checks that the transformer can return a Publisher +private fun GenericWrapper.canTransformToPublisher() = + transformer.javaClass.declaredMethods + .filter { it.name == "invoke" } + .any { it.returnType.isAssignableFrom(Publisher::class.java) } + +internal fun isOptionalParameter(parameterType: JavaType, containingType: JavaType): Boolean = + TypeUtils.getRawType(parameterType, containingType) == Optional::class.java + +/** + * A concrete scalar type is a scalar type where values always coerce to the same Java type. The ID scalar type is not concrete + * because values can be coerced to multiple different Java types (eg. String, Long, UUID). All values of a non-concrete scalar + * type must be converted to the target method parameter type. + */ +internal fun isConcreteScalarType(type: GraphQLLangType, parameterType: JavaType, scalarNames: Set): Boolean = + when (type) { + is ListType -> parameterType.typeArgument(List::class.java) + ?.let { isConcreteScalarType(type.type, it, scalarNames) } ?: false + + is TypeName -> type.name in scalarNames && type.name != "ID" + is NonNullType -> isConcreteScalarType(type.type, parameterType, scalarNames) + else -> false + } + // The two rules differ on purpose: making the build pick by instance would unwrap a Flux as a Publisher, and making the fetch // pick by declared type would stop a CompletableFuture returned for a declared Future from getting its own wrapper private class GenericWrappers(private val wrappers: List) { @@ -391,12 +430,6 @@ private fun List.awaitAll(): Any { private fun Any?.asCompletionStage(): CompletionStage = this as? CompletionStage ?: CompletableFuture.completedFuture(this) -internal enum class ResolverKind(val noun: String, val holdCompletionStage: String) { - METHOD("method", "Return a CompletionStage, such as a CompletableFuture, from the method or its generic wrapper transformer instead."), - FIELD("field", "Assign a CompletionStage, such as a CompletableFuture, to the field, or return one from its generic wrapper transformer instead."), - MAP_VALUE("map value", "Put a CompletionStage, such as a CompletableFuture, in the map, or return one from its generic wrapper transformer instead.") -} - // graphql-java only waits on a CompletionStage returned by a data fetcher, so any other future, or a future // that a suspend function completes with, would reach it as the field value private class Awaitable( diff --git a/src/main/kotlin/graphql/kickstart/tools/util/Utils.kt b/src/main/kotlin/graphql/kickstart/tools/util/Utils.kt index 14c156ed..06450246 100644 --- a/src/main/kotlin/graphql/kickstart/tools/util/Utils.kt +++ b/src/main/kotlin/graphql/kickstart/tools/util/Utils.kt @@ -8,12 +8,9 @@ import graphql.schema.idl.TypeUtil import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import org.apache.commons.lang3.reflect.TypeUtils -import java.lang.reflect.Method import java.lang.reflect.ParameterizedType -import java.lang.reflect.Proxy import java.util.concurrent.CompletionStage import java.util.concurrent.Future -import kotlin.reflect.jvm.kotlinFunction /** * @author Andrew Potter @@ -22,7 +19,6 @@ import kotlin.reflect.jvm.kotlinFunction internal typealias GraphQLRootResolver = GraphQLResolver internal typealias JavaType = java.lang.reflect.Type -internal typealias JavaMethod = Method internal typealias GraphQLLangType = Type<*> internal fun Type<*>.unwrap(): TypeName = TypeUtil.unwrapAll(this) @@ -61,26 +57,6 @@ internal fun DataFetchingEnvironment.coroutineScope(): CoroutineScope { return CoroutineScope(Dispatchers.Default) } -internal val Class<*>.declaredNonProxyMethods: List - get() { - return when { - Proxy.isProxyClass(this) -> emptyList() - else -> this.declaredMethods.toList() - } - } - -internal fun JavaMethod.isSuspendFunction(): Boolean { - return try { - this.kotlinFunction?.isSuspend == true - } catch (e: InternalError) { - false - } -} - -// the trailing Continuation of a suspend function isn't a resolver argument -internal fun JavaMethod.parameterCountWithoutContinuation(): Int = - if (isSuspendFunction()) parameterCount - 1 else parameterCount - internal fun getDocumentation(node: AbstractNode<*>, options: SchemaParserOptions): String? = when { node is AbstractDescribedNode<*> && node.description != null -> node.description?.content diff --git a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt index 607ef081..fee6571f 100644 --- a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverDataFetcherTest.kt @@ -5,7 +5,7 @@ import graphql.GraphQLContext import graphql.execution.* import graphql.execution.instrumentation.SimplePerformantInstrumentation import graphql.kickstart.tools.resolver.FieldResolverError -import graphql.kickstart.tools.resolver.FieldResolverScanner +import graphql.kickstart.tools.resolver.ResolverPlan import graphql.language.FieldDefinition import graphql.language.InputValueDefinition import graphql.language.NonNullType @@ -461,7 +461,7 @@ class MethodFieldResolverDataFetcherTest { } else { NormalResolverInfo(resolver, options) } - return FieldResolverScanner(options, GRAPHQL_SCALARS.keys).findFieldResolver(field, resolverInfo).createDataFetcher() + return ResolverPlan.Finder(options, GRAPHQL_SCALARS.keys).find(field, resolverInfo).dataFetcher() } private fun createEnvironment( diff --git a/src/test/kotlin/graphql/kickstart/tools/ResolverCompatibilityTest.kt b/src/test/kotlin/graphql/kickstart/tools/ResolverCompatibilityTest.kt index ddf81e21..1c2bd759 100644 --- a/src/test/kotlin/graphql/kickstart/tools/ResolverCompatibilityTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/ResolverCompatibilityTest.kt @@ -6,6 +6,8 @@ import graphql.GraphQL import graphql.GraphQLContext import graphql.execution.DataFetcherResult import graphql.kickstart.tools.SchemaParserOptions.GenericWrapper +import graphql.kickstart.tools.resolver.ExtraParameter +import graphql.kickstart.tools.resolver.ResolverKind import graphql.schema.DataFetchingEnvironment import kotlinx.coroutines.flow.toList import kotlinx.coroutines.reactive.asFlow @@ -91,6 +93,54 @@ class ResolverCompatibilityTest { ) } + @Test + fun `every resolver kind serves a field`() { + ResolverKind.entries.forEach { kind -> + val holder: Any = when (kind) { + ResolverKind.METHOD -> NounMethodHolder(kind.noun) + ResolverKind.FIELD -> NounFieldHolder(kind.noun) + ResolverKind.MAP_VALUE -> hashMapOf("value" to kind.noun) + } + val schema = SchemaParser.newParser() + .schemaString("type Query { holder: Holder } type Holder { value: String }") + .resolvers(object : GraphQLQueryResolver { + fun holder(): Any = holder + }) + .dictionary("Holder", holder.javaClass) + .build() + .makeExecutableSchema() + + assertEquals(GraphQL.newGraphQL(schema).build().execute("{ holder { value } }").render(), "{holder={value=${kind.noun}}}") + } + } + + @Test + fun `every extra parameter binds after the arguments`() { + val options = SchemaParserOptions.newOptions().contextClass(AppContext::class).build() + val context = mapOf(AppContext::class.java to AppContext("ann"), "key" to "k") + + ExtraParameter.entries.forEach { parameter -> + val (resolver, expected) = when (parameter) { + ExtraParameter.ENVIRONMENT -> object : GraphQLQueryResolver { + fun value(first: String, env: DataFetchingEnvironment) = "$first ${env.field.name}" + } to "x value" + + ExtraParameter.GRAPHQL_CONTEXT -> object : GraphQLQueryResolver { + fun value(first: String, context: GraphQLContext) = "$first ${context.get("key")}" + } to "x k" + + ExtraParameter.CONTEXT_CLASS -> object : GraphQLQueryResolver { + fun value(first: String, context: AppContext) = "$first ${context.user}" + } to "x ann" + } + + assertEquals( + execute("type Query { value(first: String!): String }", "{ value(first: \"x\") }", resolver, options = options, context = context), + "{value=$expected}" + ) + } + } + @Test fun `omitted arguments reach primitive parameters as their default, and a type variable parameter gets a value of its resolved type argument`() { val result = execute( @@ -349,6 +399,12 @@ class ResolverCompatibilityTest { class AppContext(val user: String) + class NounMethodHolder(private val noun: String) { + fun value() = noun + } + + class NounFieldHolder(@JvmField val value: String) + class ContextResolver : GraphQLQueryResolver { fun extras(first: String, env: DataFetchingEnvironment, context: AppContext, graphQLContext: GraphQLContext, again: DataFetchingEnvironment) = "$first ${env.field.name} ${context.user} ${graphQLContext.get("key")} ${again === env}" diff --git a/src/test/kotlin/graphql/kickstart/tools/ResolverOwnershipTest.kt b/src/test/kotlin/graphql/kickstart/tools/ResolverOwnershipTest.kt new file mode 100644 index 00000000..5f76cffd --- /dev/null +++ b/src/test/kotlin/graphql/kickstart/tools/ResolverOwnershipTest.kt @@ -0,0 +1,176 @@ +package graphql.kickstart.tools + +import com.tngtech.archunit.base.DescribedPredicate +import com.tngtech.archunit.core.domain.JavaAccess +import com.tngtech.archunit.core.domain.JavaClass +import com.tngtech.archunit.core.domain.JavaClasses +import com.tngtech.archunit.core.importer.ClassFileImporter +import com.tngtech.archunit.lang.ArchCondition +import com.tngtech.archunit.lang.ConditionEvents +import com.tngtech.archunit.lang.SimpleConditionEvent +import com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes +import com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses +import graphql.execution.DataFetcherResult +import graphql.kickstart.tools.SchemaParserOptions.GenericWrapper +import graphql.kickstart.tools.resolver.ArgumentConverter +import graphql.kickstart.tools.resolver.ResolverPlan +import graphql.kickstart.tools.resolver.TypeShape +import graphql.schema.DataFetcher +import org.apache.commons.lang3.reflect.FieldUtils +import org.junit.Test +import tools.jackson.databind.ObjectMapper +import tools.jackson.databind.ObjectReader +import tools.jackson.databind.ObjectWriter +import tools.jackson.databind.util.TokenBuffer +import java.lang.reflect.AccessibleObject +import java.lang.reflect.Executable +import java.lang.reflect.Field +import java.lang.reflect.Method +import java.nio.file.Paths + +class ResolverOwnershipTest { + + companion object { + private val classes: JavaClasses = ClassFileImporter() + .importPath(Paths.get(ResolverPlan::class.java.protectionDomain.codeSource.location.toURI())) + + private val resolverPackage = ResolverPlan::class.java.packageName + private const val MEMBER = "graphql.kickstart.tools.resolver.Member" + } + + private fun declaredIn(fileName: String) = object : DescribedPredicate("are declared in $resolverPackage's $fileName") { + override fun test(javaClass: JavaClass) = + javaClass.packageName == resolverPackage && javaClass.source.flatMap { it.fileName }.orElse(null) == fileName + } + + private fun DescribedPredicate.orOwnerOf(vararg allowed: Class<*>): DescribedPredicate { + val names = allowed.map { it.name } + return object : DescribedPredicate("${this@orOwnerOf.description} or are ${names.joinToString()}") { + override fun test(javaClass: JavaClass) = this@orOwnerOf.test(javaClass) || javaClass.name.substringBefore('$') in names + } + } + + private class Calls(description: String, private val targets: Map>) : DescribedPredicate>(description) { + override fun test(access: JavaAccess<*>) = + targets.any { (owner, names) -> access.target.owner.isAssignableTo(owner) && access.target.name in names } + } + + private val memberReflection = Calls( + "reads, invokes or opens a Java member by reflection", + mapOf( + Method::class.java.name to setOf("invoke"), + Field::class.java.name to setOf("get", "getGenericType"), + Executable::class.java.name to setOf("getGenericParameterTypes", "getGenericReturnType", "getParameterTypes", "getParameterCount"), + AccessibleObject::class.java.name to setOf("setAccessible", "trySetAccessible"), + "kotlin.reflect.jvm.ReflectJvmMapping" to setOf("getKotlinFunction"), + FieldUtils::class.java.name to setOf("getAllFields", "getAllFieldsList"), + ) + ) + + // input-object field types are guessed from getters until they come from the mapping Jackson uses + private val guessesInputFieldTypes = object : DescribedPredicate>("is in ScanDictionary.findInputValueTypeInType") { + override fun test(access: JavaAccess<*>) = + access.origin.owner.isEquivalentTo(ScanDictionary::class.java) && access.origin.name.startsWith("findInputValueTypeInType") + } + + private val tokenBufferConversion = Calls( + "converts a value through a Jackson TokenBuffer", + mapOf( + TokenBuffer::class.java.name to setOf("", "forInputParsing", "forGeneration", "asParser"), + ObjectWriter::class.java.name to setOf("writeValueIntoBuffer"), + ObjectMapper::class.java.name to setOf("readValue", "convertValue", "treeToValue", "valueToTree"), + ObjectReader::class.java.name to setOf("readValue", "readValues", "treeToValue"), + ) + ) + + private val argumentConversion = Calls( + "builds or calls an ArgumentConverter", + mapOf(ArgumentConverter::class.java.name to setOf("", "convert")) + ) + + private val genericWrapperDispatch = Calls( + "reads a generic wrapper's type, schema wrapper or transformer", + mapOf(GenericWrapper::class.java.name to setOf("getType", "getIndex", "getSchemaWrapper", "getTransformer", "getTransformerReturnsCompletionStage")) + ) + + private val buildsAnUnwrapper = object : DescribedPredicate>("builds a TypeShape.Unwrapper") { + override fun test(access: JavaAccess<*>) = + access.target.owner.isEquivalentTo(TypeShape.Unwrapper::class.java) && access.target.name == "" + } + + private val holdsAnUnwrapper =object : ArchCondition("hold a TypeShape.Unwrapper") { + override fun check(javaClass: JavaClass, events: ConditionEvents) { + if (javaClass.allFields.none { it.rawType.isEquivalentTo(TypeShape.Unwrapper::class.java) }) { + events.add(SimpleConditionEvent.violated(javaClass, "${javaClass.name} implements DataFetcher without a TypeShape.Unwrapper field")) + } + } + } + + @Test + fun `imports the compiled resolver classes`() { + assert(classes.contain(MEMBER)) { "expected $MEMBER, the member variants' base class in ResolverPlan.kt, among ${classes.size} classes" } + assert(classes.any { declaredIn("ResolverPlan.kt").test(it) }) { "no class compiled from ResolverPlan.kt was imported" } + } + + @Test + fun `only the resolver plan reflects on resolver members`() { + noClasses().that(DescribedPredicate.not(declaredIn("ResolverPlan.kt"))) + .should().accessTargetWhere(DescribedPredicate.and(memberReflection, DescribedPredicate.not(guessesInputFieldTypes))) + .because("ResolverPlan.kt reads each field's member once, at schema build, and is the only place that invokes it. " + + "Read what you need from the ResolverPlan, such as its argument targets or type shapes, or add it there") + .check(classes) + } + + @Test + fun `member variants expose raw invocation, never a data fetcher`() { + noClasses().that().areAssignableTo(MEMBER) + .should().dependOnClassesThat().areAssignableTo(DataFetcher::class.java) + .orShould().dependOnClassesThat().areAssignableTo(TypeShape.Unwrapper::class.java) + .because("ResolverPlan builds every data fetcher around a member's read and unwraps its value with TypeShape.Unwrapper, " + + "so a member that builds its own fetcher skips the unwrapping. Give the member a read and let ResolverPlan.dataFetcher wrap it") + .check(classes) + } + + @Test + fun `every library data fetcher unwraps through TypeShape`() { + classes().that().implement(DataFetcher::class.java) + .should(holdsAnUnwrapper) + .because("a resolver's value reaches graphql-java fully unwrapped only through TypeShape.Unwrapper. Add the member to ResolverPlan instead of a new fetcher") + .check(classes) + } + + @Test + fun `only the resolver plan builds an unwrapper`() { + noClasses().that(DescribedPredicate.not(declaredIn("ResolverPlan.kt").orOwnerOf(TypeShape::class.java))) + .should().accessTargetWhere(buildsAnUnwrapper) + .because("ResolverPlan.dataFetcher builds one unwrapper per field from the type shape the scan matched") + .check(classes) + } + + @Test + fun `only TypeShape dispatches on containers`() { + noClasses().that(DescribedPredicate.not(declaredIn("TypeShape.kt").orOwnerOf(GenericWrapper::class.java))) + .should().accessTargetWhere(genericWrapperDispatch) + .orShould().dependOnClassesThat().areAssignableTo(DataFetcherResult::class.java) + .because("TypeShape.kt walks a declared type once and unwraps values by the same layers, so a container handled anywhere else " + + "can disagree with it. Add a ContainerKind or a generic wrapper, or ask TypeShape") + .check(classes) + } + + @Test + fun `only the argument converter converts through a TokenBuffer`() { + noClasses().that(DescribedPredicate.not(declaredIn("ArgumentConverter.kt"))) + .should().accessTargetWhere(tokenBufferConversion) + .orShould().dependOnClassesThat().areAssignableTo(TokenBuffer::class.java) + .because("ArgumentConverter is the one entry point that turns a coerced argument into a parameter's type, with its fallbacks for scalars Jackson can't convert. Use the ResolverPlan's ArgumentConverter") + .check(classes) + } + + @Test + fun `only the resolver plan converts arguments`() { + noClasses().that(DescribedPredicate.not(declaredIn("ResolverPlan.kt").orOwnerOf(ArgumentConverter::class.java))) + .should().accessTargetWhere(argumentConversion) + .because("ResolverPlan builds one ArgumentConverter per argument at schema build, next to the argument's Optional and scalar handling. Read the plan's argument targets instead") + .check(classes) + } +} diff --git a/src/test/kotlin/graphql/kickstart/tools/FieldResolverScannerTest.kt b/src/test/kotlin/graphql/kickstart/tools/ResolverPlanTest.kt similarity index 58% rename from src/test/kotlin/graphql/kickstart/tools/FieldResolverScannerTest.kt rename to src/test/kotlin/graphql/kickstart/tools/ResolverPlanTest.kt index c760c2a7..ff48e52c 100644 --- a/src/test/kotlin/graphql/kickstart/tools/FieldResolverScannerTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/ResolverPlanTest.kt @@ -2,9 +2,7 @@ package graphql.kickstart.tools import graphql.kickstart.tools.SchemaParserOptions.Companion.defaultOptions import graphql.kickstart.tools.resolver.FieldResolverError -import graphql.kickstart.tools.resolver.FieldResolverScanner -import graphql.kickstart.tools.resolver.MethodFieldResolver -import graphql.kickstart.tools.resolver.PropertyFieldResolver +import graphql.kickstart.tools.resolver.ResolverPlan import graphql.language.FieldDefinition import graphql.language.InputValueDefinition import graphql.language.TypeName @@ -15,91 +13,90 @@ import graphql.schema.DataFetchingEnvironment import org.junit.Test import java.util.* -class FieldResolverScannerTest { +class ResolverPlanTest { private val options = defaultOptions() - private val scanner = FieldResolverScanner(options, GRAPHQL_SCALARS.keys) + private val finder = ResolverPlan.Finder(options, GRAPHQL_SCALARS.keys) + + private fun ResolverPlan.reference() = typeShapes().last().reference.toString() @Test - fun `scanner finds fields on multiple root types`() { + fun `finder finds fields on multiple root types`() { val resolver = RootResolverInfo(listOf(RootQuery1(), RootQuery2()), options) - val result1 = scanner.findFieldResolver(FieldDefinition("field1", TypeName("String")), resolver) - val result2 = scanner.findFieldResolver(FieldDefinition("field2", TypeName("String")), resolver) + val result1 = finder.find(FieldDefinition("field1", TypeName("String")), resolver) + val result2 = finder.find(FieldDefinition("field2", TypeName("String")), resolver) assertNotEquals(result1.search.source, result2.search.source) } @Test(expected = FieldResolverError::class) - fun `scanner throws exception when more than one resolver method is found`() { + fun `finder throws exception when more than one resolver method is found`() { val resolver = RootResolverInfo(listOf(RootQuery1(), DuplicateQuery()), options) - scanner.findFieldResolver(FieldDefinition("field1", TypeName("String")), resolver) + finder.find(FieldDefinition("field1", TypeName("String")), resolver) } @Test(expected = FieldResolverError::class) - fun `scanner throws exception when no resolver methods are found`() { + fun `finder throws exception when no resolver methods are found`() { val resolver = RootResolverInfo(listOf(), options) - scanner.findFieldResolver(FieldDefinition("field1", TypeName("String")), resolver) + finder.find(FieldDefinition("field1", TypeName("String")), resolver) } @Test - fun `scanner finds properties when no method is found`() { + fun `finder finds properties when no method is found`() { val resolver = RootResolverInfo(listOf(PropertyQuery()), options) - val name = scanner.findFieldResolver(FieldDefinition("name", TypeName("String")), resolver) - val version = scanner.findFieldResolver(FieldDefinition("version", TypeName("Integer")), resolver) + val name = finder.find(FieldDefinition("name", TypeName("String")), resolver) + val version = finder.find(FieldDefinition("version", TypeName("Integer")), resolver) - assert(name is PropertyFieldResolver) - assert(version is PropertyFieldResolver) + assertEquals(name.reference(), "type of field ${PropertyQuery::class.java.getDeclaredField("name")}") + assertEquals(version.reference(), "type of field ${ParentPropertyQuery::class.java.getDeclaredField("version")}") } @Test - fun `scanner finds generic return type`() { + fun `finder finds generic return type`() { val resolver = RootResolverInfo(listOf(GenericQuery()), options) - val users = scanner.findFieldResolver(FieldDefinition("users", TypeName("UserConnection")), resolver) + val users = finder.find(FieldDefinition("users", TypeName("UserConnection")), resolver) - assert(users is MethodFieldResolver) + assertEquals(users.reference(), "return type of method ${GenericQuery::class.java.getMethod("getUsers")}") } @Test - fun `scanner prefers concrete resolver`() { + fun `finder prefers concrete resolver`() { val resolver = DataClassResolverInfo(Kayak::class.java) - val meta = scanner.findFieldResolver(FieldDefinition("information", TypeName("VehicleInformation")), resolver) + val meta = finder.find(FieldDefinition("information", TypeName("VehicleInformation")), resolver) - assert(meta is MethodFieldResolver) - assertEquals((meta as MethodFieldResolver).method.returnType, BoatInformation::class.java) + assertEquals(meta.typeShapes().last().leafJavaType, BoatInformation::class.java) } @Test - fun `scanner finds field resolver method using camelCase for snake_cased field_name`() { + fun `finder finds field resolver method using camelCase for snake_cased field_name`() { val resolver = RootResolverInfo(listOf(CamelCaseQuery1()), options) - val meta = scanner.findFieldResolver(FieldDefinition("hull_type", TypeName("HullType")), resolver) + val meta = finder.find(FieldDefinition("hull_type", TypeName("HullType")), resolver) - assert(meta is MethodFieldResolver) - assertEquals((meta as MethodFieldResolver).method.returnType, HullType::class.java) + assertEquals(meta.reference(), "return type of method ${CamelCaseQuery1::class.java.getMethod("getHullType")}") } @Test - fun `scanner finds field resolver method using capitalize field_name in different locale`() { + fun `finder finds field resolver method using capitalize field_name in different locale`() { val default = Locale.getDefault() Locale.setDefault(Locale.forLanguageTag("tr-TR")) val resolverInfo = RootResolverInfo(listOf(CapitalizeQuery()), options) - val fieldResolver = scanner.findFieldResolver(FieldDefinition("id", TypeName("HullType")), resolverInfo) + val plan = finder.find(FieldDefinition("id", TypeName("HullType")), resolverInfo) - assert(fieldResolver is MethodFieldResolver) - assertEquals((fieldResolver as MethodFieldResolver).method.returnType, HullType::class.java) + assertEquals(plan.reference(), "return type of method ${CapitalizeQuery::class.java.getMethod("getId")}") Locale.setDefault(default) } @Test - fun `scanner ignores extension functions on root resolvers`() { + fun `finder ignores extension functions on root resolvers`() { val resolver = RootResolverInfo(listOf(UserQuery(), ExtensionQuery()), options) val field = FieldDefinition.newFieldDefinition() .name("user") @@ -107,18 +104,17 @@ class FieldResolverScannerTest { .inputValueDefinition(InputValueDefinition("id", TypeName("ID"))) .build() - val user = scanner.findFieldResolver(field, resolver) + val user = finder.find(field, resolver) - assert(user is MethodFieldResolver) - assertEquals((user as MethodFieldResolver).method.declaringClass, UserQuery::class.java) + assertEquals(user.reference(), "return type of method ${UserQuery::class.java.getMethod("user", String::class.java)}") } @Test - fun `scanner finds field resolver methods in priority order`() { + fun `finder 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 + finder.find(FieldDefinition(name, TypeName(type)), resolverInfo).reference().substringAfter("${PriorityQuery::class.java.name}.").substringBefore("(") assertEquals(methodName("name", "String"), "name") assertEquals(methodName("active", "Boolean"), "isActive") diff --git a/src/test/kotlin/graphql/kickstart/tools/TypeClassMatcherTest.kt b/src/test/kotlin/graphql/kickstart/tools/TypeClassMatcherTest.kt index deb1adee..24b00af3 100644 --- a/src/test/kotlin/graphql/kickstart/tools/TypeClassMatcherTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/TypeClassMatcherTest.kt @@ -2,7 +2,7 @@ package graphql.kickstart.tools import graphql.kickstart.tools.SchemaParserOptions.GenericWrapper import graphql.kickstart.tools.SchemaParserOptions.GenericWrapper.Companion.listCollectionWithTransformer -import graphql.kickstart.tools.resolver.FieldResolverScanner +import graphql.kickstart.tools.resolver.ResolverPlan import graphql.kickstart.tools.util.ParameterizedTypeImpl import graphql.language.* import graphql.language.Type @@ -45,11 +45,11 @@ class TypeClassMatcherTest { ) { x -> x } ).build() - private val scanner: FieldResolverScanner = FieldResolverScanner(options, GRAPHQL_SCALARS.keys) + private val finder = ResolverPlan.Finder(options, GRAPHQL_SCALARS.keys) private val resolver = RootResolverInfo(listOf(QueryMethods()), options) private fun matchReturnType(methodName: String, graphQLType: Type<*>): TypeClassMatcher.Match = - matcher.match(scanner.findFieldResolver(FieldDefinition(methodName, graphQLType), resolver).scanForMatches().single()) + matcher.match(finder.find(FieldDefinition(methodName, graphQLType), resolver).typeShapes().single()) private fun list(other: Type<*> = customType): Type<*> = ListType(other) private fun nonNull(other: Type<*> = customType): Type<*> = NonNullType(other) From 948c1ca65e10570a31aef2468b0434cd93223baf Mon Sep 17 00:00:00 2001 From: Oryan Date: Thu, 5 Nov 2026 21:54:50 -0500 Subject: [PATCH 02/10] Trim the resolver plan's comments and per-argument state, and pin the member lookup order Argument shapes are walked when the scan asks for them instead of being kept for the schema's lifetime, the method fetcher's slots are an array, and the source check captures only the search type. A test pins that a method wins over a field and a field over a Map key, and ResolverOwnershipTest keeps ArgumentConverter construction in ResolverPlan.kt. --- .../kickstart/tools/resolver/ResolverPlan.kt | 80 +++++-------------- .../kickstart/tools/resolver/TypeShape.kt | 5 -- .../kickstart/tools/ResolverOwnershipTest.kt | 1 - .../kickstart/tools/ResolverPlanTest.kt | 23 ++++++ 4 files changed, 42 insertions(+), 67 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt index df227b3f..ef076ed7 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt @@ -46,24 +46,18 @@ import kotlin.reflect.full.extensionReceiverParameter import kotlin.reflect.jvm.javaType import kotlin.reflect.jvm.kotlinFunction -/** - * How one object field is resolved: the Java member behind it, the type shapes the scan matches, and its data fetcher. Every - * fact about the member's signature is read here once, so the scan, the data fetcher and the checks after the build can't - * disagree about it. - */ internal class ResolverPlan private constructor( val field: FieldDefinition, val search: ResolverSearch, private val options: SchemaParserOptions, - // null when the field has no resolver and the options allow that private val member: Member? ) { val resolverInfo: ResolverInfo get() = search.resolverInfo - // provided at parse time, once per method field, so the user's provider doesn't run for a schema that fails to scan + // provided at parse time, so the user's provider doesn't run for a schema that fails to scan val objectMapper: ObjectMapper by lazy { options.objectMapperProvider.provide(field) } - val argumentTargets: List get() = (member as? MethodMember)?.arguments.orEmpty().map { ArgumentTarget(it.definition, it.type) } + val argumentTargets: List get() = (member as? MethodMember)?.arguments.orEmpty() private val returnShape by lazy { member?.let { TypeShape.returnType(field.type, it.declaredValueType, search.isSubscription, search.type, options, it.reference) } @@ -71,7 +65,7 @@ internal class ResolverPlan private constructor( // each shape is walked only when the scanner gets to it, so an earlier match's dictionary error is reported first fun typeShapes(): Sequence = sequence { - (member as? MethodMember)?.arguments?.forEach { yield(it.shape) } + (member as? MethodMember)?.let { yieldAll(it.parameterShapes()) } returnShape?.let { yield(it) } } @@ -90,7 +84,7 @@ internal class ResolverPlan private constructor( is MapMember -> { val unwrapper = if (member.hasTypedValues) TypeShape.Unwrapper(returnShape!!, ResolverKind.MAP_VALUE, member.name) else null - LightFetcher(source, MapMember.sourceResolver, member, unwrapper) + LightFetcher(source, MapMember.anyMap, member, unwrapper) } } } @@ -101,14 +95,14 @@ internal class ResolverPlan private constructor( return SourceResolver { _, _ -> resolver } } + val type = search.type return SourceResolver { environment, sourceObject -> - // if source object is known, environment is null as an optimization (LightDataFetcher) val source = sourceObject ?: environment?.getSource() ?: throw ResolverError("Expected DataFetchingEnvironment and source object to not be null!") - if (!TypeUtils.isAssignable(source.javaClass, search.type)) { - throw ResolverError("Expected source object to be an instance of '${TypeUtils.getRawType(search.type, search.type).name}' but instead got '${source.javaClass.name}'") + if (!TypeUtils.isAssignable(source.javaClass, type)) { + throw ResolverError("Expected source object to be an instance of '${TypeUtils.getRawType(type, type).name}' but instead got '${source.javaClass.name}'") } source @@ -152,10 +146,6 @@ internal class ResolverPlan private constructor( data class NotCallableOnResolver(val message: String, val isField: Boolean) - /** - * Finds the member behind each field. Root fields are searched on every root resolver, so it indexes each class's methods - * only once. - */ class Finder(private val options: SchemaParserOptions, private val scalarNames: Set) { private val log = LoggerFactory.getLogger(javaClass) @@ -249,12 +239,6 @@ internal class ResolverPlan private constructor( 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 listOfNotNull( name, if (isBoolean(field.type)) "is$capitalizedName" else null, @@ -281,8 +265,6 @@ internal class ResolverPlan private constructor( .asSequence() .filter { !it.isSynthetic } .filter { !Modifier.isPrivate(it.modifiers) } - // discard any methods that are coming off the root of the class hierarchy - // to avoid issues with duplicate method declarations .filter { it.declaringClass != Any::class.java } .filter { !search.isSubscription || returnsPublisher(it.returnType, it.genericReturnType, options.genericWrappers) } .toList() @@ -296,7 +278,6 @@ internal class ResolverPlan private constructor( it.eraseUnboundedWildcards() == search.requiredFirstParameterType || method.declaringClass.typeParameters.contains(it) } ?: false } else { - // an extension receiver can only take the source object !isExtensionFunction(method) } @@ -378,18 +359,12 @@ internal class ResolverPlan private constructor( } } -/** - * The kinds of Java member a field can resolve to. The [ResolverPlan.Finder] looks for them in this order. - */ internal enum class ResolverKind(val noun: String, val holdCompletionStage: String) { METHOD("method", "Return a CompletionStage, such as a CompletableFuture, from the method or its generic wrapper transformer instead."), FIELD("field", "Assign a CompletionStage, such as a CompletableFuture, to the field, or return one from its generic wrapper transformer instead."), MAP_VALUE("map value", "Put a CompletionStage, such as a CompletableFuture, in the map, or return one from its generic wrapper transformer instead.") } -/** - * The parameters a resolver method may have after its arguments, listed in the order the missing-resolver error names them. - */ internal enum class ExtraParameter(private val bindingPrecedence: Int) { ENVIRONMENT(bindingPrecedence = 2) { override fun type(options: SchemaParserOptions): Class<*> = DataFetchingEnvironment::class.java @@ -404,7 +379,6 @@ internal enum class ExtraParameter(private val bindingPrecedence: Int) { CONTEXT_CLASS(bindingPrecedence = 1) { override fun type(options: SchemaParserOptions): Class<*>? = options.contextClass - // the context class can also be an argument's type (e.g. a Map for an input object), so only spec scalars count for it override fun takesArgumentSlotOf(argument: InputValueDefinition) = argument.type.unwrap().name in ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS @@ -429,11 +403,9 @@ internal enum class ExtraParameter(private val bindingPrecedence: Int) { fun types(options: SchemaParserOptions): List> = entries.mapNotNull { it.type(options) } - // a class can be the type of more than one entry, e.g. a context class of DataFetchingEnvironment fun of(parameterType: Class<*>, options: SchemaParserOptions): ExtraParameter? = byBindingPrecedence.find { it.type(options) == parameterType } - // whether a parameter of this type in an argument's position is an extra parameter rather than the argument's fun takesArgumentSlot(parameterType: Class<*>, argument: InputValueDefinition, options: SchemaParserOptions): Boolean { val matching = entries.filter { it.type(options) == parameterType } return matching.isNotEmpty() && matching.all { it.takesArgumentSlotOf(argument) } @@ -476,19 +448,15 @@ private class MethodMember( return resolveTypeVariables(declaredType, search.type) } - val arguments: List = field.inputValueDefinitions.mapIndexed { index, definition -> - val type = resolveTypeVariables(method.genericParameterTypes[index + search.firstArgumentIndex], search.type) - Argument(definition, type, lazy { TypeShape.parameterType(definition.type, type, search.type, options, ScanDictionary.MethodParameterReference(method, index)) }) + val arguments: List = field.inputValueDefinitions.mapIndexed { index, definition -> + ResolverPlan.ArgumentTarget(definition, resolveTypeVariables(method.genericParameterTypes[index + search.firstArgumentIndex], search.type)) } - class Argument(val definition: InputValueDefinition, val type: JavaType, private val lazyShape: Lazy) { - val shape by lazyShape + fun parameterShapes(): Sequence = arguments.asSequence().mapIndexed { index, (definition, type) -> + TypeShape.parameterType(definition.type, type, search.type, options, ScanDictionary.MethodParameterReference(method, index)) } - /** - * What to pass for each parameter in the method's order, leaving out the Continuation of a suspend function. - */ - fun slots(mapper: ObjectMapper): List<(DataFetchingEnvironment) -> Any?> { + fun slots(mapper: ObjectMapper): Array<(DataFetchingEnvironment) -> Any?> { val slots = mutableListOf<(DataFetchingEnvironment) -> Any?>() search.requiredFirstParameterType?.let { expectedType -> @@ -509,10 +477,10 @@ private class MethodMember( slots.add(ExtraParameter.of(parameterType, options)!!.bind(parameterType, method)) } - return slots + return slots.toTypedArray() } - private fun argumentSlot(argument: Argument, mapper: ObjectMapper): (DataFetchingEnvironment) -> Any? { + private fun argumentSlot(argument: ResolverPlan.ArgumentTarget, mapper: ObjectMapper): (DataFetchingEnvironment) -> Any? { val definition = argument.definition val isNonNull = definition.type is NonNullType val isOptional = isOptionalParameter(argument.type, search.type) @@ -568,7 +536,6 @@ private class PropertyMember(private val property: Field) : Member() { private class MapMember(field: FieldDefinition, private val search: ResolverSearch) : Member() { private val key = field.name - // Classmate keeps the value type a Map subclass fixes, e.g. a class extending HashMap override val declaredValueType: JavaType = TypeResolver().resolve(search.type.unwrap()).typeParametersFor(Map::class.java).elementAtOrElse(1) { Any::class.java } @@ -583,8 +550,7 @@ private class MapMember(field: FieldDefinition, private val search: ResolverSear override fun toString() = "MapFieldResolver{key=$key}" companion object { - // the light data fetcher reads any Map it's given, without checking its class against the search type - val sourceResolver = SourceResolver { _, source -> + val anyMap = SourceResolver { _, source -> source as? Map<*, *> ?: throw RuntimeException("MapFieldResolver attempt to fetch a field from an object instance that was not a map") } } @@ -595,7 +561,7 @@ private val noArguments = emptyArray() private class Fetcher( private val source: SourceResolver, private val member: MethodMember, - private val slots: List<(DataFetchingEnvironment) -> Any?>, + private val slots: Array<(DataFetchingEnvironment) -> Any?>, private val options: SchemaParserOptions, private val unwrapper: TypeShape.Unwrapper ) : DataFetcher { @@ -603,7 +569,9 @@ private class Fetcher( override fun get(environment: DataFetchingEnvironment): Any? { val target = source.resolve(environment, null) val arguments = arrayOfNulls(if (member.isSuspend) slots.size + 1 else slots.size) - slots.forEachIndexed { index, slot -> arguments[index] = slot(environment) } + for (index in slots.indices) { + arguments[index] = slots[index](environment) + } return if (member.isSuspend) { // start undispatched so DataLoader loads are queued before graphql-java dispatches them, @@ -617,16 +585,10 @@ private class Fetcher( } } - /** - * 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 = source.resolve(environment, null) } -/** - * For a member without parameters, which needs the environment only if a generic wrapper's transformer asks for it. - */ private class LightFetcher( private val source: SourceResolver, private val lightSource: SourceResolver, @@ -642,9 +604,6 @@ private class LightFetcher( override fun get(environment: DataFetchingEnvironment): Any? = get(environment.fieldDefinition, source.resolve(environment, null)) { environment } - /** - * 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 = source.resolve(environment, null) } @@ -665,6 +624,5 @@ private fun Method.isSuspendFunction(): Boolean { } } -// the trailing Continuation of a suspend function isn't a resolver argument private fun Method.parameterCountWithoutContinuation(): Int = if (isSuspendFunction()) parameterCount - 1 else parameterCount diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt index acbb83da..5d38376f 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt @@ -283,11 +283,6 @@ private fun GenericWrapper.canTransformToPublisher() = internal fun isOptionalParameter(parameterType: JavaType, containingType: JavaType): Boolean = TypeUtils.getRawType(parameterType, containingType) == Optional::class.java -/** - * A concrete scalar type is a scalar type where values always coerce to the same Java type. The ID scalar type is not concrete - * because values can be coerced to multiple different Java types (eg. String, Long, UUID). All values of a non-concrete scalar - * type must be converted to the target method parameter type. - */ internal fun isConcreteScalarType(type: GraphQLLangType, parameterType: JavaType, scalarNames: Set): Boolean = when (type) { is ListType -> parameterType.typeArgument(List::class.java) diff --git a/src/test/kotlin/graphql/kickstart/tools/ResolverOwnershipTest.kt b/src/test/kotlin/graphql/kickstart/tools/ResolverOwnershipTest.kt index 5f76cffd..6ceb40c3 100644 --- a/src/test/kotlin/graphql/kickstart/tools/ResolverOwnershipTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/ResolverOwnershipTest.kt @@ -67,7 +67,6 @@ class ResolverOwnershipTest { ) ) - // input-object field types are guessed from getters until they come from the mapping Jackson uses private val guessesInputFieldTypes = object : DescribedPredicate>("is in ScanDictionary.findInputValueTypeInType") { override fun test(access: JavaAccess<*>) = access.origin.owner.isEquivalentTo(ScanDictionary::class.java) && access.origin.name.startsWith("findInputValueTypeInType") diff --git a/src/test/kotlin/graphql/kickstart/tools/ResolverPlanTest.kt b/src/test/kotlin/graphql/kickstart/tools/ResolverPlanTest.kt index ff48e52c..f8ec035b 100644 --- a/src/test/kotlin/graphql/kickstart/tools/ResolverPlanTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/ResolverPlanTest.kt @@ -122,6 +122,29 @@ class ResolverPlanTest { assertEquals(methodName("count", "Int"), "getFieldCount") } + @Test + fun `finder takes a method before a field and a field before a Map key`() { + fun reference(dataClass: Class<*>) = finder.find(FieldDefinition("value", TypeName("String")), DataClassResolverInfo(dataClass)).reference() + + assertEquals(reference(MapWithMethod::class.java), "return type of method ${MapWithMethod::class.java.getMethod("getValue")}") + assertEquals(reference(MapWithField::class.java), "type of field ${MapWithField::class.java.getField("value")}") + assertEquals(reference(PlainMap::class.java), "type of field value") + } + + class PlainMap : HashMap() + + class MapWithField : HashMap() { + @JvmField + val value: String = "field" + } + + class MapWithMethod : HashMap() { + @JvmField + val value: String = "field" + + fun getValue(): String = "method" + } + class RootQuery1 : GraphQLQueryResolver { fun field1() {} } From 7f71049ac8bd1adf205d9a7f2ad2ed53547e037b Mon Sep 17 00:00:00 2001 From: Oryan Date: Thu, 5 Nov 2026 21:55:01 -0500 Subject: [PATCH 03/10] Describe the resolver plan in the rule table and the bounded check skill The rows for deciding a method's signature once and for one argument conversion entry point now name ResolverPlan, ArgumentConverter and ResolverOwnershipTest. The bounded check skill names ResolverPlan.kt where it named the deleted resolver files. --- .../verify-return-value-unwrapping/SKILL.md | 24 +++++++++---------- AGENTS.md | 7 +++--- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/.claude/skills/verify-return-value-unwrapping/SKILL.md b/.claude/skills/verify-return-value-unwrapping/SKILL.md index f0a32b40..0a35bd1c 100644 --- a/.claude/skills/verify-return-value-unwrapping/SKILL.md +++ b/.claude/skills/verify-return-value-unwrapping/SKILL.md @@ -1,6 +1,6 @@ --- name: verify-return-value-unwrapping -description: Exhaustively check, up to a bound, that the value of a resolver method, a field with no matching getter or a typed Map reaches graphql-java fully unwrapped. Futures are awaited, generic wrappers are transformed, Optional and DataFetcherResult are handled, and list order, nulls and failures are kept, for every return type nested up to 3 containers deep. Use it before pushing any change to TypeShape (the type walk, ContainerKind, the generic wrapper rules or the Unwrapper), the data fetchers in MethodFieldResolver, PropertyFieldResolver or MapFieldResolver, or the GenericWrapper defaults, and whenever a bug report says graphql-java got a CompletableFuture, Optional or wrapper object as a field value. +description: Exhaustively check, up to a bound, that the value of a resolver method, a field with no matching getter or a typed Map reaches graphql-java fully unwrapped. Futures are awaited, generic wrappers are transformed, Optional and DataFetcherResult are handled, and list order, nulls and failures are kept, for every return type nested up to 3 containers deep. Use it before pushing any change to TypeShape (the type walk, ContainerKind, the generic wrapper rules or the Unwrapper), the data fetchers in ResolverPlan, or the GenericWrapper defaults, and whenever a bug report says graphql-java got a CompletableFuture, Optional or wrapper object as a field value. --- # Verify return value unwrapping @@ -38,11 +38,11 @@ and an all-non-null GraphQL type. Each return type runs as three kinds of resolver, each compiled as a Java class at runtime: -| Kind | Generated resolver | Data fetcher | +| Kind | Generated resolver | Member in `ResolverPlan.kt` | |---|---|---| -| `method` | `public field()` on the query resolver | `MethodFieldResolver` | -| `property` | `public field;` on the query resolver, which is also what a Kotlin `@JvmField` compiles to | `PropertyFieldResolver` | -| `map` | `extends HashMap>`, bound to `type Holder` and returned by `Query.holder` | `MapFieldResolver` | +| `method` | `public field()` on the query resolver | `MethodMember` | +| `property` | `public field;` on the query resolver, which is also what a Kotlin `@JvmField` compiles to | `PropertyMember` | +| `map` | `extends HashMap>`, bound to `type Holder` and returned by `Query.holder` | `MapMember` | The check builds each schema through the public `SchemaParser` API, which rejects about a fifth of the combinations, such as `Optional` below the top level. For every accepted one it executes the query once for each value that makes at most @@ -154,8 +154,8 @@ or delete it when none fail, so that a regression shows up as `FAIL`. ## What this does not cover - Suspend functions, subscriptions and `Publisher`, arrays, and Kotlin-declared resolver methods. The generated resolvers - are Java methods without parameters, so only `LightMethodFieldResolverDataFetcher` runs, not - `MethodFieldResolverDataFetcher`. + are Java methods without parameters, so only `LightFetcher` in `ResolverPlan.kt` runs, not + `Fetcher`. - Wrappers picked by a subclass of the value (`Box` and `SpecialBox` in `NestedGenericWrapperTest`), wrappers with a type argument index other than 0, custom `schemaWrapper`s other than the list one, transformers that return a `FutureTask`, and DataLoader futures. @@ -169,20 +169,18 @@ or delete it when none fail, so that a regression shows up as `FAIL`. ## Coverage and sync -Last confirmed to describe the code at commit `5ce3d1db` (2026-11-05). It covers: +Last confirmed to describe the code at commit `948c1ca6` (2026-11-06). It covers: | File | What | |---|---| -| `src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt` | all of it: the walk, `ContainerKind`, `GenericWrappers`, `resolveTypeVariables`, the `Unwrapper` and its steps | -| `src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt` | `createDataFetcher` (the `TypeShape.Unwrapper`), `returnShape`, `LightMethodFieldResolverDataFetcher` | +| `src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt` | the walk, `ContainerKind`, `GenericWrappers`, `resolveTypeVariables`, the `Unwrapper` and its steps | +| `src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt` | `dataFetcher` (the `TypeShape.Unwrapper` per kind), `returnShape`, the members' `declaredValueType` and `read`, `Fetcher` and `LightFetcher` | | `src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt` | `GenericWrapper` and the default wrappers in `build()` | -| `src/main/kotlin/graphql/kickstart/tools/resolver/PropertyFieldResolver.kt` | all of it | -| `src/main/kotlin/graphql/kickstart/tools/resolver/MapFieldResolver.kt` | all of it, for Map subclasses with a typed value | To see whether a change needs another look: ```bash -git log --oneline 5ce3d1db..HEAD -- src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt src/main/kotlin/graphql/kickstart/tools/resolver/PropertyFieldResolver.kt src/main/kotlin/graphql/kickstart/tools/resolver/MapFieldResolver.kt +git log --oneline 948c1ca6..HEAD -- src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt ``` Move the sync commit forward only after you've read those changes and confirmed that the containers, variants and diff --git a/AGENTS.md b/AGENTS.md index 23b1caf9..cd466de8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -7,7 +7,8 @@ resolver classes. Published to Maven Central. Kotlin, with a few Java files, tar - `src/main/kotlin/graphql/kickstart/tools`: `SchemaParser` and its builder and options, `MergedDefinitions.kt` (merges each SDL type with its extensions into a `MergedType`. Other code, apart from the Relay factory, reads SDL types only through - it), `SchemaClassScanner` (matches schema types to classes), `resolver/` (field resolvers and data fetchers), `directive/`, + it), `SchemaClassScanner` (matches schema types to classes), `resolver/` (`ResolverPlan.kt` finds the Java member behind each field and builds its data fetcher, `TypeShape.kt` walks + declared types and unwraps values, `ArgumentConverter.kt` converts arguments), `directive/`, `relay/`, `proxy/` (finds the class behind a Spring, Guice, Weld or Javassist proxied resolver), `util/` - `src/test/kotlin`: JUnit tests, one class per feature. A test for an issue goes into the class for its feature. `EndToEndTest` and `EndToEndSpecHelper` hold the shared schema and resolvers @@ -93,13 +94,13 @@ happening. "docs only" means nothing does yet, so the rule depends on you readin | Rule | Enforced by | |---|---| | The value of a resolver method, a field with no matching getter or a typed Map is unwrapped by one rule that the schema build and the data fetcher share | `TypeShape`, whose one walk yields both the class the schema build matches and the layers that `TypeShape.Unwrapper` unwraps in the method, field and Map data fetchers. An untyped Map's values reach graphql-java as they are. A container kind can't be added without its placement rule and its value step, since both are abstract on the enum, and `ReturnValueUnwrappingTest` fails until the bounded check builds values for it | -| A resolver method's parameter layout, type variables and source are decided once, at schema build | docs only | +| A resolver method's parameter layout, type variables and source are decided once, at schema build | `ResolverPlan`, which reads the field's member once and hands the scan its type shapes, the data fetcher its parameters and `UnmappedInputFieldChecker` its argument targets. Its member variants are private to `ResolverPlan.kt` and expose a raw read, never a data fetcher. A new `ResolverKind` doesn't compile until the lookup's `when` handles it, a new `ExtraParameter` doesn't compile without its binder, and the fixture tests in `ResolverCompatibilityTest` don't compile until they build a sample of each. `ResolverOwnershipTest` fails on member reflection outside `ResolverPlan.kt` (apart from the input-field getters in `ScanDictionary.findInputValueTypeInType`), on a member that depends on a data fetcher, on a data fetcher without a `TypeShape.Unwrapper`, and on generic-wrapper or `DataFetcherResult` handling outside `TypeShape.kt` | | A change to how resolvers are found, bound or unwrapped keeps what a method, a property and a Map field return | `ResolverCompatibilityTest`, which compares field and Map values with a method's, and the bounded check in `ReturnValueUnwrappingTest`, which compares all three with the plain value. Neither has known findings, so a value that differs on a return type they run fails the build. The bounded check also pins how many return types it generates, and for each kind how many the build rejects and how many executions run, so a change that stops running some fails until the same diff updates `pinnedCoverage`. It doesn't judge whether a newly rejected return type should build | | One output has one builder. Don't add a second path that builds the same thing | docs only | | Read an SDL type's members merged with its extensions, never from the raw definition | `MergedType` in `MergedDefinitions.kt` and `DefinitionsBoundaryTest`, which fails on a raw member, directive or child-node read, an extension class, an SDL node walk or a `TypeDefinitionRegistry` call outside it. A read by reflection gets past it. `TypeExtensionTest` builds and queries a schema with an extension of every kind, so a build that leaves an extension's fields, interfaces, members, values, input fields or directives out of the built type fails it, however the code read them | | Every kind of SDL type starts its graphql-java type from the same recipe | `MergedType.prepare`, `DefinitionsBoundaryTest` and `TypeKindRecipeTest` | | Reuse an existing structure before you add a parallel one | docs only | -| Argument conversion goes through one entry point | docs only | +| Argument conversion goes through one entry point | `ArgumentConverter`, which `ResolverPlan` builds once per argument. `ResolverOwnershipTest` fails on a TokenBuffer or a Jackson `readValue`, `convertValue`, `treeToValue` or `valueToTree` outside `ArgumentConverter.kt`, and on an `ArgumentConverter` built or called outside `ResolverPlan.kt`. Jackson's per-call retry for scalars it can't convert stays inside `ArgumentConverter` | | An input field's Java type comes from the mapping Jackson uses, not from guessed getters | docs only | | Every recursive walk over the type graph handles cycles, and a test covers a cycle | docs only | | Public signatures stay binary compatible, data class `copy()` and `componentN` included | japicmp with `build/japicmp-kotlin-public.groovy` at `mvn verify`, which allows excludes and other narrowing options only at an X.0.0 version | From 145f5b1f72e15cbb82f65eeeb01d08df0b40ea19 Mon Sep 17 00:00:00 2001 From: Oryan Date: Thu, 5 Nov 2026 22:00:43 -0500 Subject: [PATCH 04/10] Keep logging resolver lookup warnings under the scanner's logger name The probe diff against trunk showed the missing-resolver and inaccessible-field WARNs moving from graphql.kickstart.tools.resolver.FieldResolverScanner to ResolverPlan$Finder, which would change what an app's logging config raises or silences. MissingFieldResolverTest now checks the logger names. --- .../graphql/kickstart/tools/resolver/ResolverPlan.kt | 3 ++- .../kickstart/tools/MissingFieldResolverTest.kt | 10 +++++----- src/test/kotlin/graphql/kickstart/tools/TestUtils.kt | 7 ++++++- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt index ef076ed7..b58fb0d9 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt @@ -148,7 +148,8 @@ internal class ResolverPlan private constructor( class Finder(private val options: SchemaParserOptions, private val scalarNames: Set) { - private val log = LoggerFactory.getLogger(javaClass) + // apps raise or silence these warnings by logger name in their logging config + private val log = LoggerFactory.getLogger("graphql.kickstart.tools.resolver.FieldResolverScanner") private val methodsByNameCache = mutableMapOf, Boolean>, Map>>() diff --git a/src/test/kotlin/graphql/kickstart/tools/MissingFieldResolverTest.kt b/src/test/kotlin/graphql/kickstart/tools/MissingFieldResolverTest.kt index 9272f70e..e2143786 100644 --- a/src/test/kotlin/graphql/kickstart/tools/MissingFieldResolverTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/MissingFieldResolverTest.kt @@ -30,8 +30,8 @@ class MissingFieldResolverTest { } @Test - fun `should warn about each missing field when unimplemented resolvers are allowed`() { - val (schema, warnings) = withWarnings { + fun `should warn about each missing field when unimplemented resolvers are allowed, from the loggers apps configure`() { + val (schema, events) = withWarningEvents { SchemaParser.newParser() .schemaString( """ @@ -51,10 +51,10 @@ class MissingFieldResolverTest { // missingField has no resolver, so querying it throws schema.assertEveryFieldResolves(except = setOf("Query.missingField")) - assertEquals(warnings, listOf( - "Option 'allowUnimplementedResolvers' should only be set to true during development, as it can cause schema errors to be moved " + + assertEquals(events.map { "${it.loggerName}: ${it.formattedMessage}" }, listOf( + "graphql.kickstart.tools.SchemaClassScanner: Option 'allowUnimplementedResolvers' should only be set to true during development, as it can cause schema errors to be moved " + "to query time instead of schema creation time. Make sure this is turned off in production.", - "Missing resolver for field: ${schema.queryType.getFieldDefinition("missingField").definition}" + "graphql.kickstart.tools.resolver.FieldResolverScanner: Missing resolver for field: ${schema.queryType.getFieldDefinition("missingField").definition}" )) } diff --git a/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt b/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt index 2874c8f7..36e060a3 100644 --- a/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt +++ b/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt @@ -41,6 +41,11 @@ fun assertNotNull(actual: T) { } fun withWarnings(block: () -> T): Pair> { + val (result, events) = withWarningEvents(block) + return result to events.map { it.formattedMessage } +} + +fun withWarningEvents(block: () -> T): Pair> { // the tests run in the library's module, which doesn't read logback's listOf("ch.qos.logback.classic", "ch.qos.logback.core").forEach { name -> ModuleLayer.boot().findModule(name).ifPresent { SchemaParser::class.java.module.addReads(it) } @@ -50,7 +55,7 @@ fun withWarnings(block: () -> T): Pair> { logger.addAppender(appender) try { val result = block() - return result to appender.list.filter { it.level.isGreaterOrEqual(Level.WARN) }.map { it.formattedMessage } + return result to appender.list.filter { it.level.isGreaterOrEqual(Level.WARN) } } finally { logger.detachAppender(appender) } From ce6c5e822a093fa8121a757c04940fdf411ee203 Mon Sep 17 00:00:00 2001 From: Oryan Date: Thu, 5 Nov 2026 22:32:14 -0500 Subject: [PATCH 05/10] Keep the data fetcher class names apps reach the resolver through Apps call getWrappedFetchingObject by reflection from Instrumentation.instrumentDataFetcher, and some check a fetcher's class, so the fetchers keep their names and stay public in bytecode. Each member's fetcher is a ResolverDataFetcher, whose constructor takes the unwrapper, and the member's class decides its ResolverKind. The bounded check now fails with MODEL ERROR when a ResolverKind has no kind in Check.java, and the fixture tests name the fixture to add instead of offering an else branch. --- .../kickstart/tools/resolver/ResolverPlan.kt | 159 +++++++++++------- .../kickstart/tools/resolver/TypeShape.kt | 4 + .../tools/ResolverCompatibilityTest.kt | 81 ++++++--- .../tools/WrappedFetchingObjectTest.kt | 66 ++++++++ .../returnvalue/ReturnValueUnwrappingTest.kt | 4 +- src/test/resources/returnvalue/Check.java | 11 ++ 6 files changed, 238 insertions(+), 87 deletions(-) create mode 100644 src/test/kotlin/graphql/kickstart/tools/WrappedFetchingObjectTest.kt diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt index b58fb0d9..3d5ce430 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt @@ -69,23 +69,24 @@ internal class ResolverPlan private constructor( returnShape?.let { yield(it) } } - fun dataFetcher(): DataFetcher<*> { - val member = member - ?: return (options.missingResolverDataFetcherProvider ?: notImplemented).createDataFetcher(field, options) + fun dataFetcher(): DataFetcher<*> = + member?.let(::fetcherOf) ?: (options.missingResolverDataFetcherProvider ?: notImplemented).createDataFetcher(field, options) + + private fun fetcherOf(member: Member): ResolverDataFetcher { val source = sourceResolver() + val unwrapper = TypeShape.Unwrapper(returnShape!!, member.kind, member.name, member.isSuspend) return when (member) { is MethodMember -> { val slots = member.slots(objectMapper) - val unwrapper = TypeShape.Unwrapper(returnShape!!, ResolverKind.METHOD, member.name, member.isSuspend) - if (slots.isNotEmpty() || member.isSuspend) Fetcher(source, member, slots, options, unwrapper) else LightFetcher(source, source, member, unwrapper) + if (slots.isNotEmpty() || member.isSuspend) { + MethodFieldResolverDataFetcher(source, member.method, slots, member.isSuspend, options, unwrapper) + } else { + LightMethodFieldResolverDataFetcher(source, member.method, unwrapper) + } } - is PropertyMember -> LightFetcher(source, source, member, TypeShape.Unwrapper(returnShape!!, ResolverKind.FIELD, member.name)) - - is MapMember -> { - val unwrapper = if (member.hasTypedValues) TypeShape.Unwrapper(returnShape!!, ResolverKind.MAP_VALUE, member.name) else null - LightFetcher(source, MapMember.anyMap, member, unwrapper) - } + is PropertyMember -> PropertyFieldResolverDataFetcher(source, member.property, unwrapper) + is MapMember -> MapFieldResolverDataFetcher(source, member.key, unwrapper) } } @@ -360,10 +361,15 @@ internal class ResolverPlan private constructor( } } -internal enum class ResolverKind(val noun: String, val holdCompletionStage: String) { +internal enum class ResolverKind(val noun: String, val holdCompletionStage: String, val unwrapsObjectValues: Boolean = true) { METHOD("method", "Return a CompletionStage, such as a CompletableFuture, from the method or its generic wrapper transformer instead."), FIELD("field", "Assign a CompletionStage, such as a CompletableFuture, to the field, or return one from its generic wrapper transformer instead."), - MAP_VALUE("map value", "Put a CompletionStage, such as a CompletableFuture, in the map, or return one from its generic wrapper transformer instead.") + + // an untyped Map declares no value type, so unwrapping its values would mean inspecting each one at runtime + MAP_VALUE( + "map value", "Put a CompletionStage, such as a CompletableFuture, in the map, or return one from its generic wrapper transformer instead.", + unwrapsObjectValues = false + ) } internal enum class ExtraParameter(private val bindingPrecedence: Int) { @@ -380,6 +386,7 @@ internal enum class ExtraParameter(private val bindingPrecedence: Int) { CONTEXT_CLASS(bindingPrecedence = 1) { override fun type(options: SchemaParserOptions): Class<*>? = options.contextClass + // the context class can also be an argument's type (e.g. a Map for an input object), so only spec scalars count for it override fun takesArgumentSlotOf(argument: InputValueDefinition) = argument.type.unwrap().name in ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS @@ -416,13 +423,12 @@ internal enum class ExtraParameter(private val bindingPrecedence: Int) { internal class FieldResolverError(msg: String) : RuntimeException(msg) -private sealed class Member { +private sealed class Member(val kind: ResolverKind) { abstract val name: String abstract val declaredValueType: JavaType abstract val reference: ScanDictionary.Reference abstract val javaMember: java.lang.reflect.Member? - - abstract fun read(target: Any, arguments: Array): Any? + open val isSuspend get() = false } private class MethodMember( @@ -431,9 +437,8 @@ private class MethodMember( val method: Method, private val options: SchemaParserOptions, private val scalarNames: Set -) : Member() { - val isSuspend = method.isSuspendFunction() - private val parameterCount = if (isSuspend) method.parameterCount - 1 else method.parameterCount +) : Member(ResolverKind.METHOD) { + override val isSuspend = method.isSuspendFunction() override val name get() = "${method.declaringClass.name}.${method.name}" override val javaMember get() = method @@ -474,7 +479,7 @@ private class MethodMember( arguments.forEach { slots.add(argumentSlot(it, mapper)) } - method.parameterTypes.slice(slots.size until parameterCount).forEach { parameterType -> + method.parameterTypes.slice(slots.size until method.parameterCountWithoutContinuation()).forEach { parameterType -> slots.add(ExtraParameter.of(parameterType, options)!!.bind(parameterType, method)) } @@ -512,103 +517,127 @@ private class MethodMember( } } - override fun read(target: Any, arguments: Array): Any? = invoke(method, target, arguments) - - suspend fun readSuspending(target: Any, arguments: Array): Any? = - suspendCoroutineUninterceptedOrReturn { continuation -> - arguments[arguments.lastIndex] = continuation - invoke(method, target, arguments) - } - + // the build error for a field more than one root resolver serves lists the resolvers by these names override fun toString() = "MethodFieldResolver{method=$method}" } -private class PropertyMember(private val property: Field) : Member() { +private class PropertyMember(val property: Field) : Member(ResolverKind.FIELD) { override val name get() = "${property.declaringClass.name}.${property.name}" override val javaMember get() = property override val declaredValueType: JavaType get() = property.genericType override val reference get() = ScanDictionary.FieldTypeReference(property.toString()) - override fun read(target: Any, arguments: Array): Any? = property.get(target) - override fun toString() = "PropertyFieldResolver{property=$property}" } -private class MapMember(field: FieldDefinition, private val search: ResolverSearch) : Member() { - private val key = field.name +private class MapMember(field: FieldDefinition, private val search: ResolverSearch) : Member(ResolverKind.MAP_VALUE) { + val key: String = field.name override val declaredValueType: JavaType = TypeResolver().resolve(search.type.unwrap()).typeParametersFor(Map::class.java).elementAtOrElse(1) { Any::class.java } - val hasTypedValues get() = resolveTypeVariables(declaredValueType, search.type) != Any::class.java - override val name get() = "${search.type.unwrap().name}.$key" override val javaMember: java.lang.reflect.Member? get() = null override val reference get() = ScanDictionary.FieldTypeReference(key) - override fun read(target: Any, arguments: Array): Any? = (target as Map<*, *>)[key] - override fun toString() = "MapFieldResolver{key=$key}" - - companion object { - val anyMap = SourceResolver { _, source -> - source as? Map<*, *> ?: throw RuntimeException("MapFieldResolver attempt to fetch a field from an object instance that was not a map") - } - } } -private val noArguments = emptyArray() +// the only data fetchers a member gets, so every resolver value goes through the unwrapper the scan matched for its field +internal sealed class ResolverDataFetcher(protected val source: SourceResolver, protected val unwrapper: TypeShape.Unwrapper) : DataFetcher -private class Fetcher( - private val source: SourceResolver, - private val member: MethodMember, +internal class MethodFieldResolverDataFetcher( + source: SourceResolver, + private val method: Method, private val slots: Array<(DataFetchingEnvironment) -> Any?>, + private val isSuspend: Boolean, private val options: SchemaParserOptions, - private val unwrapper: TypeShape.Unwrapper -) : DataFetcher { + unwrapper: TypeShape.Unwrapper +) : ResolverDataFetcher(source, unwrapper) { override fun get(environment: DataFetchingEnvironment): Any? { val target = source.resolve(environment, null) - val arguments = arrayOfNulls(if (member.isSuspend) slots.size + 1 else slots.size) + val arguments = arrayOfNulls(if (isSuspend) slots.size + 1 else slots.size) for (index in slots.indices) { arguments[index] = slots[index](environment) } - return if (member.isSuspend) { + return if (isSuspend) { // 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() - member.readSuspending(target, arguments)?.let { unwrapper.unwrapSuspending(it) { environment } } + invokeSuspending(method, target, arguments)?.let { unwrapper.unwrapSuspending(it) { environment } } } } else { - member.read(target, arguments)?.let { unwrapper.unwrap(it) { environment } } + invoke(method, target, arguments)?.let { unwrapper.unwrap(it) { environment } } } } - @Suppress("unused") + /** + * The object the method is called on, the resolver or the source object. Apps call this by reflection from graphql-java's + * `Instrumentation.instrumentDataFetcher` to reach the resolver behind a field. + */ fun getWrappedFetchingObject(environment: DataFetchingEnvironment): Any = source.resolve(environment, null) } -private class LightFetcher( - private val source: SourceResolver, - private val lightSource: SourceResolver, - private val member: Member, - private val unwrapper: TypeShape.Unwrapper? -) : LightDataFetcher { +/** + * [MethodFieldResolverDataFetcher] for a method without parameters, which graphql-java calls without building an environment. + */ +internal class LightMethodFieldResolverDataFetcher( + source: SourceResolver, + private val method: Method, + unwrapper: TypeShape.Unwrapper +) : ResolverDataFetcher(source, unwrapper), LightDataFetcher { - override fun get(fieldDefinition: GraphQLFieldDefinition?, sourceObject: Any?, environmentSupplier: Supplier): Any? { - val value = member.read(lightSource.resolve(null, sourceObject), noArguments) - return if (value == null || unwrapper == null) value else unwrapper.unwrap(value, environmentSupplier) - } + override fun get(fieldDefinition: GraphQLFieldDefinition?, sourceObject: Any?, environmentSupplier: Supplier): Any? = + invoke(method, source.resolve(null, sourceObject), noArguments)?.let { unwrapper.unwrap(it, environmentSupplier) } override fun get(environment: DataFetchingEnvironment): Any? = get(environment.fieldDefinition, source.resolve(environment, null)) { environment } - @Suppress("unused") + /** + * See [MethodFieldResolverDataFetcher.getWrappedFetchingObject]. + */ fun getWrappedFetchingObject(environment: DataFetchingEnvironment): Any = source.resolve(environment, null) } +internal class PropertyFieldResolverDataFetcher( + source: SourceResolver, + private val property: Field, + unwrapper: TypeShape.Unwrapper +) : ResolverDataFetcher(source, unwrapper), LightDataFetcher { + + override fun get(fieldDefinition: GraphQLFieldDefinition?, sourceObject: Any?, environmentSupplier: Supplier): Any? = + property.get(source.resolve(null, sourceObject))?.let { unwrapper.unwrap(it, environmentSupplier) } + + override fun get(environment: DataFetchingEnvironment): Any? = + get(environment.fieldDefinition, source.resolve(environment, null)) { environment } +} + +internal class MapFieldResolverDataFetcher( + source: SourceResolver, + private val key: String, + unwrapper: TypeShape.Unwrapper +) : ResolverDataFetcher(source, unwrapper), LightDataFetcher { + + override fun get(fieldDefinition: GraphQLFieldDefinition?, sourceObject: Any?, environmentSupplier: Supplier): Any? { + val map = sourceObject as? Map<*, *> ?: throw RuntimeException("MapFieldResolver attempt to fetch a field from an object instance that was not a map") + return map[key]?.let { unwrapper.unwrap(it, environmentSupplier) } + } + + override fun get(environment: DataFetchingEnvironment): Any? = + get(environment.fieldDefinition, source.resolve(environment, null)) { environment } +} + +private val noArguments = emptyArray() + +private suspend fun invokeSuspending(method: Method, instance: Any, arguments: Array): Any? = + suspendCoroutineUninterceptedOrReturn { continuation -> + arguments[arguments.lastIndex] = continuation + invoke(method, instance, arguments) + } + private fun invoke(method: Method, instance: Any, arguments: Array): Any? { try { return method.invoke(instance, *arguments) diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt index 5d38376f..de203674 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt @@ -49,6 +49,7 @@ internal class TypeShape private constructor( fun error(msg: String) = matchError(graphQLType, reference, javaType, msg) class Unwrapper(shape: TypeShape, resolverKind: ResolverKind, resolverName: String, isSuspendFunction: Boolean = false) { + private val returnsValueAsIs = !resolverKind.unwrapsObjectValues && shape.declaredType == Any::class.java private val wrappers = shape.wrappers private val awaitable: Awaitable private val isOutermostFuture: Boolean @@ -71,6 +72,9 @@ internal class TypeShape private constructor( } fun unwrap(value: Any, environmentSupplier: Supplier): Any? { + if (returnsValueAsIs) { + return value + } val wrapper = wrappers.ofValue(value) val transformed = if (wrapper == null) value else wrapper.transformer(value, environmentSupplier.get()) val unwrapped = awaitable.requireOutermost(transformed) diff --git a/src/test/kotlin/graphql/kickstart/tools/ResolverCompatibilityTest.kt b/src/test/kotlin/graphql/kickstart/tools/ResolverCompatibilityTest.kt index 1c2bd759..ffeeea07 100644 --- a/src/test/kotlin/graphql/kickstart/tools/ResolverCompatibilityTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/ResolverCompatibilityTest.kt @@ -7,12 +7,15 @@ import graphql.GraphQLContext import graphql.execution.DataFetcherResult import graphql.kickstart.tools.SchemaParserOptions.GenericWrapper import graphql.kickstart.tools.resolver.ExtraParameter +import graphql.kickstart.tools.resolver.FieldResolverError import graphql.kickstart.tools.resolver.ResolverKind import graphql.schema.DataFetchingEnvironment +import graphql.schema.FieldCoordinates import kotlinx.coroutines.flow.toList import kotlinx.coroutines.reactive.asFlow import kotlinx.coroutines.reactive.publish import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertThrows import org.junit.Assert.fail import org.junit.Test import org.reactivestreams.Publisher @@ -94,45 +97,49 @@ class ResolverCompatibilityTest { } @Test - fun `every resolver kind serves a field`() { + fun `every resolver kind gets its own data fetcher, which unwraps the value`() { ResolverKind.entries.forEach { kind -> - val holder: Any = when (kind) { - ResolverKind.METHOD -> NounMethodHolder(kind.noun) - ResolverKind.FIELD -> NounFieldHolder(kind.noun) - ResolverKind.MAP_VALUE -> hashMapOf("value" to kind.noun) - } + val fixture = kindFixtures[kind] + ?: throw AssertionError("ResolverKind.$kind has no entry in ResolverCompatibilityTest.kindFixtures. Add a holder whose 'value' resolves through it as a Box, and the name of the data fetcher it gets") + val holder = fixture.holder(kind.noun) val schema = SchemaParser.newParser() .schemaString("type Query { holder: Holder } type Holder { value: String }") .resolvers(object : GraphQLQueryResolver { fun holder(): Any = holder }) .dictionary("Holder", holder.javaClass) + .options(SchemaParserOptions.newOptions().genericWrappers(boxWrapper).build()) .build() .makeExecutableSchema() + val holderType = schema.getObjectType("Holder")!! + val dataFetcher = schema.codeRegistry.getDataFetcher(FieldCoordinates.coordinates(holderType, "value"), holderType.getFieldDefinition("value")) + assertEquals(dataFetcher.javaClass.simpleName, fixture.dataFetcher) assertEquals(GraphQL.newGraphQL(schema).build().execute("{ holder { value } }").render(), "{holder={value=${kind.noun}}}") } } + @Test + fun `a field more than one root resolver serves fails the build, naming each resolver`() { + val error = assertThrows(FieldResolverError::class.java) { + SchemaParser.newParser().schemaString("type Query { value: String }").resolvers(ValueMethodRoot(), ValueFieldRoot()).build().makeExecutableSchema() + } + + assertEquals( + error.message!!.substringAfter("': "), + "[MethodFieldResolver{method=public final java.lang.String ${ValueMethodRoot::class.java.name}.value()}, " + + "PropertyFieldResolver{property=public final java.lang.String ${ValueFieldRoot::class.java.name}.value}]" + ) + } + @Test fun `every extra parameter binds after the arguments`() { val options = SchemaParserOptions.newOptions().contextClass(AppContext::class).build() val context = mapOf(AppContext::class.java to AppContext("ann"), "key" to "k") ExtraParameter.entries.forEach { parameter -> - val (resolver, expected) = when (parameter) { - ExtraParameter.ENVIRONMENT -> object : GraphQLQueryResolver { - fun value(first: String, env: DataFetchingEnvironment) = "$first ${env.field.name}" - } to "x value" - - ExtraParameter.GRAPHQL_CONTEXT -> object : GraphQLQueryResolver { - fun value(first: String, context: GraphQLContext) = "$first ${context.get("key")}" - } to "x k" - - ExtraParameter.CONTEXT_CLASS -> object : GraphQLQueryResolver { - fun value(first: String, context: AppContext) = "$first ${context.user}" - } to "x ann" - } + val (resolver, expected) = extraParameterFixtures[parameter] + ?: throw AssertionError("ExtraParameter.$parameter has no entry in ResolverCompatibilityTest.extraParameterFixtures. Add a resolver whose value(first) takes it last, and what it returns") assertEquals( execute("type Query { value(first: String!): String }", "{ value(first: \"x\") }", resolver, options = options, context = context), @@ -400,10 +407,24 @@ class ResolverCompatibilityTest { class AppContext(val user: String) class NounMethodHolder(private val noun: String) { - fun value() = noun + fun value() = Box(noun) + } + + class NounFieldHolder(noun: String) { + @JvmField val value = Box(noun) + } + + class NounMap : HashMap>() + + class KindFixture(val holder: (String) -> Any, val dataFetcher: String) + + class ValueMethodRoot : GraphQLQueryResolver { + fun value() = "method" } - class NounFieldHolder(@JvmField val value: String) + class ValueFieldRoot : GraphQLQueryResolver { + @JvmField val value = "field" + } class ContextResolver : GraphQLQueryResolver { fun extras(first: String, env: DataFetchingEnvironment, context: AppContext, graphQLContext: GraphQLContext, again: DataFetchingEnvironment) = @@ -443,6 +464,24 @@ class ResolverCompatibilityTest { val leftWrapper = GenericWrapper.withTransformer(Left::class, 0, { wrapper -> wrapper.left }) val rightWrapper = GenericWrapper.withTransformer(Right::class, 0, { wrapper -> wrapper.right }) + val kindFixtures = mapOf( + ResolverKind.METHOD to KindFixture(::NounMethodHolder, "LightMethodFieldResolverDataFetcher"), + ResolverKind.FIELD to KindFixture(::NounFieldHolder, "PropertyFieldResolverDataFetcher"), + ResolverKind.MAP_VALUE to KindFixture({ noun -> NounMap().apply { put("value", Box(noun)) } }, "MapFieldResolverDataFetcher") + ) + + val extraParameterFixtures = mapOf( + ExtraParameter.ENVIRONMENT to (object : GraphQLQueryResolver { + fun value(first: String, env: DataFetchingEnvironment) = "$first ${env.field.name}" + } to "x value"), + ExtraParameter.GRAPHQL_CONTEXT to (object : GraphQLQueryResolver { + fun value(first: String, context: GraphQLContext) = "$first ${context.get("key")}" + } to "x k"), + ExtraParameter.CONTEXT_CLASS to (object : GraphQLQueryResolver { + fun value(first: String, context: AppContext) = "$first ${context.user}" + } to "x ann") + ) + val shapes = listOf( Shape("future", "Foo", "{name=a}") { FutureMap().apply { put("future", it.future) } }, Shape("futureOfListOfFutures", "[Foo]", "[{name=a}, {name=b}]") { FutureOfListOfFuturesMap().apply { put("futureOfListOfFutures", it.futureOfListOfFutures) } }, diff --git a/src/test/kotlin/graphql/kickstart/tools/WrappedFetchingObjectTest.kt b/src/test/kotlin/graphql/kickstart/tools/WrappedFetchingObjectTest.kt new file mode 100644 index 00000000..1fc37cbb --- /dev/null +++ b/src/test/kotlin/graphql/kickstart/tools/WrappedFetchingObjectTest.kt @@ -0,0 +1,66 @@ +package graphql.kickstart.tools + +import graphql.GraphQL +import graphql.execution.instrumentation.Instrumentation +import graphql.execution.instrumentation.InstrumentationState +import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters +import graphql.schema.DataFetcher +import graphql.schema.DataFetchingEnvironment +import org.junit.Test + +class WrappedFetchingObjectTest { + + @Test + fun `an instrumentation in another package reaches the object behind a method's data fetcher by reflection`() { + val holder = Holder() + val query = Query(holder) + val holderResolver = HolderResolver() + val wrapped = mutableMapOf() + val instrumentation = object : Instrumentation { + override fun instrumentDataFetcher(dataFetcher: DataFetcher<*>, parameters: InstrumentationFieldFetchParameters, state: InstrumentationState?): DataFetcher<*> = + DataFetcher { environment -> + val fetchingObject = dataFetcher.javaClass.getMethod("getWrappedFetchingObject", DataFetchingEnvironment::class.java).invoke(dataFetcher, environment) + wrapped[environment.field.name] = "${dataFetcher.javaClass.name} -> ${identify(fetchingObject, query, holderResolver, holder)}" + dataFetcher.get(environment) + } + } + val schema = SchemaParser.newParser() + .schemaString("type Query { holder: Holder } type Holder { method: String withArgument(x: String): String resolved: String }") + .resolvers(query, holderResolver) + .build() + .makeExecutableSchema() + + val result = GraphQL.newGraphQL(schema).instrumentation(instrumentation).build().execute("{ holder { method withArgument(x: \"a\") resolved } }") + + assertEquals(result.errors, listOf()) + assertEquals( + wrapped, + mapOf( + "holder" to "graphql.kickstart.tools.resolver.LightMethodFieldResolverDataFetcher -> query resolver", + "method" to "graphql.kickstart.tools.resolver.LightMethodFieldResolverDataFetcher -> source object", + "withArgument" to "graphql.kickstart.tools.resolver.MethodFieldResolverDataFetcher -> source object", + "resolved" to "graphql.kickstart.tools.resolver.MethodFieldResolverDataFetcher -> holder resolver" + ) + ) + } + + private fun identify(fetchingObject: Any, query: Query, holderResolver: HolderResolver, holder: Holder) = when { + fetchingObject === query -> "query resolver" + fetchingObject === holderResolver -> "holder resolver" + fetchingObject === holder -> "source object" + else -> "unexpected $fetchingObject" + } + + class Query(private val holder: Holder) : GraphQLQueryResolver { + fun holder() = holder + } + + class Holder { + fun method() = "method" + fun withArgument(x: String) = "argument $x" + } + + class HolderResolver : GraphQLResolver { + fun resolved(holder: Holder) = "resolved ${holder.method()}" + } +} diff --git a/src/test/kotlin/graphql/kickstart/tools/returnvalue/ReturnValueUnwrappingTest.kt b/src/test/kotlin/graphql/kickstart/tools/returnvalue/ReturnValueUnwrappingTest.kt index 6e3da8c0..b2620193 100644 --- a/src/test/kotlin/graphql/kickstart/tools/returnvalue/ReturnValueUnwrappingTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/returnvalue/ReturnValueUnwrappingTest.kt @@ -1,5 +1,6 @@ package graphql.kickstart.tools.returnvalue +import graphql.kickstart.tools.resolver.ResolverKind import graphql.kickstart.tools.resolver.TypeShape import org.junit.Assert.fail import org.junit.Test @@ -24,7 +25,8 @@ class ReturnValueUnwrappingTest { val args = System.getProperty("unwrapping.args", "").split(" ").filter { it.isNotBlank() } + listOf( "--known", copy("known-findings.txt", dir).toString(), - "--container-kinds", TypeShape.containerTypes.joinToString(",") { it.canonicalName } + "--container-kinds", TypeShape.containerTypes.joinToString(",") { it.canonicalName }, + "--resolver-kinds", ResolverKind.entries.joinToString(",") { it.noun } ) val report = ByteArrayOutputStream() val passed = URLClassLoader(arrayOf(dir.toUri().toURL()), javaClass.classLoader).use { loader -> diff --git a/src/test/resources/returnvalue/Check.java b/src/test/resources/returnvalue/Check.java index 815dce54..ee8fa9ea 100644 --- a/src/test/resources/returnvalue/Check.java +++ b/src/test/resources/returnvalue/Check.java @@ -338,6 +338,17 @@ public static boolean check(String[] args, String classpath, PrintStream out) th return false; } + Set exercised = Arrays.stream(Kind.values()).map(it -> it.resolverKind).collect(Collectors.toSet()); + List unexercised = Arrays.stream(stringArg(args, "--resolver-kinds").split(",")) + .filter(it -> !exercised.contains(it)) + .collect(Collectors.toList()); + if (!unexercised.isEmpty()) { + out.println("MODEL ERROR: ResolverPlan.kt has a ResolverKind with these nouns, but no Kind in Check.java resolves through them, so a pass would prove nothing about them:"); + unexercised.forEach(it -> out.println(" " + it)); + out.println("Add a Kind for each to Check.java with its holdCompletionStage text, and build its resolver in the switches over Kind."); + return false; + } + List cases = cases(depth); Map> resolvers = compile(cases, classpath); From 6cb39afe079f49bece693427eb97090388b61e6c Mon Sep 17 00:00:00 2001 From: Oryan Date: Thu, 5 Nov 2026 22:32:18 -0500 Subject: [PATCH 06/10] Scope the resolver ownership rules to exact classes and close their reflection gaps Exemptions now name the top-level classes each owning file declares and count nested and lambda classes through their enclosing class, so neither a file of the same name elsewhere nor a '$' in a class name inherits one. The ScanDictionary exception covers only the type reads findInputValueTypeInType makes. The reflection rule also covers MethodHandles, kotlin-reflect calls and return types, Field writes and primitive reads, and commons-lang3 field and method utilities, and the TokenBuffer rule covers updateValue and readValueAs. Every library data fetcher must be a ResolverDataFetcher. --- .../kickstart/tools/ResolverOwnershipTest.kt | 134 ++++++++++++------ 1 file changed, 87 insertions(+), 47 deletions(-) diff --git a/src/test/kotlin/graphql/kickstart/tools/ResolverOwnershipTest.kt b/src/test/kotlin/graphql/kickstart/tools/ResolverOwnershipTest.kt index 6ceb40c3..dd2b5e5d 100644 --- a/src/test/kotlin/graphql/kickstart/tools/ResolverOwnershipTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/ResolverOwnershipTest.kt @@ -5,23 +5,25 @@ import com.tngtech.archunit.core.domain.JavaAccess import com.tngtech.archunit.core.domain.JavaClass import com.tngtech.archunit.core.domain.JavaClasses import com.tngtech.archunit.core.importer.ClassFileImporter -import com.tngtech.archunit.lang.ArchCondition -import com.tngtech.archunit.lang.ConditionEvents -import com.tngtech.archunit.lang.SimpleConditionEvent import com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes import com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses import graphql.execution.DataFetcherResult import graphql.kickstart.tools.SchemaParserOptions.GenericWrapper import graphql.kickstart.tools.resolver.ArgumentConverter +import graphql.kickstart.tools.resolver.ResolverDataFetcher import graphql.kickstart.tools.resolver.ResolverPlan import graphql.kickstart.tools.resolver.TypeShape import graphql.schema.DataFetcher import org.apache.commons.lang3.reflect.FieldUtils +import org.apache.commons.lang3.reflect.MethodUtils import org.junit.Test +import tools.jackson.core.JsonParser import tools.jackson.databind.ObjectMapper import tools.jackson.databind.ObjectReader import tools.jackson.databind.ObjectWriter import tools.jackson.databind.util.TokenBuffer +import java.lang.invoke.MethodHandle +import java.lang.invoke.MethodHandles import java.lang.reflect.AccessibleObject import java.lang.reflect.Executable import java.lang.reflect.Field @@ -35,41 +37,77 @@ class ResolverOwnershipTest { .importPath(Paths.get(ResolverPlan::class.java.protectionDomain.codeSource.location.toURI())) private val resolverPackage = ResolverPlan::class.java.packageName - private const val MEMBER = "graphql.kickstart.tools.resolver.Member" - } + private val MEMBER = "$resolverPackage.Member" - private fun declaredIn(fileName: String) = object : DescribedPredicate("are declared in $resolverPackage's $fileName") { - override fun test(javaClass: JavaClass) = - javaClass.packageName == resolverPackage && javaClass.source.flatMap { it.fileName }.orElse(null) == fileName + private val resolverPlanFile = OwningFile( + "ResolverPlan.kt", "ResolverPlan", "ResolverPlanKt", "ResolverKind", "ExtraParameter", "FieldResolverError", "Member", "MethodMember", + "PropertyMember", "MapMember", "ResolverDataFetcher", "MethodFieldResolverDataFetcher", "LightMethodFieldResolverDataFetcher", + "PropertyFieldResolverDataFetcher", "MapFieldResolverDataFetcher" + ) + private val typeShapeFile = OwningFile( + "TypeShape.kt", "TypeShape", "TypeShapeKt", "Position", "Walk", "GenericWrappers", "CompareGenericWrappers", "Layer", "ContainerKind", "Step", + "Await", "Peel", "EachElement", "DataOf", "ValueOf", "Awaitable" + ) + private val argumentConverterFile = OwningFile("ArgumentConverter.kt", "ArgumentConverter", "ArgumentConverterKt") + private val owningFiles = listOf(resolverPlanFile, typeShapeFile, argumentConverterFile) } - private fun DescribedPredicate.orOwnerOf(vararg allowed: Class<*>): DescribedPredicate { - val names = allowed.map { it.name } - return object : DescribedPredicate("${this@orOwnerOf.description} or are ${names.joinToString()}") { - override fun test(javaClass: JavaClass) = this@orOwnerOf.test(javaClass) || javaClass.name.substringBefore('$') in names - } + // A nested, anonymous or lambda class belongs to the class that encloses it. Neither the file name a class was compiled from + // nor a '$' in its name makes it one of these, since any file can declare either + private class OwningFile(val fileName: String, vararg topLevelClasses: String) : + DescribedPredicate("are declared in $fileName") { + val names = topLevelClasses.map { "${ResolverPlan::class.java.packageName}.$it" }.toSet() + + override fun test(javaClass: JavaClass) = generateSequence(javaClass) { it.enclosingClass.orElse(null) }.last().name in names } + private fun not(predicate: DescribedPredicate) = DescribedPredicate.not(predicate) + + private fun DescribedPredicate.or(other: DescribedPredicate) = DescribedPredicate.or(this, other) + private class Calls(description: String, private val targets: Map>) : DescribedPredicate>(description) { override fun test(access: JavaAccess<*>) = targets.any { (owner, names) -> access.target.owner.isAssignableTo(owner) && access.target.name in names } } private val memberReflection = Calls( - "reads, invokes or opens a Java member by reflection", + "reads, invokes or opens a Java or Kotlin member by reflection", mapOf( Method::class.java.name to setOf("invoke"), - Field::class.java.name to setOf("get", "getGenericType"), + Field::class.java.name to setOf( + "get", "getBoolean", "getByte", "getChar", "getShort", "getInt", "getLong", "getFloat", "getDouble", "getGenericType", + "set", "setBoolean", "setByte", "setChar", "setShort", "setInt", "setLong", "setFloat", "setDouble" + ), Executable::class.java.name to setOf("getGenericParameterTypes", "getGenericReturnType", "getParameterTypes", "getParameterCount"), AccessibleObject::class.java.name to setOf("setAccessible", "trySetAccessible"), - "kotlin.reflect.jvm.ReflectJvmMapping" to setOf("getKotlinFunction"), - FieldUtils::class.java.name to setOf("getAllFields", "getAllFieldsList"), + MethodHandles::class.java.name to setOf("privateLookupIn"), + MethodHandles.Lookup::class.java.name to setOf( + "unreflect", "unreflectGetter", "unreflectSetter", "unreflectSpecial", "unreflectVarHandle", "findVirtual", "findStatic", "findSpecial", + "findGetter", "findSetter", "findStaticGetter", "findStaticSetter", "findVarHandle", "findStaticVarHandle" + ), + MethodHandle::class.java.name to setOf("invoke", "invokeExact", "invokeWithArguments"), + "kotlin.reflect.KCallable" to setOf("call", "callBy", "getReturnType"), + "kotlin.reflect.KProperty" to setOf("get", "set", "invoke", "getDelegate"), + "kotlin.reflect.jvm.ReflectJvmMapping" to setOf( + "getKotlinFunction", "getKotlinProperty", "getJavaType", "getJavaMethod", "getJavaField", "getJavaGetter", "getJavaSetter" + ), + FieldUtils::class.java.name to setOf( + "getAllFields", "getAllFieldsList", "getField", "getDeclaredField", "readField", "readDeclaredField", "readStaticField", + "readDeclaredStaticField", "writeField", "writeDeclaredField", "writeStaticField", "writeDeclaredStaticField" + ), + MethodUtils::class.java.name to setOf("invokeMethod", "invokeExactMethod", "invokeStaticMethod", "invokeExactStaticMethod"), ) ) - private val guessesInputFieldTypes = object : DescribedPredicate>("is in ScanDictionary.findInputValueTypeInType") { + // an input object's field types come from its class's getters and fields, which ScanDictionary finds before any ResolverPlan exists + private val readsInputFieldTypes = object : DescribedPredicate>("is ScanDictionary.findInputValueTypeInType reading a member's type") { + private val reads = Calls( + "reads a member's declared type", + mapOf(Executable::class.java.name to setOf("getGenericReturnType", "getParameterCount"), Field::class.java.name to setOf("getGenericType")) + ) + override fun test(access: JavaAccess<*>) = - access.origin.owner.isEquivalentTo(ScanDictionary::class.java) && access.origin.name.startsWith("findInputValueTypeInType") + access.origin.owner.isEquivalentTo(ScanDictionary::class.java) && access.origin.name == "findInputValueTypeInType" && reads.test(access) } private val tokenBufferConversion = Calls( @@ -77,8 +115,9 @@ class ResolverOwnershipTest { mapOf( TokenBuffer::class.java.name to setOf("", "forInputParsing", "forGeneration", "asParser"), ObjectWriter::class.java.name to setOf("writeValueIntoBuffer"), - ObjectMapper::class.java.name to setOf("readValue", "convertValue", "treeToValue", "valueToTree"), + ObjectMapper::class.java.name to setOf("readValue", "convertValue", "updateValue", "treeToValue", "valueToTree"), ObjectReader::class.java.name to setOf("readValue", "readValues", "treeToValue"), + JsonParser::class.java.name to setOf("readValueAs", "readValueAsTree"), ) ) @@ -92,55 +131,56 @@ class ResolverOwnershipTest { mapOf(GenericWrapper::class.java.name to setOf("getType", "getIndex", "getSchemaWrapper", "getTransformer", "getTransformerReturnsCompletionStage")) ) - private val buildsAnUnwrapper = object : DescribedPredicate>("builds a TypeShape.Unwrapper") { - override fun test(access: JavaAccess<*>) = - access.target.owner.isEquivalentTo(TypeShape.Unwrapper::class.java) && access.target.name == "" - } - - private val holdsAnUnwrapper =object : ArchCondition("hold a TypeShape.Unwrapper") { - override fun check(javaClass: JavaClass, events: ConditionEvents) { - if (javaClass.allFields.none { it.rawType.isEquivalentTo(TypeShape.Unwrapper::class.java) }) { - events.add(SimpleConditionEvent.violated(javaClass, "${javaClass.name} implements DataFetcher without a TypeShape.Unwrapper field")) - } - } - } + private val buildsAnUnwrapper = Calls("builds a TypeShape.Unwrapper", mapOf(TypeShape.Unwrapper::class.java.name to setOf(""))) @Test - fun `imports the compiled resolver classes`() { - assert(classes.contain(MEMBER)) { "expected $MEMBER, the member variants' base class in ResolverPlan.kt, among ${classes.size} classes" } - assert(classes.any { declaredIn("ResolverPlan.kt").test(it) }) { "no class compiled from ResolverPlan.kt was imported" } + fun `each owning file lists exactly the top-level classes it declares`() { + val notCompiled = owningFiles.flatMap { file -> file.names.filter { !classes.contain(it) } } + assert(notCompiled.isEmpty()) { "ResolverOwnershipTest lists classes that weren't compiled: $notCompiled. Remove or rename them in its OwningFile lists" } + + val unlisted = owningFiles.flatMap { file -> + classes.filter { it.packageName == resolverPackage && !it.enclosingClass.isPresent && it.source.flatMap { source -> source.fileName }.orElse(null) == file.fileName } + .map { it.name } + .filter { it !in file.names } + } + assert(unlisted.isEmpty()) { + "These classes were compiled from a file named like an owning file but aren't in its OwningFile list in ResolverOwnershipTest: $unlisted. " + + "Add a class declared in that file to its list. Rename any other file, since only one file in $resolverPackage owns each name" + } + assert(classes.contain(MEMBER)) { "expected $MEMBER, the member variants' base class in ResolverPlan.kt" } } @Test fun `only the resolver plan reflects on resolver members`() { - noClasses().that(DescribedPredicate.not(declaredIn("ResolverPlan.kt"))) - .should().accessTargetWhere(DescribedPredicate.and(memberReflection, DescribedPredicate.not(guessesInputFieldTypes))) + noClasses().that(not(resolverPlanFile)) + .should().accessTargetWhere(DescribedPredicate.and(memberReflection, DescribedPredicate.not(readsInputFieldTypes))) .because("ResolverPlan.kt reads each field's member once, at schema build, and is the only place that invokes it. " + "Read what you need from the ResolverPlan, such as its argument targets or type shapes, or add it there") .check(classes) } @Test - fun `member variants expose raw invocation, never a data fetcher`() { + fun `member variants never build a data fetcher`() { noClasses().that().areAssignableTo(MEMBER) .should().dependOnClassesThat().areAssignableTo(DataFetcher::class.java) .orShould().dependOnClassesThat().areAssignableTo(TypeShape.Unwrapper::class.java) - .because("ResolverPlan builds every data fetcher around a member's read and unwraps its value with TypeShape.Unwrapper, " + - "so a member that builds its own fetcher skips the unwrapping. Give the member a read and let ResolverPlan.dataFetcher wrap it") + .because("ResolverPlan builds every data fetcher for a member and unwraps its value with TypeShape.Unwrapper, " + + "so a member that builds its own fetcher skips the unwrapping. Give the member what its fetcher needs and let ResolverPlan.dataFetcher build it") .check(classes) } @Test - fun `every library data fetcher unwraps through TypeShape`() { + fun `every library data fetcher is a ResolverDataFetcher`() { classes().that().implement(DataFetcher::class.java) - .should(holdsAnUnwrapper) - .because("a resolver's value reaches graphql-java fully unwrapped only through TypeShape.Unwrapper. Add the member to ResolverPlan instead of a new fetcher") + .should().beAssignableTo(ResolverDataFetcher::class.java) + .because("ResolverDataFetcher's constructor takes the TypeShape.Unwrapper the scan matched, so a resolver's value reaches graphql-java " + + "fully unwrapped. Add the member to ResolverPlan and its fetcher as a ResolverDataFetcher in ResolverPlan.kt instead of a new fetcher") .check(classes) } @Test fun `only the resolver plan builds an unwrapper`() { - noClasses().that(DescribedPredicate.not(declaredIn("ResolverPlan.kt").orOwnerOf(TypeShape::class.java))) + noClasses().that(not(resolverPlanFile.or(typeShapeFile))) .should().accessTargetWhere(buildsAnUnwrapper) .because("ResolverPlan.dataFetcher builds one unwrapper per field from the type shape the scan matched") .check(classes) @@ -148,7 +188,7 @@ class ResolverOwnershipTest { @Test fun `only TypeShape dispatches on containers`() { - noClasses().that(DescribedPredicate.not(declaredIn("TypeShape.kt").orOwnerOf(GenericWrapper::class.java))) + noClasses().that(not(typeShapeFile)) .should().accessTargetWhere(genericWrapperDispatch) .orShould().dependOnClassesThat().areAssignableTo(DataFetcherResult::class.java) .because("TypeShape.kt walks a declared type once and unwraps values by the same layers, so a container handled anywhere else " + @@ -158,7 +198,7 @@ class ResolverOwnershipTest { @Test fun `only the argument converter converts through a TokenBuffer`() { - noClasses().that(DescribedPredicate.not(declaredIn("ArgumentConverter.kt"))) + noClasses().that(not(argumentConverterFile)) .should().accessTargetWhere(tokenBufferConversion) .orShould().dependOnClassesThat().areAssignableTo(TokenBuffer::class.java) .because("ArgumentConverter is the one entry point that turns a coerced argument into a parameter's type, with its fallbacks for scalars Jackson can't convert. Use the ResolverPlan's ArgumentConverter") @@ -167,7 +207,7 @@ class ResolverOwnershipTest { @Test fun `only the resolver plan converts arguments`() { - noClasses().that(DescribedPredicate.not(declaredIn("ResolverPlan.kt").orOwnerOf(ArgumentConverter::class.java))) + noClasses().that(not(resolverPlanFile.or(argumentConverterFile))) .should().accessTargetWhere(argumentConversion) .because("ResolverPlan builds one ArgumentConverter per argument at schema build, next to the argument's Optional and scalar handling. Read the plan's argument targets instead") .check(classes) From 1b3519349dc281866f511aa8e03da6152a54af14 Mon Sep 17 00:00:00 2001 From: Oryan Date: Thu, 5 Nov 2026 22:43:56 -0500 Subject: [PATCH 07/10] Trim the fix round's comments and check that each listed class comes from its owning file Comment pass and /simplify: drop comments the types and tests already say, rename the Map flag to unwrapsUntypedValues, move the ScanDictionary exemption's reason into its description so a failure prints it, use ArchUnit's own not, and, or and isTopLevelClass, and fail when an OwningFile lists a class compiled from another file. --- .../kickstart/tools/resolver/ResolverPlan.kt | 10 ++----- .../kickstart/tools/resolver/TypeShape.kt | 2 +- .../kickstart/tools/ResolverOwnershipTest.kt | 30 ++++++++++--------- 3 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt index 3d5ce430..7d59234c 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt @@ -361,14 +361,12 @@ internal class ResolverPlan private constructor( } } -internal enum class ResolverKind(val noun: String, val holdCompletionStage: String, val unwrapsObjectValues: Boolean = true) { +internal enum class ResolverKind(val noun: String, val holdCompletionStage: String, val unwrapsUntypedValues: Boolean = true) { METHOD("method", "Return a CompletionStage, such as a CompletableFuture, from the method or its generic wrapper transformer instead."), FIELD("field", "Assign a CompletionStage, such as a CompletableFuture, to the field, or return one from its generic wrapper transformer instead."), - - // an untyped Map declares no value type, so unwrapping its values would mean inspecting each one at runtime MAP_VALUE( "map value", "Put a CompletionStage, such as a CompletableFuture, in the map, or return one from its generic wrapper transformer instead.", - unwrapsObjectValues = false + unwrapsUntypedValues = false ) } @@ -543,7 +541,6 @@ private class MapMember(field: FieldDefinition, private val search: ResolverSear override fun toString() = "MapFieldResolver{key=$key}" } -// the only data fetchers a member gets, so every resolver value goes through the unwrapper the scan matched for its field internal sealed class ResolverDataFetcher(protected val source: SourceResolver, protected val unwrapper: TypeShape.Unwrapper) : DataFetcher internal class MethodFieldResolverDataFetcher( @@ -581,9 +578,6 @@ internal class MethodFieldResolverDataFetcher( fun getWrappedFetchingObject(environment: DataFetchingEnvironment): Any = source.resolve(environment, null) } -/** - * [MethodFieldResolverDataFetcher] for a method without parameters, which graphql-java calls without building an environment. - */ internal class LightMethodFieldResolverDataFetcher( source: SourceResolver, private val method: Method, diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt index de203674..57c4a239 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt @@ -49,7 +49,7 @@ internal class TypeShape private constructor( fun error(msg: String) = matchError(graphQLType, reference, javaType, msg) class Unwrapper(shape: TypeShape, resolverKind: ResolverKind, resolverName: String, isSuspendFunction: Boolean = false) { - private val returnsValueAsIs = !resolverKind.unwrapsObjectValues && shape.declaredType == Any::class.java + private val returnsValueAsIs = !resolverKind.unwrapsUntypedValues && shape.declaredType == Any::class.java private val wrappers = shape.wrappers private val awaitable: Awaitable private val isOutermostFuture: Boolean diff --git a/src/test/kotlin/graphql/kickstart/tools/ResolverOwnershipTest.kt b/src/test/kotlin/graphql/kickstart/tools/ResolverOwnershipTest.kt index dd2b5e5d..cd731817 100644 --- a/src/test/kotlin/graphql/kickstart/tools/ResolverOwnershipTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/ResolverOwnershipTest.kt @@ -1,6 +1,8 @@ package graphql.kickstart.tools import com.tngtech.archunit.base.DescribedPredicate +import com.tngtech.archunit.base.DescribedPredicate.and +import com.tngtech.archunit.base.DescribedPredicate.not import com.tngtech.archunit.core.domain.JavaAccess import com.tngtech.archunit.core.domain.JavaClass import com.tngtech.archunit.core.domain.JavaClasses @@ -52,19 +54,13 @@ class ResolverOwnershipTest { private val owningFiles = listOf(resolverPlanFile, typeShapeFile, argumentConverterFile) } - // A nested, anonymous or lambda class belongs to the class that encloses it. Neither the file name a class was compiled from - // nor a '$' in its name makes it one of these, since any file can declare either private class OwningFile(val fileName: String, vararg topLevelClasses: String) : DescribedPredicate("are declared in $fileName") { val names = topLevelClasses.map { "${ResolverPlan::class.java.packageName}.$it" }.toSet() - override fun test(javaClass: JavaClass) = generateSequence(javaClass) { it.enclosingClass.orElse(null) }.last().name in names + override fun test(javaClass: JavaClass) = javaClass.topLevelClass().name in names } - private fun not(predicate: DescribedPredicate) = DescribedPredicate.not(predicate) - - private fun DescribedPredicate.or(other: DescribedPredicate) = DescribedPredicate.or(this, other) - private class Calls(description: String, private val targets: Map>) : DescribedPredicate>(description) { override fun test(access: JavaAccess<*>) = targets.any { (owner, names) -> access.target.owner.isAssignableTo(owner) && access.target.name in names } @@ -99,8 +95,9 @@ class ResolverOwnershipTest { ) ) - // an input object's field types come from its class's getters and fields, which ScanDictionary finds before any ResolverPlan exists - private val readsInputFieldTypes = object : DescribedPredicate>("is ScanDictionary.findInputValueTypeInType reading a member's type") { + private val readsInputFieldTypes = object : DescribedPredicate>( + "is ScanDictionary.findInputValueTypeInType reading an input object's getter or field type, which it needs before any ResolverPlan exists" + ) { private val reads = Calls( "reads a member's declared type", mapOf(Executable::class.java.name to setOf("getGenericReturnType", "getParameterCount"), Field::class.java.name to setOf("getGenericType")) @@ -135,11 +132,13 @@ class ResolverOwnershipTest { @Test fun `each owning file lists exactly the top-level classes it declares`() { - val notCompiled = owningFiles.flatMap { file -> file.names.filter { !classes.contain(it) } } - assert(notCompiled.isEmpty()) { "ResolverOwnershipTest lists classes that weren't compiled: $notCompiled. Remove or rename them in its OwningFile lists" } + val misplaced = owningFiles.flatMap { file -> file.names.filter { !classes.contain(it) || classes.get(it).sourceFileName() != file.fileName } } + assert(misplaced.isEmpty()) { + "ResolverOwnershipTest lists classes that weren't compiled from their OwningFile: $misplaced. Move, remove or rename them in its OwningFile lists" + } val unlisted = owningFiles.flatMap { file -> - classes.filter { it.packageName == resolverPackage && !it.enclosingClass.isPresent && it.source.flatMap { source -> source.fileName }.orElse(null) == file.fileName } + classes.filter { it.packageName == resolverPackage && it.isTopLevelClass && it.sourceFileName() == file.fileName } .map { it.name } .filter { it !in file.names } } @@ -147,13 +146,12 @@ class ResolverOwnershipTest { "These classes were compiled from a file named like an owning file but aren't in its OwningFile list in ResolverOwnershipTest: $unlisted. " + "Add a class declared in that file to its list. Rename any other file, since only one file in $resolverPackage owns each name" } - assert(classes.contain(MEMBER)) { "expected $MEMBER, the member variants' base class in ResolverPlan.kt" } } @Test fun `only the resolver plan reflects on resolver members`() { noClasses().that(not(resolverPlanFile)) - .should().accessTargetWhere(DescribedPredicate.and(memberReflection, DescribedPredicate.not(readsInputFieldTypes))) + .should().accessTargetWhere(and(memberReflection, not(readsInputFieldTypes))) .because("ResolverPlan.kt reads each field's member once, at schema build, and is the only place that invokes it. " + "Read what you need from the ResolverPlan, such as its argument targets or type shapes, or add it there") .check(classes) @@ -213,3 +211,7 @@ class ResolverOwnershipTest { .check(classes) } } + +private fun JavaClass.topLevelClass(): JavaClass = generateSequence(this) { it.enclosingClass.orElse(null) }.last() + +private fun JavaClass.sourceFileName(): String? = source.flatMap { it.fileName }.orElse(null) From 0f2352571f4cedb4d392d418ae0073a57e9e0918 Mon Sep 17 00:00:00 2001 From: Oryan Date: Thu, 5 Nov 2026 22:43:58 -0500 Subject: [PATCH 08/10] Describe the restored data fetchers in the rule table and the bounded check skill --- .../verify-return-value-unwrapping/SKILL.md | 24 ++++++++++--------- AGENTS.md | 4 ++-- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/.claude/skills/verify-return-value-unwrapping/SKILL.md b/.claude/skills/verify-return-value-unwrapping/SKILL.md index 0a35bd1c..73bbd90e 100644 --- a/.claude/skills/verify-return-value-unwrapping/SKILL.md +++ b/.claude/skills/verify-return-value-unwrapping/SKILL.md @@ -38,11 +38,13 @@ and an all-non-null GraphQL type. Each return type runs as three kinds of resolver, each compiled as a Java class at runtime: -| Kind | Generated resolver | Member in `ResolverPlan.kt` | +| Kind | Generated resolver | Member and data fetcher in `ResolverPlan.kt` | |---|---|---| -| `method` | `public field()` on the query resolver | `MethodMember` | -| `property` | `public field;` on the query resolver, which is also what a Kotlin `@JvmField` compiles to | `PropertyMember` | -| `map` | `extends HashMap>`, bound to `type Holder` and returned by `Query.holder` | `MapMember` | +| `method` | `public field()` on the query resolver | `MethodMember`, `LightMethodFieldResolverDataFetcher` | +| `property` | `public field;` on the query resolver, which is also what a Kotlin `@JvmField` compiles to | `PropertyMember`, `PropertyFieldResolverDataFetcher` | +| `map` | `extends HashMap>`, bound to `type Holder` and returned by `Query.holder` | `MapMember`, `MapFieldResolverDataFetcher` | + +Each kind's `resolverKind` is the noun of a `ResolverKind`, and a `ResolverKind` without a kind here ends in `MODEL ERROR`. The check builds each schema through the public `SchemaParser` API, which rejects about a fifth of the combinations, such as `Optional` below the top level. For every accepted one it executes the query once for each value that makes at most @@ -73,7 +75,7 @@ as `-Dunwrapping.args`, and prints the report. On a failure it also prints the p |---|---| | 0 | `PASS`, or `PASS apart from the known findings above` | | 1 | `FAIL`: a return type that isn't a known finding breaks the property. `NEW`: a known finding matches more accepted return types than its line records | -| 2 | `MODEL ERROR`: the checkout or the check didn't compile, the build rejected a return type the check must exercise, or a `ContainerKind` in `TypeShape.kt` has no `Container` in `Check.java`, so a pass would prove nothing | +| 2 | `MODEL ERROR`: the checkout or the check didn't compile, the build rejected a return type the check must exercise, a `ContainerKind` in `TypeShape.kt` has no `Container` in `Check.java`, or a `ResolverKind` in `ResolverPlan.kt` has no `Kind`, so a pass would prove nothing | | 3 | `STALE`: a known finding matches a return type that no longer fails, or fewer accepted return types than its line records. Or the run's coverage differs from `pinnedCoverage` in `Check.java` | Options: `--depth N`, `--budget N`, `--property-budget N`, `--show N` (counterexamples and stale return types printed, default 15), and `--rejected`, which lists @@ -154,8 +156,8 @@ or delete it when none fail, so that a regression shows up as `FAIL`. ## What this does not cover - Suspend functions, subscriptions and `Publisher`, arrays, and Kotlin-declared resolver methods. The generated resolvers - are Java methods without parameters, so only `LightFetcher` in `ResolverPlan.kt` runs, not - `Fetcher`. + are Java methods without parameters, so only `LightMethodFieldResolverDataFetcher` in `ResolverPlan.kt` runs, not + `MethodFieldResolverDataFetcher`. - Wrappers picked by a subclass of the value (`Box` and `SpecialBox` in `NestedGenericWrapperTest`), wrappers with a type argument index other than 0, custom `schemaWrapper`s other than the list one, transformers that return a `FutureTask`, and DataLoader futures. @@ -169,22 +171,22 @@ or delete it when none fail, so that a regression shows up as `FAIL`. ## Coverage and sync -Last confirmed to describe the code at commit `948c1ca6` (2026-11-06). It covers: +Last confirmed to describe the code at commit `1b351934` (2026-11-05). It covers: | File | What | |---|---| | `src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt` | the walk, `ContainerKind`, `GenericWrappers`, `resolveTypeVariables`, the `Unwrapper` and its steps | -| `src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt` | `dataFetcher` (the `TypeShape.Unwrapper` per kind), `returnShape`, the members' `declaredValueType` and `read`, `Fetcher` and `LightFetcher` | +| `src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt` | `dataFetcher` (the `TypeShape.Unwrapper` per member), `returnShape`, `ResolverKind`, the members' `declaredValueType`, and the `ResolverDataFetcher` subclasses | | `src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt` | `GenericWrapper` and the default wrappers in `build()` | To see whether a change needs another look: ```bash -git log --oneline 948c1ca6..HEAD -- src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt +git log --oneline 1b351934..HEAD -- src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt src/main/kotlin/graphql/kickstart/tools/resolver/ResolverPlan.kt src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt ``` Move the sync commit forward only after you've read those changes and confirmed that the containers, variants and expected values in `Check.java` still describe them. For example, a new default wrapper or a new kind of unwrapping step needs a new `Container` with its variants and its plain value in `build`. A new `ContainerKind` ends in -`MODEL ERROR` until it has one. After an intended change, rerun the check, +`MODEL ERROR` until it has one, and so does a new `ResolverKind` until `Check.java` has a `Kind` with its noun. After an intended change, rerun the check, and break the new code on purpose once in a scratch worktree to see the check fail before you trust its pass. diff --git a/AGENTS.md b/AGENTS.md index cd466de8..7d5d9d44 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -94,13 +94,13 @@ happening. "docs only" means nothing does yet, so the rule depends on you readin | Rule | Enforced by | |---|---| | The value of a resolver method, a field with no matching getter or a typed Map is unwrapped by one rule that the schema build and the data fetcher share | `TypeShape`, whose one walk yields both the class the schema build matches and the layers that `TypeShape.Unwrapper` unwraps in the method, field and Map data fetchers. An untyped Map's values reach graphql-java as they are. A container kind can't be added without its placement rule and its value step, since both are abstract on the enum, and `ReturnValueUnwrappingTest` fails until the bounded check builds values for it | -| A resolver method's parameter layout, type variables and source are decided once, at schema build | `ResolverPlan`, which reads the field's member once and hands the scan its type shapes, the data fetcher its parameters and `UnmappedInputFieldChecker` its argument targets. Its member variants are private to `ResolverPlan.kt` and expose a raw read, never a data fetcher. A new `ResolverKind` doesn't compile until the lookup's `when` handles it, a new `ExtraParameter` doesn't compile without its binder, and the fixture tests in `ResolverCompatibilityTest` don't compile until they build a sample of each. `ResolverOwnershipTest` fails on member reflection outside `ResolverPlan.kt` (apart from the input-field getters in `ScanDictionary.findInputValueTypeInType`), on a member that depends on a data fetcher, on a data fetcher without a `TypeShape.Unwrapper`, and on generic-wrapper or `DataFetcherResult` handling outside `TypeShape.kt` | +| A resolver method's parameter layout, type variables and source are decided once, at schema build | `ResolverPlan`, which reads the field's member once and hands the scan its type shapes, the data fetcher its parameters and `UnmappedInputFieldChecker` its argument targets. Its member variants are private to `ResolverPlan.kt`, each decides its `ResolverKind`, and none builds a data fetcher. `ResolverPlan.dataFetcher` gives every member a `ResolverDataFetcher`, a sealed class whose constructor takes the `TypeShape.Unwrapper`, so a lambda or a fetcher without one doesn't compile. A new `ResolverKind` doesn't compile until the lookup's `when` handles it, a new `ExtraParameter` doesn't compile without its binder, the fixture tests in `ResolverCompatibilityTest` fail, naming the fixture to add, until they build a sample of each and see its value unwrapped, and `ReturnValueUnwrappingTest` ends in `MODEL ERROR` until the bounded check runs the new kind. `ResolverOwnershipTest` fails on member reflection outside `ResolverPlan.kt` (`java.lang.reflect` reads and calls, `MethodHandles`, kotlin-reflect calls and types, and commons-lang3's field and method utilities, apart from the type reads in `ScanDictionary.findInputValueTypeInType`), on a member that depends on a data fetcher, on a library data fetcher that isn't a `ResolverDataFetcher`, and on generic-wrapper or `DataFetcherResult` handling outside `TypeShape.kt`. Its exemptions name the exact classes each owning file declares. `Future`, `CompletionStage` and `Optional` checks outside `TypeShape.kt`, `Class.forName` and graphql-java's `PropertyDataFetcher` get past it, and the behavior tests catch the value they leave wrapped | | A change to how resolvers are found, bound or unwrapped keeps what a method, a property and a Map field return | `ResolverCompatibilityTest`, which compares field and Map values with a method's, and the bounded check in `ReturnValueUnwrappingTest`, which compares all three with the plain value. Neither has known findings, so a value that differs on a return type they run fails the build. The bounded check also pins how many return types it generates, and for each kind how many the build rejects and how many executions run, so a change that stops running some fails until the same diff updates `pinnedCoverage`. It doesn't judge whether a newly rejected return type should build | | One output has one builder. Don't add a second path that builds the same thing | docs only | | Read an SDL type's members merged with its extensions, never from the raw definition | `MergedType` in `MergedDefinitions.kt` and `DefinitionsBoundaryTest`, which fails on a raw member, directive or child-node read, an extension class, an SDL node walk or a `TypeDefinitionRegistry` call outside it. A read by reflection gets past it. `TypeExtensionTest` builds and queries a schema with an extension of every kind, so a build that leaves an extension's fields, interfaces, members, values, input fields or directives out of the built type fails it, however the code read them | | Every kind of SDL type starts its graphql-java type from the same recipe | `MergedType.prepare`, `DefinitionsBoundaryTest` and `TypeKindRecipeTest` | | Reuse an existing structure before you add a parallel one | docs only | -| Argument conversion goes through one entry point | `ArgumentConverter`, which `ResolverPlan` builds once per argument. `ResolverOwnershipTest` fails on a TokenBuffer or a Jackson `readValue`, `convertValue`, `treeToValue` or `valueToTree` outside `ArgumentConverter.kt`, and on an `ArgumentConverter` built or called outside `ResolverPlan.kt`. Jackson's per-call retry for scalars it can't convert stays inside `ArgumentConverter` | +| Argument conversion goes through one entry point | `ArgumentConverter`, which `ResolverPlan` builds once per argument. `ResolverOwnershipTest` fails on a TokenBuffer or a Jackson `readValue`, `readValueAs`, `convertValue`, `updateValue`, `treeToValue` or `valueToTree` outside `ArgumentConverter.kt`, and on an `ArgumentConverter` built or called outside `ResolverPlan.kt`. Jackson's per-call retry for scalars it can't convert stays inside `ArgumentConverter` | | An input field's Java type comes from the mapping Jackson uses, not from guessed getters | docs only | | Every recursive walk over the type graph handles cycles, and a test covers a cycle | docs only | | Public signatures stay binary compatible, data class `copy()` and `componentN` included | japicmp with `build/japicmp-kotlin-public.groovy` at `mvn verify`, which allows excludes and other narrowing options only at an X.0.0 version | From 7cd648525f5930130613cd0482d5f84d3393ad67 Mon Sep 17 00:00:00 2001 From: Oryan Date: Thu, 5 Nov 2026 23:11:23 -0500 Subject: [PATCH 09/10] Catch Kotlin reflection, signature reads and fetchers outside ResolverPlan.kt in the ownership test Any use of a KFunction or KProperty, kotlin-reflect's member lookups and JVM mappings, and the Field, Parameter and annotated type reads now fail outside ResolverPlan.kt. A ResolverDataFetcher subclass must be declared in ResolverPlan.kt, and the ScanDictionary exemption matches findInputValueTypeInType(String, Class) by signature, so an overload doesn't inherit it. AGENTS.md lists what the test still doesn't see. --- AGENTS.md | 2 +- .../kickstart/tools/ResolverOwnershipTest.kt | 37 +++++++++++++------ 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 7d5d9d44..2b41c5bf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -94,7 +94,7 @@ happening. "docs only" means nothing does yet, so the rule depends on you readin | Rule | Enforced by | |---|---| | The value of a resolver method, a field with no matching getter or a typed Map is unwrapped by one rule that the schema build and the data fetcher share | `TypeShape`, whose one walk yields both the class the schema build matches and the layers that `TypeShape.Unwrapper` unwraps in the method, field and Map data fetchers. An untyped Map's values reach graphql-java as they are. A container kind can't be added without its placement rule and its value step, since both are abstract on the enum, and `ReturnValueUnwrappingTest` fails until the bounded check builds values for it | -| A resolver method's parameter layout, type variables and source are decided once, at schema build | `ResolverPlan`, which reads the field's member once and hands the scan its type shapes, the data fetcher its parameters and `UnmappedInputFieldChecker` its argument targets. Its member variants are private to `ResolverPlan.kt`, each decides its `ResolverKind`, and none builds a data fetcher. `ResolverPlan.dataFetcher` gives every member a `ResolverDataFetcher`, a sealed class whose constructor takes the `TypeShape.Unwrapper`, so a lambda or a fetcher without one doesn't compile. A new `ResolverKind` doesn't compile until the lookup's `when` handles it, a new `ExtraParameter` doesn't compile without its binder, the fixture tests in `ResolverCompatibilityTest` fail, naming the fixture to add, until they build a sample of each and see its value unwrapped, and `ReturnValueUnwrappingTest` ends in `MODEL ERROR` until the bounded check runs the new kind. `ResolverOwnershipTest` fails on member reflection outside `ResolverPlan.kt` (`java.lang.reflect` reads and calls, `MethodHandles`, kotlin-reflect calls and types, and commons-lang3's field and method utilities, apart from the type reads in `ScanDictionary.findInputValueTypeInType`), on a member that depends on a data fetcher, on a library data fetcher that isn't a `ResolverDataFetcher`, and on generic-wrapper or `DataFetcherResult` handling outside `TypeShape.kt`. Its exemptions name the exact classes each owning file declares. `Future`, `CompletionStage` and `Optional` checks outside `TypeShape.kt`, `Class.forName` and graphql-java's `PropertyDataFetcher` get past it, and the behavior tests catch the value they leave wrapped | +| A resolver method's parameter layout, type variables and source are decided once, at schema build | `ResolverPlan`, which reads the field's member once and hands the scan its type shapes, the data fetcher its parameters and `UnmappedInputFieldChecker` its argument targets. Its member variants are private to `ResolverPlan.kt`, each decides its `ResolverKind`, and none builds a data fetcher. `ResolverPlan.dataFetcher` gives every member a `ResolverDataFetcher`, a sealed class whose constructor takes the `TypeShape.Unwrapper`, so a lambda or a fetcher without one doesn't compile. A new `ResolverKind` doesn't compile until the lookup's `when` handles it, a new `ExtraParameter` doesn't compile without its binder, the fixture tests in `ResolverCompatibilityTest` fail, naming the fixture to add, until they build a sample of each and see its value unwrapped, and `ReturnValueUnwrappingTest` ends in `MODEL ERROR` until the bounded check runs the new kind. `ResolverOwnershipTest` fails on member reflection outside `ResolverPlan.kt` (`java.lang.reflect` calls and signature reads, `MethodHandles`, kotlin-reflect member lookups and any use of a `KFunction` or `KProperty`, and commons-lang3's field and method utilities, apart from the type reads in `ScanDictionary.findInputValueTypeInType(String, Class)`), on a member that depends on a data fetcher, on a library data fetcher that isn't a `ResolverDataFetcher` declared in `ResolverPlan.kt`, and on generic-wrapper or `DataFetcherResult` handling outside `TypeShape.kt`. Its exemptions name the exact classes each owning file declares. It doesn't see `Future`, `CompletionStage` or `Optional` checks outside `TypeShape.kt`, `Class.forName` lookups, `Method.getReturnType`, reflection a library does for you (graphql-java's `PropertyDataFetcher`, `java.beans.Expression`), a new helper in an owning file that reflects for another file, or a `DataFetcher { }` lambda outside `ResolverPlan.kt`. The fixture tests and the bounded check catch a value these leave wrapped only in the fetchers `ResolverPlan` builds | | A change to how resolvers are found, bound or unwrapped keeps what a method, a property and a Map field return | `ResolverCompatibilityTest`, which compares field and Map values with a method's, and the bounded check in `ReturnValueUnwrappingTest`, which compares all three with the plain value. Neither has known findings, so a value that differs on a return type they run fails the build. The bounded check also pins how many return types it generates, and for each kind how many the build rejects and how many executions run, so a change that stops running some fails until the same diff updates `pinnedCoverage`. It doesn't judge whether a newly rejected return type should build | | One output has one builder. Don't add a second path that builds the same thing | docs only | | Read an SDL type's members merged with its extensions, never from the raw definition | `MergedType` in `MergedDefinitions.kt` and `DefinitionsBoundaryTest`, which fails on a raw member, directive or child-node read, an extension class, an SDL node walk or a `TypeDefinitionRegistry` call outside it. A read by reflection gets past it. `TypeExtensionTest` builds and queries a schema with an extension of every kind, so a build that leaves an extension's fields, interfaces, members, values, input fields or directives out of the built type fails it, however the code read them | diff --git a/src/test/kotlin/graphql/kickstart/tools/ResolverOwnershipTest.kt b/src/test/kotlin/graphql/kickstart/tools/ResolverOwnershipTest.kt index cd731817..7343a92d 100644 --- a/src/test/kotlin/graphql/kickstart/tools/ResolverOwnershipTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/ResolverOwnershipTest.kt @@ -5,8 +5,12 @@ import com.tngtech.archunit.base.DescribedPredicate.and import com.tngtech.archunit.base.DescribedPredicate.not import com.tngtech.archunit.core.domain.JavaAccess import com.tngtech.archunit.core.domain.JavaClass +import com.tngtech.archunit.core.domain.JavaClass.Predicates.assignableTo +import com.tngtech.archunit.core.domain.JavaClass.Predicates.resideInAPackage +import com.tngtech.archunit.core.domain.JavaClass.Predicates.resideInAnyPackage import com.tngtech.archunit.core.domain.JavaClasses import com.tngtech.archunit.core.importer.ClassFileImporter +import com.tngtech.archunit.lang.conditions.ArchConditions.be import com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes import com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses import graphql.execution.DataFetcherResult @@ -30,6 +34,7 @@ import java.lang.reflect.AccessibleObject import java.lang.reflect.Executable import java.lang.reflect.Field import java.lang.reflect.Method +import java.lang.reflect.Parameter import java.nio.file.Paths class ResolverOwnershipTest { @@ -63,18 +68,27 @@ class ResolverOwnershipTest { private class Calls(description: String, private val targets: Map>) : DescribedPredicate>(description) { override fun test(access: JavaAccess<*>) = - targets.any { (owner, names) -> access.target.owner.isAssignableTo(owner) && access.target.name in names } + targets.any { (owner, names) -> access.target.name in names && access.target.owner.isAssignableTo(owner) } } + // KCallable only from kotlin.reflect itself, since Kotlin compiles a callable reference such as Type::name into a class implementing it + private val kotlinReflection = resideInAnyPackage("kotlin.reflect.full", "kotlin.reflect.jvm") + .or(resideInAPackage("kotlin.reflect").and(assignableTo("kotlin.reflect.KCallable"))) + .`as`("are kotlin-reflect's member lookups and JVM mappings, or a KFunction, a KProperty or their getters and setters") + private val memberReflection = Calls( "reads, invokes or opens a Java or Kotlin member by reflection", mapOf( Method::class.java.name to setOf("invoke"), Field::class.java.name to setOf( - "get", "getBoolean", "getByte", "getChar", "getShort", "getInt", "getLong", "getFloat", "getDouble", "getGenericType", - "set", "setBoolean", "setByte", "setChar", "setShort", "setInt", "setLong", "setFloat", "setDouble" + "get", "getBoolean", "getByte", "getChar", "getShort", "getInt", "getLong", "getFloat", "getDouble", "getType", "getGenericType", + "getAnnotatedType", "set", "setBoolean", "setByte", "setChar", "setShort", "setInt", "setLong", "setFloat", "setDouble" + ), + Executable::class.java.name to setOf( + "getGenericParameterTypes", "getGenericReturnType", "getParameterTypes", "getParameterCount", "getAnnotatedReturnType", + "getAnnotatedParameterTypes" ), - Executable::class.java.name to setOf("getGenericParameterTypes", "getGenericReturnType", "getParameterTypes", "getParameterCount"), + Parameter::class.java.name to setOf("getType", "getParameterizedType", "getAnnotatedType"), AccessibleObject::class.java.name to setOf("setAccessible", "trySetAccessible"), MethodHandles::class.java.name to setOf("privateLookupIn"), MethodHandles.Lookup::class.java.name to setOf( @@ -82,11 +96,7 @@ class ResolverOwnershipTest { "findGetter", "findSetter", "findStaticGetter", "findStaticSetter", "findVarHandle", "findStaticVarHandle" ), MethodHandle::class.java.name to setOf("invoke", "invokeExact", "invokeWithArguments"), - "kotlin.reflect.KCallable" to setOf("call", "callBy", "getReturnType"), - "kotlin.reflect.KProperty" to setOf("get", "set", "invoke", "getDelegate"), - "kotlin.reflect.jvm.ReflectJvmMapping" to setOf( - "getKotlinFunction", "getKotlinProperty", "getJavaType", "getJavaMethod", "getJavaField", "getJavaGetter", "getJavaSetter" - ), + "kotlin.reflect.KClass" to setOf("getMembers", "getConstructors"), FieldUtils::class.java.name to setOf( "getAllFields", "getAllFieldsList", "getField", "getDeclaredField", "readField", "readDeclaredField", "readStaticField", "readDeclaredStaticField", "writeField", "writeDeclaredField", "writeStaticField", "writeDeclaredStaticField" @@ -103,8 +113,9 @@ class ResolverOwnershipTest { mapOf(Executable::class.java.name to setOf("getGenericReturnType", "getParameterCount"), Field::class.java.name to setOf("getGenericType")) ) - override fun test(access: JavaAccess<*>) = - access.origin.owner.isEquivalentTo(ScanDictionary::class.java) && access.origin.name == "findInputValueTypeInType" && reads.test(access) + private val exempt = "${ScanDictionary::class.java.name}.findInputValueTypeInType(${String::class.java.name}, ${Class::class.java.name})" + + override fun test(access: JavaAccess<*>) = access.origin.fullName == exempt && reads.test(access) } private val tokenBufferConversion = Calls( @@ -152,6 +163,7 @@ class ResolverOwnershipTest { fun `only the resolver plan reflects on resolver members`() { noClasses().that(not(resolverPlanFile)) .should().accessTargetWhere(and(memberReflection, not(readsInputFieldTypes))) + .orShould().dependOnClassesThat(kotlinReflection) .because("ResolverPlan.kt reads each field's member once, at schema build, and is the only place that invokes it. " + "Read what you need from the ResolverPlan, such as its argument targets or type shapes, or add it there") .check(classes) @@ -168,9 +180,10 @@ class ResolverOwnershipTest { } @Test - fun `every library data fetcher is a ResolverDataFetcher`() { + fun `every library data fetcher is a ResolverDataFetcher in ResolverPlan kt`() { classes().that().implement(DataFetcher::class.java) .should().beAssignableTo(ResolverDataFetcher::class.java) + .andShould(be(resolverPlanFile.`as`("declared in ${resolverPlanFile.fileName}"))) .because("ResolverDataFetcher's constructor takes the TypeShape.Unwrapper the scan matched, so a resolver's value reaches graphql-java " + "fully unwrapped. Add the member to ResolverPlan and its fetcher as a ResolverDataFetcher in ResolverPlan.kt instead of a new fetcher") .check(classes) From e73845147640669f2af45a093b6fbb32a93143a9 Mon Sep 17 00:00:00 2001 From: Oryan Date: Fri, 6 Nov 2026 00:43:08 -0500 Subject: [PATCH 10/10] Say which KFunction uses the ownership test catches --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 2b41c5bf..f9b640e8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -94,7 +94,7 @@ happening. "docs only" means nothing does yet, so the rule depends on you readin | Rule | Enforced by | |---|---| | The value of a resolver method, a field with no matching getter or a typed Map is unwrapped by one rule that the schema build and the data fetcher share | `TypeShape`, whose one walk yields both the class the schema build matches and the layers that `TypeShape.Unwrapper` unwraps in the method, field and Map data fetchers. An untyped Map's values reach graphql-java as they are. A container kind can't be added without its placement rule and its value step, since both are abstract on the enum, and `ReturnValueUnwrappingTest` fails until the bounded check builds values for it | -| A resolver method's parameter layout, type variables and source are decided once, at schema build | `ResolverPlan`, which reads the field's member once and hands the scan its type shapes, the data fetcher its parameters and `UnmappedInputFieldChecker` its argument targets. Its member variants are private to `ResolverPlan.kt`, each decides its `ResolverKind`, and none builds a data fetcher. `ResolverPlan.dataFetcher` gives every member a `ResolverDataFetcher`, a sealed class whose constructor takes the `TypeShape.Unwrapper`, so a lambda or a fetcher without one doesn't compile. A new `ResolverKind` doesn't compile until the lookup's `when` handles it, a new `ExtraParameter` doesn't compile without its binder, the fixture tests in `ResolverCompatibilityTest` fail, naming the fixture to add, until they build a sample of each and see its value unwrapped, and `ReturnValueUnwrappingTest` ends in `MODEL ERROR` until the bounded check runs the new kind. `ResolverOwnershipTest` fails on member reflection outside `ResolverPlan.kt` (`java.lang.reflect` calls and signature reads, `MethodHandles`, kotlin-reflect member lookups and any use of a `KFunction` or `KProperty`, and commons-lang3's field and method utilities, apart from the type reads in `ScanDictionary.findInputValueTypeInType(String, Class)`), on a member that depends on a data fetcher, on a library data fetcher that isn't a `ResolverDataFetcher` declared in `ResolverPlan.kt`, and on generic-wrapper or `DataFetcherResult` handling outside `TypeShape.kt`. Its exemptions name the exact classes each owning file declares. It doesn't see `Future`, `CompletionStage` or `Optional` checks outside `TypeShape.kt`, `Class.forName` lookups, `Method.getReturnType`, reflection a library does for you (graphql-java's `PropertyDataFetcher`, `java.beans.Expression`), a new helper in an owning file that reflects for another file, or a `DataFetcher { }` lambda outside `ResolverPlan.kt`. The fixture tests and the bounded check catch a value these leave wrapped only in the fetchers `ResolverPlan` builds | +| A resolver method's parameter layout, type variables and source are decided once, at schema build | `ResolverPlan`, which reads the field's member once and hands the scan its type shapes, the data fetcher its parameters and `UnmappedInputFieldChecker` its argument targets. Its member variants are private to `ResolverPlan.kt`, each decides its `ResolverKind`, and none builds a data fetcher. `ResolverPlan.dataFetcher` gives every member a `ResolverDataFetcher`, a sealed class whose constructor takes the `TypeShape.Unwrapper`, so a lambda or a fetcher without one doesn't compile. A new `ResolverKind` doesn't compile until the lookup's `when` handles it, a new `ExtraParameter` doesn't compile without its binder, the fixture tests in `ResolverCompatibilityTest` fail, naming the fixture to add, until they build a sample of each and see its value unwrapped, and `ReturnValueUnwrappingTest` ends in `MODEL ERROR` until the bounded check runs the new kind. `ResolverOwnershipTest` fails on member reflection outside `ResolverPlan.kt` (`java.lang.reflect` calls and signature reads, `MethodHandles`, `kotlin.reflect.full` and `kotlin.reflect.jvm` calls, `KClass` member lookups, a value typed as a `KFunction` or `KProperty`, and commons-lang3's field and method utilities, apart from the type reads in `ScanDictionary.findInputValueTypeInType(String, Class)`), on a member that depends on a data fetcher, on a library data fetcher that isn't a `ResolverDataFetcher` declared in `ResolverPlan.kt`, and on generic-wrapper or `DataFetcherResult` handling outside `TypeShape.kt`. Its exemptions name the exact classes each owning file declares. It doesn't see `Future`, `CompletionStage` or `Optional` checks outside `TypeShape.kt`, `Class.forName` lookups, `Method.getReturnType`, a `KFunction` held as `Any` and called through a cast, reflection a library does for you (graphql-java's `PropertyDataFetcher`, `java.beans.Expression`), a new helper in an owning file that reflects for another file, or a `DataFetcher { }` lambda outside `ResolverPlan.kt`. The fixture tests and the bounded check catch a value these leave wrapped only in the fetchers `ResolverPlan` builds | | A change to how resolvers are found, bound or unwrapped keeps what a method, a property and a Map field return | `ResolverCompatibilityTest`, which compares field and Map values with a method's, and the bounded check in `ReturnValueUnwrappingTest`, which compares all three with the plain value. Neither has known findings, so a value that differs on a return type they run fails the build. The bounded check also pins how many return types it generates, and for each kind how many the build rejects and how many executions run, so a change that stops running some fails until the same diff updates `pinnedCoverage`. It doesn't judge whether a newly rejected return type should build | | One output has one builder. Don't add a second path that builds the same thing | docs only | | Read an SDL type's members merged with its extensions, never from the raw definition | `MergedType` in `MergedDefinitions.kt` and `DefinitionsBoundaryTest`, which fails on a raw member, directive or child-node read, an extension class, an SDL node walk or a `TypeDefinitionRegistry` call outside it. A read by reflection gets past it. `TypeExtensionTest` builds and queries a schema with an extension of every kind, so a build that leaves an extension's fields, interfaces, members, values, input fields or directives out of the built type fails it, however the code read them |