diff --git a/.claude/skills/verify-return-value-unwrapping/SKILL.md b/.claude/skills/verify-return-value-unwrapping/SKILL.md index 6030c798..dba6d4b3 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, public field or 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 ReturnValueUnwrapper, TypeClassMatcher's unwrapping, GenericTypeResolver.resolve, 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, public field or 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. --- # Verify return value unwrapping @@ -73,7 +73,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, or the build rejected a return type the check must exercise, 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, or a `ContainerKind` in `TypeShape.kt` has no `Container` in `Check.java`, 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 | Options: `--depth N`, `--budget N`, `--property-budget N`, `--show N` (counterexamples and stale return types printed, default 15), and `--rejected`, which lists @@ -157,14 +157,12 @@ 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 `af9ba4ad` (agent-friendly, 2026-11-04). It covers: +Last confirmed to describe the code at commit `6d41328d` (2026-11-05). It covers: | File | What | |---|---| -| `src/main/kotlin/graphql/kickstart/tools/resolver/ReturnValueUnwrapper.kt` | all of it | -| `src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt` | `PotentialMatch.unwrap`, `Unwrapped` | -| `src/main/kotlin/graphql/kickstart/tools/GenericTypeResolver.kt` | `resolve`, `resolveTypeVariables` | -| `src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt` | `createDataFetcher` (`valueClass`, the unwrapper), `returnType`, `returnValueMatch`, `LightMethodFieldResolverDataFetcher` | +| `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/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 | @@ -172,10 +170,11 @@ Last confirmed to describe the code at commit `af9ba4ad` (agent-friendly, 2026-1 To see whether a change needs another look: ```bash -git log --oneline af9ba4ad..HEAD -- src/main/kotlin/graphql/kickstart/tools/resolver/ReturnValueUnwrapper.kt src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt src/main/kotlin/graphql/kickstart/tools/GenericTypeResolver.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 6d41328d..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 ``` 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`. After an intended change, rerun the check, +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, 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 2da586e8..900d03a3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -91,7 +91,7 @@ happening. "docs only" means nothing does yet, so the rule depends on you readin | Rule | Enforced by | |---|---| -| A resolver method's return value is unwrapped by one rule that the schema build and the data fetcher share | docs only | +| A resolver method's return value 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 the data fetcher unwraps. 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 change to how resolvers are found, bound or unwrapped keeps what a method, a property and a Map field return | `ResolverCompatibilityTest` and the bounded check in `ReturnValueUnwrappingTest`. Each lists exactly today's property and Map gaps as known findings, and fails when one is fixed until it's removed, or when a new failure would hide under one | | One output has one builder. Don't add a second path that builds the same thing | docs only | diff --git a/src/main/kotlin/graphql/kickstart/tools/GenericTypeResolver.kt b/src/main/kotlin/graphql/kickstart/tools/GenericTypeResolver.kt deleted file mode 100644 index 4ad02eb8..00000000 --- a/src/main/kotlin/graphql/kickstart/tools/GenericTypeResolver.kt +++ /dev/null @@ -1,97 +0,0 @@ -package graphql.kickstart.tools - -import com.fasterxml.classmate.ResolvedType -import graphql.kickstart.tools.util.JavaType -import graphql.kickstart.tools.util.ParameterizedTypeImpl -import graphql.kickstart.tools.util.Primitives -import org.apache.commons.lang3.reflect.TypeUtils -import java.lang.reflect.ParameterizedType -import java.lang.reflect.TypeVariable -import java.lang.reflect.WildcardType - -/** - * @author Andrew Potter - */ -internal class GenericTypeResolver(private val containingType: JavaType, private val options: SchemaParserOptions) { - - fun getRawClass() = getRawClass(containingType) - - fun getRawClass(type: JavaType): Class<*> = TypeUtils.getRawType(type, containingType) - - fun isAssignableFrom(type: JavaType) = TypeUtils.isAssignable(type, containingType) - - /** - * Turns a type as declared in the source (a method's return or parameter type, or a field's type) into the type - * the GraphQL type is matched against: - * - type variables are resolved relative to the containing type, - * e.g. `T` of `AbstractItem.getId()` becomes `Long` for `class Item : AbstractItem()` - * - generic wrappers are replaced by their wrapped type, e.g. `CompletableFuture` becomes `Foo` - * - wildcards are replaced by their upper bound, e.g. `? extends Foo` becomes `Foo` - * - primitives are replaced by their boxed class, e.g. `int` becomes `Integer` - */ - fun resolve(javaType: JavaType, onUnwrap: ((SchemaParserOptions.GenericWrapper) -> Unit)? = null): JavaType { - return when (val type = resolveTypeVariables(javaType)) { - is ParameterizedType -> { - val rawType = type.rawType - val wrapper = options.genericWrappers.find { it.type == rawType } - ?: return type - - val typeArguments = type.actualTypeArguments - if (typeArguments.size <= wrapper.index) { - throw IndexOutOfBoundsException("Generic type '${TypeUtils.toString(type)}' does not have a type argument at index ${wrapper.index}!") - } - - onUnwrap?.invoke(wrapper) - val unwrapsTo = wrapper.schemaWrapper.invoke(typeArguments[wrapper.index]) - resolve(unwrapsTo, onUnwrap) - } - is TypeVariable<*> -> error("Could not resolve type variable '${TypeUtils.toLongString(type)}' relative to ${TypeUtils.toString(containingType)}") - // Kotlin reflection gives a suspend function returning List the type List when Foo is open. - // A type variable bound stays as it is, since a method's own variable can't be resolved and a scalar doesn't need it - is WildcardType -> when (val bound = type.upperBounds.firstOrNull() ?: error("Unable to unwrap type, wildcard has no upper bound: $type")) { - is ParameterizedType -> resolve(bound, onUnwrap) - else -> bound - } - is Class<*> -> if (type.isPrimitive) Primitives.wrap(type) else type - else -> error("Unable to unwrap type: $type") - } - } - - fun resolveTypeVariables(type: JavaType, resolving: Set> = emptySet()): JavaType { - return when (type) { - is ParameterizedType -> { - val actualTypeArguments = type.actualTypeArguments.map { resolveTypeVariables(it, resolving) }.toTypedArray() - ParameterizedTypeImpl(type.rawType as Class<*>, actualTypeArguments, type.ownerType?.let { resolveTypeVariables(it, resolving) }) - } - is WildcardType -> TypeUtils.wildcardType() - .withUpperBounds(*type.upperBounds.map { resolveTypeVariables(it, resolving) }.toTypedArray()) - .withLowerBounds(*type.lowerBounds.map { resolveTypeVariables(it, resolving) }.toTypedArray()) - .build() - is ResolvedType -> { - if (type.typeParameters.isEmpty()) { - type.erasedType - } else { - val actualTypeArguments = type.typeParameters.map { resolveTypeVariables(it) }.toTypedArray() - ParameterizedTypeImpl(type.erasedType, actualTypeArguments, null) - } - } - is TypeVariable<*> -> { - val genericDeclaration = type.genericDeclaration - when { - // only a variable leaked from a raw type can be bound to a type containing itself (e.g. T -> List), - // erase it like the raw type does instead of expanding it forever - type in resolving -> TypeUtils.getRawType(type.bounds.first(), null) ?: Any::class.java - // the containing type binds the variables of all its supertypes - genericDeclaration is Class<*> -> generateSequence(containingType) { (it as? ParameterizedType)?.ownerType } - // an inner class can also use the variables of its outer class, those are bound by its owner type (e.g. Connection.Entry) - .firstNotNullOfOrNull { TypeUtils.getTypeArguments(it, genericDeclaration)?.get(type) } - // keep the full type argument (e.g. List) rather than its raw class so nested generics aren't lost - ?.let { resolveTypeVariables(it, resolving + type) } - ?: type - else -> type - } - } - else -> type - } - } -} diff --git a/src/main/kotlin/graphql/kickstart/tools/ScanDictionary.kt b/src/main/kotlin/graphql/kickstart/tools/ScanDictionary.kt index 7a76bdd7..656a667b 100644 --- a/src/main/kotlin/graphql/kickstart/tools/ScanDictionary.kt +++ b/src/main/kotlin/graphql/kickstart/tools/ScanDictionary.kt @@ -1,5 +1,6 @@ package graphql.kickstart.tools +import graphql.kickstart.tools.resolver.TypeShape import graphql.kickstart.tools.util.JavaType import graphql.kickstart.tools.util.unwrap import graphql.language.* @@ -91,11 +92,12 @@ internal class ScanDictionary( return@forEach } - handleFoundType(typeClassMatcher.match(TypeClassMatcher.PotentialMatch.parameterType( + handleFoundType(typeClassMatcher.match(TypeShape.parameterType( // a class from the dictionary is for the unwrapped type, not for its list or non-null wrapper if (propertyType != null) inputValueDefinition.type else inputGraphQLType, inputValueJavaType, - GenericTypeResolver(javaType ?: inputValueJavaType, options), + javaType ?: inputValueJavaType, + options, InputObjectReference(inputValueDefinition) ))) } diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt index ec170918..db08b8fb 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt @@ -15,6 +15,7 @@ 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 @@ -428,8 +429,8 @@ internal class SchemaClassScanner( fieldResolversByType.getOrPut(type) { mutableMapOf() }[fieldResolver.field] = fieldResolver - fieldResolver.scanForMatches().forEach { potentialMatch -> - dictionary.handleFoundType(typeClassMatcher.match(potentialMatch)) + fieldResolver.scanForMatches().forEach { shape -> + dictionary.handleFoundType(typeClassMatcher.match(shape)) } requireParameterForEachArgument(type, fieldResolver) } @@ -471,7 +472,7 @@ internal class SchemaClassScanner( } fieldResolver.field.inputValueDefinitions.forEachIndexed { index, argument -> - val parameter = fieldResolver.getMethodParameterType(index)?.let(fieldResolver.typeResolver::getRawClass) ?: return@forEachIndexed + val parameter = fieldResolver.getMethodParameterType(index)?.let { TypeUtils.getRawType(it, fieldResolver.search.type) } ?: return@forEachIndexed if (!fieldResolverScanner.isContextParameter(parameter, argument)) { return@forEachIndexed } diff --git a/src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt b/src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt index 93cffe0e..853acae5 100644 --- a/src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt +++ b/src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt @@ -1,34 +1,27 @@ package graphql.kickstart.tools -import graphql.execution.DataFetcherResult -import graphql.kickstart.tools.SchemaParserOptions.GenericWrapper -import graphql.kickstart.tools.util.GraphQLLangType +import graphql.kickstart.tools.resolver.TypeShape import graphql.kickstart.tools.util.JavaType -import graphql.kickstart.tools.util.isSubtypeOf import graphql.language.* -import graphql.schema.idl.TypeUtil -import java.lang.reflect.ParameterizedType -import java.util.* /** * @author Andrew Potter */ internal class TypeClassMatcher(private val definitions: MergedDefinitions) { - fun match(potentialMatch: PotentialMatch): Match { - val unwrapping = potentialMatch.unwrapping - return when (val graphQLType = unwrapping.graphQLType) { + fun match(shape: TypeShape): Match { + return when (val graphQLType = shape.leafGraphQLType) { is TypeName -> { val typeDefinition = definitions.resolve(graphQLType.name) - ?: throw potentialMatch.error("No ${TypeDefinition::class.java.simpleName} for type name ${graphQLType.name}") + ?: throw shape.error("No ${TypeDefinition::class.java.simpleName} for type name ${graphQLType.name}") if (typeDefinition is MergedType.Scalar) { ScalarMatch(typeDefinition) } else { - ValidMatch(typeDefinition, unwrapping.javaType, potentialMatch.reference) + ValidMatch(typeDefinition, shape.leafJavaType, shape.reference) } } - else -> throw potentialMatch.error("Unknown type: ${unwrapping.javaType.javaClass.name}") + else -> throw shape.error("Unknown type: ${shape.leafJavaType.javaClass.name}") } } @@ -37,100 +30,4 @@ internal class TypeClassMatcher(private val definitions: MergedDefinitions) { internal data class ScalarMatch(val type: MergedType.Scalar) : Match internal data class ValidMatch(val type: MergedType<*>, val javaType: JavaType, val reference: ScanDictionary.Reference) : Match - - internal enum class Location(val prettyName: String) { - RETURN_TYPE("return type"), - PARAMETER_TYPE("parameter"), - } - - internal data class PotentialMatch( - val graphQLType: GraphQLLangType, - val javaType: JavaType, - val typeResolver: GenericTypeResolver, - val reference: ScanDictionary.Reference, - val location: Location - ) { - val unwrapping: Unwrapping by lazy { unwrap(graphQLType, javaType, root = true, mutableListOf()) } - - fun error(msg: String) = SchemaClassScannerError("Unable to match type definition (${TypeUtil.simplePrint(graphQLType)}) for reference $reference with java type ($javaType): $msg") - - private fun unwrap(graphQLType: GraphQLLangType, javaType: JavaType, root: Boolean, unwrapped: MutableList): Unwrapping { - val onUnwrap = { wrapper: GenericWrapper -> unwrapped += Unwrapped.Wrapper(wrapper) } - var realType = typeResolver.resolve(javaType, onUnwrap) - var data = false - var optional = false - - while (realType is ParameterizedType) { - when { - realType.isSubtypeOf(DataFetcherResult::class.java) -> { - if (location != Location.RETURN_TYPE) { - throw error("${DataFetcherResult::class.java.name} can only be used as a return type") - } - if (data) { - throw error("${DataFetcherResult::class.java.name} cannot be nested within itself") - } - data = true - unwrapped += Unwrapped.Data - } - - realType.isSubtypeOf(Optional::class.java) -> { - if (location == Location.RETURN_TYPE && !root) { - throw error("${Optional::class.java.name} can only be used at the top level of a return type") - } - if (optional) { - throw error("${Optional::class.java.name} cannot be nested within itself") - } - optional = true - unwrapped += Unwrapped.Present - } - - else -> break - } - realType = typeResolver.resolve(realType.actualTypeArguments.first(), onUnwrap) - } - - // Match graphql type to java type. - return when (graphQLType) { - is NonNullType -> { - if (optional) { - throw error("graphql type is marked as nonnull but ${Optional::class.java.name} was used") - } - unwrap(graphQLType.type, realType, root = false, unwrapped) - } - - is ListType -> when { - realType is ParameterizedType && realType.isSubtypeOf(Iterable::class.java) -> { - unwrapped += Unwrapped.Elements - unwrap(graphQLType.type, realType.actualTypeArguments.first(), root = false, unwrapped) - } - realType is Class<*> && realType.isArray -> - unwrap(graphQLType.type, realType.componentType, root = false, unwrapped) - // e.g. a property map value: the element type is unknown, so match it as Object too - realType == Any::class.java -> - unwrap(graphQLType.type, realType, root = false, unwrapped) - else -> - throw error("Java class is not a List or generic type information was lost: $realType") - } - - else -> Unwrapping(unwrapped, graphQLType, realType) - } - } - - companion object { - fun returnValue(graphQLType: GraphQLLangType, javaType: JavaType, typeResolver: GenericTypeResolver, reference: ScanDictionary.Reference) = - PotentialMatch(graphQLType, javaType, typeResolver, reference, Location.RETURN_TYPE) - - fun parameterType(graphQLType: GraphQLLangType, javaType: JavaType, typeResolver: GenericTypeResolver, reference: ScanDictionary.Reference) = - PotentialMatch(graphQLType, javaType, typeResolver, reference, Location.PARAMETER_TYPE) - } - } - - internal class Unwrapping(val unwrapped: List, val graphQLType: GraphQLLangType, val javaType: JavaType) - - internal sealed interface Unwrapped { - class Wrapper(val wrapper: GenericWrapper) : Unwrapped - object Data : Unwrapped - object Present : Unwrapped - object Elements : Unwrapped - } } diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolver.kt index 4cc378fd..22a93e03 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolver.kt @@ -4,6 +4,7 @@ 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 @@ -14,9 +15,8 @@ internal abstract class FieldResolver( val options: SchemaParserOptions ) { val resolverInfo: ResolverInfo = search.resolverInfo - val typeResolver = GenericTypeResolver(search.type, options) - abstract fun scanForMatches(): List + abstract fun scanForMatches(): Sequence abstract fun createDataFetcher(): DataFetcher<*> @@ -33,8 +33,8 @@ internal abstract class FieldResolver( ?: environment?.getSource() ?: throw ResolverError("Expected DataFetchingEnvironment and source object to not be null!") - if (!this.typeResolver.isAssignableFrom(source.javaClass)) { - throw ResolverError("Expected source object to be an instance of '${this.typeResolver.getRawClass().name}' but instead got '${source.javaClass.name}'") + 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 diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/MapFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/MapFieldResolver.kt index 79b5bb4f..c13f67f4 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/MapFieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/MapFieldResolver.kt @@ -3,7 +3,6 @@ package graphql.kickstart.tools.resolver import com.fasterxml.classmate.TypeResolver import graphql.kickstart.tools.ScanDictionary import graphql.kickstart.tools.SchemaParserOptions -import graphql.kickstart.tools.TypeClassMatcher import graphql.kickstart.tools.util.JavaType import graphql.language.FieldDefinition import graphql.schema.DataFetcher @@ -43,8 +42,8 @@ internal class MapFieldResolver( return MapFieldResolverDataFetcher(createSourceResolver(), field.name) } - override fun scanForMatches(): List { - return listOf(TypeClassMatcher.PotentialMatch.returnValue(field.type, mapGenericValue, typeResolver, ScanDictionary.FieldTypeReference(field.name))) + override fun scanForMatches(): Sequence { + return sequenceOf(TypeShape.returnType(field.type, mapGenericValue, isSubscription = false, search.type, options, ScanDictionary.FieldTypeReference(field.name))) } override fun toString() = "MapFieldResolver{key=${field.name}}" diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt index 7457dfc4..63fc38e2 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt @@ -4,7 +4,6 @@ import graphql.GraphQLContext import graphql.kickstart.tools.* import graphql.kickstart.tools.util.JavaType import graphql.kickstart.tools.util.coroutineScope -import graphql.kickstart.tools.util.futureValueType import graphql.kickstart.tools.util.isSuspendFunction import graphql.kickstart.tools.util.parameterCountWithoutContinuation import graphql.kickstart.tools.util.typeArgument @@ -18,12 +17,10 @@ import kotlinx.coroutines.CoroutineStart import kotlinx.coroutines.ensureActive import kotlinx.coroutines.future.future import org.apache.commons.lang3.reflect.TypeUtils -import org.reactivestreams.Publisher import tools.jackson.databind.ObjectMapper import java.lang.reflect.InvocationTargetException import java.lang.reflect.Method import java.util.* -import java.util.concurrent.Future import java.util.function.Supplier import kotlin.coroutines.intrinsics.suspendCoroutineUninterceptedOrReturn import kotlin.reflect.jvm.javaType @@ -70,7 +67,7 @@ internal class MethodFieldResolver( ?: 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 = this.typeResolver.getRawClass(parameterType) == Optional::class.java + 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) @@ -117,11 +114,7 @@ internal class MethodFieldResolver( } } - // the class the return type unwraps to, so a value that happens to implement Future isn't mistaken for a wrapper. - // Object (e.g. for a union) can't tell them apart, so then every future of a method whose return type is Future is checked - val valueClass = (TypeUtils.getRawType(getResolvedReturnType(), null) ?: Any::class.java) - .takeUnless { it == Any::class.java && TypeUtils.getRawType(returnType, null) == Future::class.java } - val unwrapper = ReturnValueUnwrapper.create(method, returnValueMatch.unwrapping, valueClass, isSuspendFunction, search.isSubscription, options.genericWrappers) + val unwrapper = TypeShape.Unwrapper(returnShape, method, isSuspendFunction) return if (numberOfParameters > 0 || isSuspendFunction) { // requires arguments and environment or is a suspend function @@ -148,37 +141,23 @@ internal class MethodFieldResolver( } } - override fun scanForMatches(): List { - return field.inputValueDefinitions.mapIndexed { i, inputDefinition -> - TypeClassMatcher.PotentialMatch.parameterType(inputDefinition.type, getMethodParameterType(i)!!, typeResolver, ScanDictionary.MethodParameterReference(method, i)) - } + listOf(returnValueMatch) + // 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) } - // generic wrappers only match their exact type, so a Publisher implementation (e.g. Flux), or a future of one, - // is matched as a plain Publisher of its events - private fun JavaType.asPublisher(): JavaType = - (futureValueType() ?: this).typeArgument(Publisher::class.java) - ?.let { TypeUtils.parameterize(Publisher::class.java, it) } - ?: this - - // the declared return type with type variables resolved, e.g. Future for get(): T in a subclass of Base> - private val returnType: JavaType by lazy { + private val returnShape by lazy { val declaredType = try { method.kotlinFunction?.returnType?.javaType ?: method.genericReturnType } catch (e: InternalError) { method.genericReturnType } - typeResolver.resolveTypeVariables(declaredType) - } - - private val returnValueMatch by lazy { - TypeClassMatcher.PotentialMatch.returnValue(field.type, matchedReturnType(), typeResolver, ScanDictionary.ReturnValueReference(method)) + TypeShape.returnType(field.type, resolveTypeVariables(declaredType, search.type), search.isSubscription, search.type, options, ScanDictionary.ReturnValueReference(method)) } - private fun matchedReturnType(): JavaType = if (search.isSubscription) returnType.asPublisher() else returnType - - private fun getResolvedReturnType(): JavaType = typeResolver.resolve(matchedReturnType()) - private fun getJavaMethodParameterIndex(index: Int) = index + search.firstArgumentIndex internal fun getMethodParameterType(index: Int): JavaType? { @@ -186,7 +165,7 @@ internal class MethodFieldResolver( val parameters = method.parameterTypes return if (parameters.size > methodIndex) { - typeResolver.resolveTypeVariables(method.genericParameterTypes[methodIndex]) + resolveTypeVariables(method.genericParameterTypes[methodIndex], search.type) } else { null } @@ -201,7 +180,7 @@ internal class MethodFieldResolverDataFetcher( private val args: List, private val options: SchemaParserOptions, private val isSuspendFunction: Boolean, - private val unwrapper: ReturnValueUnwrapper + private val unwrapper: TypeShape.Unwrapper ) : DataFetcher { override fun get(environment: DataFetchingEnvironment): Any? { @@ -235,7 +214,7 @@ internal class MethodFieldResolverDataFetcher( internal class LightMethodFieldResolverDataFetcher( private val sourceResolver: SourceResolver, private val method: Method, - private val unwrapper: ReturnValueUnwrapper + private val unwrapper: TypeShape.Unwrapper ) : LightDataFetcher { override fun get(fieldDefinition: GraphQLFieldDefinition?, sourceObject: Any?, environmentSupplier: Supplier): Any? { diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/MissingFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/MissingFieldResolver.kt index 652af687..98f6f00a 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/MissingFieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/MissingFieldResolver.kt @@ -2,7 +2,6 @@ package graphql.kickstart.tools.resolver import graphql.kickstart.tools.MissingResolverInfo import graphql.kickstart.tools.SchemaParserOptions -import graphql.kickstart.tools.TypeClassMatcher import graphql.language.FieldDefinition import graphql.schema.DataFetcher @@ -13,6 +12,6 @@ internal class MissingFieldResolver( private val missingResolverDataFetcherProvider: MissingResolverDataFetcherProvider = options.missingResolverDataFetcherProvider ?: MissingResolverDataFetcherProvider { _, _ -> DataFetcher { TODO("Schema resolver not implemented") } } - override fun scanForMatches(): List = listOf() + 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 index 1e6cb062..7f93c648 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/PropertyFieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/PropertyFieldResolver.kt @@ -2,7 +2,6 @@ package graphql.kickstart.tools.resolver import graphql.kickstart.tools.ScanDictionary import graphql.kickstart.tools.SchemaParserOptions -import graphql.kickstart.tools.TypeClassMatcher import graphql.language.FieldDefinition import graphql.schema.DataFetcher import graphql.schema.DataFetchingEnvironment @@ -25,12 +24,14 @@ internal class PropertyFieldResolver( return PropertyFieldResolverDataFetcher(createSourceResolver(), property) } - override fun scanForMatches(): List { - return listOf( - TypeClassMatcher.PotentialMatch.returnValue( + override fun scanForMatches(): Sequence { + return sequenceOf( + TypeShape.returnType( field.type, property.genericType, - typeResolver, + isSubscription = false, + search.type, + options, ScanDictionary.FieldTypeReference(property.toString()) ) ) diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/ReturnValueUnwrapper.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/ReturnValueUnwrapper.kt deleted file mode 100644 index 2c28bc27..00000000 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/ReturnValueUnwrapper.kt +++ /dev/null @@ -1,188 +0,0 @@ -package graphql.kickstart.tools.resolver - -import graphql.execution.DataFetcherResult -import graphql.kickstart.tools.ResolverError -import graphql.kickstart.tools.SchemaParserOptions.GenericWrapper -import graphql.kickstart.tools.TypeClassMatcher.Unwrapped -import graphql.kickstart.tools.TypeClassMatcher.Unwrapping -import graphql.schema.DataFetchingEnvironment -import kotlinx.coroutines.future.asDeferred -import org.apache.commons.lang3.reflect.TypeUtils -import java.lang.reflect.Method -import java.util.* -import java.util.concurrent.CompletableFuture -import java.util.concurrent.CompletionStage -import java.util.concurrent.Future -import java.util.function.Supplier - -internal class ReturnValueUnwrapper private constructor( - private val genericWrappers: List, - private val awaitable: Awaitable, - private val isOutermostFuture: Boolean, - private val nested: Step? -) { - - fun unwrap(value: Any, environmentSupplier: Supplier): Any? { - val wrapper = genericWrappers.wrapperOf(value) - val transformed = if (wrapper == null) value else wrapper.transformer(value, environmentSupplier.get()) - val unwrapped = awaitable.requireOutermost(transformed) - return if (isOutermostFuture || wrapper?.transformerReturnsCompletionStage == true) { - unwrapped.then(nested, environmentSupplier) - } else { - unwrapped.thenUnlessNextAwaits(nested, environmentSupplier) - } - } - - suspend fun unwrapSuspending(value: Any, environmentSupplier: Supplier): Any? { - val unwrapped = unwrap(value, environmentSupplier) - // the coroutine's future would complete with the future made below the outermost value, which graphql-java - // doesn't wait on. asDeferred, so cancelling the coroutine doesn't cancel a future the resolver returned - return if (nested != null && unwrapped is CompletionStage<*>) unwrapped.asDeferred().await() else unwrapped - } - - companion object { - fun create( - method: Method, - unwrapping: Unwrapping, - valueClass: Class<*>?, - isSuspendFunction: Boolean, - isSubscription: Boolean, - genericWrappers: List - ): ReturnValueUnwrapper { - val unwrapped = unwrapping.unwrapped - val outermost = unwrapped.firstOrNull() as? Unwrapped.Wrapper - val belowOutermost = if (outermost == null) unwrapped else unwrapped.drop(1) - val leafClass = TypeUtils.getRawType(unwrapping.javaType, null) - val specificLeafClass = leafClass?.takeUnless { it == Any::class.java } - val awaits = Awaitable(method, specificLeafClass, isSuspendFunction) - val peels = Awaitable(method, leafClass, isSuspendFunction) - // graphql-java completes a subscription's events one at a time - val nested = if (isSubscription) null else steps(belowOutermost, awaits, peels) - return ReturnValueUnwrapper(genericWrappers, Awaitable(method, valueClass, isSuspendFunction), outermost?.isFuture == true, nested) - } - } -} - -private sealed interface Step { - fun unwrap(value: Any?, environmentSupplier: Supplier): Any? -} - -private class Await(private val wrapper: GenericWrapper, private val awaitable: Awaitable, private val next: Step?) : Step { - override fun unwrap(value: Any?, environmentSupplier: Supplier): Any? = - awaitable.requireNested(value?.let { wrapper.transformer(it, environmentSupplier.get()) }).then(next, environmentSupplier) -} - -private class Peel(private val wrapper: GenericWrapper, private val awaitable: Awaitable, private val next: Step?) : Step { - override fun unwrap(value: Any?, environmentSupplier: Supplier): Any? { - val transformed = value?.let { wrapper.transformer(it, environmentSupplier.get()) } - return if (next is Await && !wrapper.transformerReturnsCompletionStage) { - next.unwrap(transformed, environmentSupplier) - } else { - awaitable.requireNested(transformed).then(next, environmentSupplier) - } - } -} - -private class EachElement(private val element: Step) : Step { - override fun unwrap(value: Any?, environmentSupplier: Supplier): Any? = - if (value is Iterable<*>) value.map { element.unwrap(it, environmentSupplier) }.awaitAll() else value -} - -private class DataOf(private val data: Step) : Step { - @Suppress("UNCHECKED_CAST") - override fun unwrap(value: Any?, environmentSupplier: Supplier): Any? = - if (value is DataFetcherResult<*>) { - data.unwrap(value.data, environmentSupplier).then { unwrapped -> (value as DataFetcherResult).map { unwrapped } } - } else { - value - } -} - -private class ValueOf(private val present: Step?) : Step { - override fun unwrap(value: Any?, environmentSupplier: Supplier): Any? = when { - value !is Optional<*> -> value - present == null -> value.orElse(null) - else -> present.unwrap(value.orElse(null), environmentSupplier) - } -} - -private val Unwrapped.Wrapper.isFuture - get() = Future::class.java.isAssignableFrom(wrapper.type) || CompletionStage::class.java.isAssignableFrom(wrapper.type) - -private fun steps(unwrapped: List, awaits: Awaitable, peels: Awaitable): Step? { - val outermost = unwrapped.firstOrNull() ?: return null - val inner = unwrapped.subList(1, unwrapped.size) - val next = steps(inner, awaits, peels) - return when (outermost) { - is Unwrapped.Wrapper -> if (outermost.isFuture) Await(outermost.wrapper, awaits, next) else Peel(outermost.wrapper, peels, next) - Unwrapped.Data -> next?.let(::DataOf) - // graphql-java reads a DataFetcherResult and then an Optional in its data, so an Optional around one is ours to unwrap - Unwrapped.Present -> if (next != null || inner.firstOrNull() == Unwrapped.Data) ValueOf(next) else null - Unwrapped.Elements -> next?.let(::EachElement) - } -} - -private fun Any?.thenUnlessNextAwaits(next: Step?, environmentSupplier: Supplier): Any? = - if (next is Await) next.unwrap(this, environmentSupplier) else then(next, environmentSupplier) - -private fun Any?.then(next: Step?, environmentSupplier: Supplier): Any? = - if (next == null) this else then { next.unwrap(it, environmentSupplier) } - -private inline fun Any?.then(crossinline next: (Any?) -> Any?): Any? = - if (this is CompletionStage<*>) thenCompose { next(it).asCompletionStage() } else next(this) - -private fun List.awaitAll(): Any { - if (none { it is CompletionStage<*> }) { - return this - } - val futures = map { it.asCompletionStage().toCompletableFuture() } - return CompletableFuture.allOf(*futures.toTypedArray()).thenApply { futures.map { it.join() } } -} - -@Suppress("UNCHECKED_CAST") -private fun Any?.asCompletionStage(): CompletionStage = - this as? CompletionStage ?: CompletableFuture.completedFuture(this) - -private fun List.wrapperOf(value: Any): GenericWrapper? { - // the JIT doesn't optimize away the sequence below on this call path, so values that aren't wrappers, which is - // most of them, return before it's allocated - if (none { it.type.isInstance(value) }) { - return null - } - return asSequence() - .filter { it.type.isInstance(value) } - .sortedWith(CompareGenericWrappers) - .firstOrNull() -} - -// 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(private val method: Method, private val valueClass: Class<*>?, private val isSuspendFunction: Boolean) { - - fun requireOutermost(value: Any?): Any? = - if (value !is Future<*> || valueClass?.isInstance(value) == true || value is CompletionStage<*> && !isSuspendFunction) value else fail(value) - - fun requireNested(value: Any?): Any? = - if (value !is Future<*> || value is CompletionStage<*> || valueClass?.isInstance(value) == true) value else fail(value) - - private fun fail(value: Any): Nothing = throw ResolverError( - if (isSuspendFunction) { - "Suspend function '${method.resolverName}' resolved to a ${value.javaClass.name}, which graphql-java can't wait on. Return its value instead." - } else { - "Resolver method '${method.resolverName}' resolved to a ${value.javaClass.name}, which graphql-java can't wait on. " + - "Return a CompletionStage, such as a CompletableFuture, from the method or its generic wrapper transformer instead." - } - ) -} - -private val Method.resolverName - get() = "${declaringClass.name}.$name" - -private class CompareGenericWrappers { - companion object : Comparator { - override fun compare(w1: GenericWrapper, w2: GenericWrapper): Int = when { - w1.type.isAssignableFrom(w2.type) -> 1 - else -> -1 - } - } -} diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt new file mode 100644 index 00000000..1696a7ed --- /dev/null +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/TypeShape.kt @@ -0,0 +1,413 @@ +package graphql.kickstart.tools.resolver + +import com.fasterxml.classmate.ResolvedType +import graphql.execution.DataFetcherResult +import graphql.kickstart.tools.ResolverError +import graphql.kickstart.tools.ScanDictionary +import graphql.kickstart.tools.SchemaClassScannerError +import graphql.kickstart.tools.SchemaParserOptions +import graphql.kickstart.tools.SchemaParserOptions.GenericWrapper +import graphql.kickstart.tools.util.GraphQLLangType +import graphql.kickstart.tools.util.JavaType +import graphql.kickstart.tools.util.ParameterizedTypeImpl +import graphql.kickstart.tools.util.Primitives +import graphql.kickstart.tools.util.futureValueType +import graphql.kickstart.tools.util.isSubtypeOf +import graphql.kickstart.tools.util.typeArgument +import graphql.language.ListType +import graphql.language.NonNullType +import graphql.schema.DataFetchingEnvironment +import graphql.schema.idl.TypeUtil +import kotlinx.coroutines.future.asDeferred +import org.apache.commons.lang3.reflect.TypeUtils +import org.reactivestreams.Publisher +import java.lang.reflect.Method +import java.lang.reflect.ParameterizedType +import java.lang.reflect.TypeVariable +import java.lang.reflect.WildcardType +import java.util.* +import java.util.concurrent.CompletableFuture +import java.util.concurrent.CompletionStage +import java.util.concurrent.Future +import java.util.function.Supplier + +// the leaf the dictionary binds and the layers a value is unwrapped through come from one walk, so they can't disagree +internal class TypeShape private constructor( + private val graphQLType: GraphQLLangType, + private val javaType: JavaType, + val reference: ScanDictionary.Reference, + val leafGraphQLType: GraphQLLangType, + val leafJavaType: JavaType, + private val layers: List, + private val declaredType: JavaType, + private val unwrappedRootType: JavaType, + private val isSubscription: Boolean, + private val wrappers: GenericWrappers +) { + + fun error(msg: String) = matchError(graphQLType, reference, javaType, msg) + + class Unwrapper(shape: TypeShape, method: Method, isSuspendFunction: Boolean) { + private val wrappers = shape.wrappers + private val awaitable: Awaitable + private val isOutermostFuture: Boolean + private val nested: Step? + + init { + // the class the return type unwraps to, so a value that happens to implement Future isn't mistaken for a wrapper. + // Object (e.g. for a union) can't tell them apart, so then every future of a method whose return type is Future is checked + val valueClass = (TypeUtils.getRawType(shape.unwrappedRootType, null) ?: Any::class.java) + .takeUnless { it == Any::class.java && TypeUtils.getRawType(shape.declaredType, null) == Future::class.java } + awaitable = Awaitable(method, valueClass, isSuspendFunction) + val outermost = shape.layers.firstOrNull() as? Layer.Wrapper + val belowOutermost = if (outermost == null) shape.layers else shape.layers.drop(1) + val leafClass = TypeUtils.getRawType(shape.leafJavaType, null) + val awaits = Awaitable(method, leafClass?.takeUnless { it == Any::class.java }, isSuspendFunction) + val peels = Awaitable(method, leafClass, isSuspendFunction) + isOutermostFuture = outermost?.isFuture == true + // graphql-java completes a subscription's events one at a time + nested = if (shape.isSubscription) null else steps(belowOutermost, awaits, peels) + } + + fun unwrap(value: Any, environmentSupplier: Supplier): Any? { + val wrapper = wrappers.ofValue(value) + val transformed = if (wrapper == null) value else wrapper.transformer(value, environmentSupplier.get()) + val unwrapped = awaitable.requireOutermost(transformed) + return if (isOutermostFuture || wrapper?.transformerReturnsCompletionStage == true) { + unwrapped.then(nested, environmentSupplier) + } else { + unwrapped.thenUnlessNextAwaits(nested, environmentSupplier) + } + } + + suspend fun unwrapSuspending(value: Any, environmentSupplier: Supplier): Any? { + val unwrapped = unwrap(value, environmentSupplier) + // the coroutine's future would complete with the future made below the outermost value, which graphql-java + // doesn't wait on. asDeferred, so cancelling the coroutine doesn't cancel a future the resolver returned + return if (nested != null && unwrapped is CompletionStage<*>) unwrapped.asDeferred().await() else unwrapped + } + } + + companion object { + val containerTypes: List> get() = ContainerKind.entries.map { it.type } + + fun returnType( + graphQLType: GraphQLLangType, + declaredType: JavaType, + isSubscription: Boolean, + containingType: JavaType, + options: SchemaParserOptions, + reference: ScanDictionary.Reference + ): TypeShape { + val javaType = if (isSubscription) declaredType.asPublisher() else declaredType + val walk = Walk(graphQLType, javaType, containingType, GenericWrappers(options.genericWrappers), reference, Position.RETURN_TYPE) + val (leafGraphQLType, leafJavaType) = walk.leaf() + return TypeShape(graphQLType, javaType, reference, leafGraphQLType, leafJavaType, walk.layers, declaredType, walk.unwrappedRootType, isSubscription, walk.wrappers) + } + + fun parameterType( + graphQLType: GraphQLLangType, + javaType: JavaType, + containingType: JavaType, + options: SchemaParserOptions, + reference: ScanDictionary.Reference + ): TypeShape { + val walk = Walk(graphQLType, javaType, containingType, GenericWrappers(options.genericWrappers), reference, Position.PARAMETER_TYPE) + val (leafGraphQLType, leafJavaType) = walk.leaf() + return TypeShape(graphQLType, javaType, reference, leafGraphQLType, leafJavaType, walk.layers, javaType, walk.unwrappedRootType, false, walk.wrappers) + } + } +} + +// unlike the walk, keeps generic wrappers, wildcards and primitives, which argument conversion needs +internal fun resolveTypeVariables(type: JavaType, containingType: JavaType, resolving: Set> = emptySet()): JavaType { + return when (type) { + is ParameterizedType -> { + val actualTypeArguments = type.actualTypeArguments.map { resolveTypeVariables(it, containingType, resolving) }.toTypedArray() + ParameterizedTypeImpl(type.rawType as Class<*>, actualTypeArguments, type.ownerType?.let { resolveTypeVariables(it, containingType, resolving) }) + } + is WildcardType -> TypeUtils.wildcardType() + .withUpperBounds(*type.upperBounds.map { resolveTypeVariables(it, containingType, resolving) }.toTypedArray()) + .withLowerBounds(*type.lowerBounds.map { resolveTypeVariables(it, containingType, resolving) }.toTypedArray()) + .build() + is ResolvedType -> { + if (type.typeParameters.isEmpty()) { + type.erasedType + } else { + val actualTypeArguments = type.typeParameters.map { resolveTypeVariables(it, containingType) }.toTypedArray() + ParameterizedTypeImpl(type.erasedType, actualTypeArguments, null) + } + } + is TypeVariable<*> -> { + val genericDeclaration = type.genericDeclaration + when { + // only a variable leaked from a raw type can be bound to a type containing itself (e.g. T -> List), + // erase it like the raw type does instead of expanding it forever + type in resolving -> TypeUtils.getRawType(type.bounds.first(), null) ?: Any::class.java + genericDeclaration is Class<*> -> generateSequence(containingType) { (it as? ParameterizedType)?.ownerType } + // an inner class can also use the variables of its outer class, those are bound by its owner type (e.g. Connection.Entry) + .firstNotNullOfOrNull { TypeUtils.getTypeArguments(it, genericDeclaration)?.get(type) } + // keep the full type argument (e.g. List) rather than its raw class so nested generics aren't lost + ?.let { resolveTypeVariables(it, containingType, resolving + type) } + ?: type + else -> type + } + } + else -> type + } +} + +private enum class Position { + RETURN_TYPE, + PARAMETER_TYPE, +} + +private class Walk( + private val graphQLType: GraphQLLangType, + private val javaType: JavaType, + private val containingType: JavaType, + val wrappers: GenericWrappers, + private val reference: ScanDictionary.Reference, + private val position: Position +) { + val layers = mutableListOf() + + lateinit var unwrappedRootType: JavaType + private set + + fun leaf(): Pair = walk(graphQLType, javaType, root = true) + + private fun walk(graphQLType: GraphQLLangType, javaType: JavaType, root: Boolean): Pair { + var realType = matchedType(javaType) + if (root) { + unwrappedRootType = realType + } + val contained = mutableSetOf() + + while (true) { + val type = realType as? ParameterizedType ?: break + val kind = ContainerKind.entries.find { type.isSubtypeOf(it.type) } ?: break + kind.placementError(position, root)?.let { throw mismatch(it) } + if (!contained.add(kind)) { + throw mismatch("${kind.type.name} cannot be nested within itself") + } + layers += kind + realType = matchedType(type.actualTypeArguments.first()) + } + + return when (graphQLType) { + is NonNullType -> { + contained.find { !it.canBeNonNull }?.let { throw mismatch("graphql type is marked as nonnull but ${it.type.name} was used") } + walk(graphQLType.type, realType, root = false) + } + + is ListType -> when { + realType is ParameterizedType && realType.isSubtypeOf(Iterable::class.java) -> { + layers += Layer.Elements + walk(graphQLType.type, realType.actualTypeArguments.first(), root = false) + } + realType is Class<*> && realType.isArray -> + walk(graphQLType.type, realType.componentType, root = false) + // e.g. a property map value: the element type is unknown, so match it as Object too + realType == Any::class.java -> + walk(graphQLType.type, realType, root = false) + else -> + throw mismatch("Java class is not a List or generic type information was lost: $realType") + } + + else -> graphQLType to realType + } + } + + private fun matchedType(javaType: JavaType): JavaType { + return when (val type = resolveTypeVariables(javaType, containingType)) { + is ParameterizedType -> { + val wrapper = wrappers.ofDeclaredType(type) ?: return type + + val typeArguments = type.actualTypeArguments + if (typeArguments.size <= wrapper.index) { + throw IndexOutOfBoundsException("Generic type '${TypeUtils.toString(type)}' does not have a type argument at index ${wrapper.index}!") + } + + layers += Layer.Wrapper(wrapper) + matchedType(wrapper.schemaWrapper.invoke(typeArguments[wrapper.index])) + } + is TypeVariable<*> -> error("Could not resolve type variable '${TypeUtils.toLongString(type)}' relative to ${TypeUtils.toString(containingType)}") + // Kotlin reflection gives a suspend function returning List the type List when Foo is open. + // A type variable bound stays as it is, since a method's own variable can't be resolved and a scalar doesn't need it + is WildcardType -> when (val bound = type.upperBounds.firstOrNull() ?: error("Unable to unwrap type, wildcard has no upper bound: $type")) { + is ParameterizedType -> matchedType(bound) + else -> bound + } + is Class<*> -> if (type.isPrimitive) Primitives.wrap(type) else type + else -> error("Unable to unwrap type: $type") + } + } + + private fun mismatch(msg: String) = matchError(graphQLType, reference, javaType, msg) +} + +private fun matchError(graphQLType: GraphQLLangType, reference: ScanDictionary.Reference, javaType: JavaType, msg: String) = + SchemaClassScannerError("Unable to match type definition (${TypeUtil.simplePrint(graphQLType)}) for reference $reference with java type ($javaType): $msg") + +// generic wrappers only match their exact type, so a Publisher implementation (e.g. Flux), or a future of one, +// is matched as a plain Publisher of its events +private fun JavaType.asPublisher(): JavaType = + (futureValueType() ?: this).typeArgument(Publisher::class.java) + ?.let { TypeUtils.parameterize(Publisher::class.java, it) } + ?: this + +// 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) { + + fun ofDeclaredType(type: ParameterizedType): GenericWrapper? = wrappers.find { it.type == type.rawType } + + fun ofValue(value: Any): GenericWrapper? { + // the JIT doesn't optimize away the sequence below on this call path, so values that aren't wrappers, which is + // most of them, return before it's allocated + if (wrappers.none { it.type.isInstance(value) }) { + return null + } + return wrappers.asSequence() + .filter { it.type.isInstance(value) } + .sortedWith(CompareGenericWrappers) + .firstOrNull() + } +} + +private class CompareGenericWrappers { + companion object : Comparator { + override fun compare(w1: GenericWrapper, w2: GenericWrapper): Int = when { + w1.type.isAssignableFrom(w2.type) -> 1 + else -> -1 + } + } +} + +private sealed interface Layer { + class Wrapper(val wrapper: GenericWrapper) : Layer { + val isFuture get() = Future::class.java.isAssignableFrom(wrapper.type) || CompletionStage::class.java.isAssignableFrom(wrapper.type) + } + + object Elements : Layer +} + +private enum class ContainerKind(val type: Class<*>, val canBeNonNull: Boolean) : Layer { + DATA_FETCHER_RESULT(DataFetcherResult::class.java, canBeNonNull = true) { + override fun placementError(position: Position, root: Boolean) = + if (position != Position.RETURN_TYPE) "${type.name} can only be used as a return type" else null + + override fun step(next: Step?, inner: Layer?): Step? = next?.let(::DataOf) + }, + OPTIONAL(Optional::class.java, canBeNonNull = false) { + override fun placementError(position: Position, root: Boolean) = + if (position == Position.RETURN_TYPE && !root) "${type.name} can only be used at the top level of a return type" else null + + // graphql-java reads a DataFetcherResult and then an Optional in its data, so an Optional around one is ours to unwrap + override fun step(next: Step?, inner: Layer?): Step? = if (next != null || inner == DATA_FETCHER_RESULT) ValueOf(next) else null + }; + + abstract fun placementError(position: Position, root: Boolean): String? + + // null leaves the value to graphql-java + abstract fun step(next: Step?, inner: Layer?): Step? +} + +private fun steps(layers: List, awaits: Awaitable, peels: Awaitable): Step? { + val outermost = layers.firstOrNull() ?: return null + val inner = layers.subList(1, layers.size) + val next = steps(inner, awaits, peels) + return when (outermost) { + is Layer.Wrapper -> if (outermost.isFuture) Await(outermost.wrapper, awaits, next) else Peel(outermost.wrapper, peels, next) + is ContainerKind -> outermost.step(next, inner.firstOrNull()) + Layer.Elements -> next?.let(::EachElement) + } +} + +private sealed interface Step { + fun unwrap(value: Any?, environmentSupplier: Supplier): Any? +} + +private class Await(private val wrapper: GenericWrapper, private val awaitable: Awaitable, private val next: Step?) : Step { + override fun unwrap(value: Any?, environmentSupplier: Supplier): Any? = + awaitable.requireNested(value?.let { wrapper.transformer(it, environmentSupplier.get()) }).then(next, environmentSupplier) +} + +private class Peel(private val wrapper: GenericWrapper, private val awaitable: Awaitable, private val next: Step?) : Step { + override fun unwrap(value: Any?, environmentSupplier: Supplier): Any? { + val transformed = value?.let { wrapper.transformer(it, environmentSupplier.get()) } + return if (next is Await && !wrapper.transformerReturnsCompletionStage) { + next.unwrap(transformed, environmentSupplier) + } else { + awaitable.requireNested(transformed).then(next, environmentSupplier) + } + } +} + +private class EachElement(private val element: Step) : Step { + override fun unwrap(value: Any?, environmentSupplier: Supplier): Any? = + if (value is Iterable<*>) value.map { element.unwrap(it, environmentSupplier) }.awaitAll() else value +} + +private class DataOf(private val data: Step) : Step { + @Suppress("UNCHECKED_CAST") + override fun unwrap(value: Any?, environmentSupplier: Supplier): Any? = + if (value is DataFetcherResult<*>) { + data.unwrap(value.data, environmentSupplier).then { unwrapped -> (value as DataFetcherResult).map { unwrapped } } + } else { + value + } +} + +private class ValueOf(private val present: Step?) : Step { + override fun unwrap(value: Any?, environmentSupplier: Supplier): Any? = when { + value !is Optional<*> -> value + present == null -> value.orElse(null) + else -> present.unwrap(value.orElse(null), environmentSupplier) + } +} + +private fun Any?.thenUnlessNextAwaits(next: Step?, environmentSupplier: Supplier): Any? = + if (next is Await) next.unwrap(this, environmentSupplier) else then(next, environmentSupplier) + +private fun Any?.then(next: Step?, environmentSupplier: Supplier): Any? = + if (next == null) this else then { next.unwrap(it, environmentSupplier) } + +private inline fun Any?.then(crossinline next: (Any?) -> Any?): Any? = + if (this is CompletionStage<*>) thenCompose { next(it).asCompletionStage() } else next(this) + +private fun List.awaitAll(): Any { + if (none { it is CompletionStage<*> }) { + return this + } + val futures = map { it.asCompletionStage().toCompletableFuture() } + return CompletableFuture.allOf(*futures.toTypedArray()).thenApply { futures.map { it.join() } } +} + +@Suppress("UNCHECKED_CAST") +private fun Any?.asCompletionStage(): CompletionStage = + this as? CompletionStage ?: CompletableFuture.completedFuture(this) + +// 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(private val method: Method, private val valueClass: Class<*>?, private val isSuspendFunction: Boolean) { + + fun requireOutermost(value: Any?): Any? = + if (value !is Future<*> || valueClass?.isInstance(value) == true || value is CompletionStage<*> && !isSuspendFunction) value else fail(value) + + fun requireNested(value: Any?): Any? = + if (value !is Future<*> || value is CompletionStage<*> || valueClass?.isInstance(value) == true) value else fail(value) + + private fun fail(value: Any): Nothing = throw ResolverError( + if (isSuspendFunction) { + "Suspend function '${method.resolverName}' resolved to a ${value.javaClass.name}, which graphql-java can't wait on. Return its value instead." + } else { + "Resolver method '${method.resolverName}' resolved to a ${value.javaClass.name}, which graphql-java can't wait on. " + + "Return a CompletionStage, such as a CompletableFuture, from the method or its generic wrapper transformer instead." + } + ) +} + +private val Method.resolverName + get() = "${declaringClass.name}.$name" diff --git a/src/test/kotlin/graphql/kickstart/tools/NestedGenericWrapperTest.kt b/src/test/kotlin/graphql/kickstart/tools/NestedGenericWrapperTest.kt index 673542da..da1a934b 100644 --- a/src/test/kotlin/graphql/kickstart/tools/NestedGenericWrapperTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/NestedGenericWrapperTest.kt @@ -9,7 +9,7 @@ import graphql.execution.DataFetcherResult import graphql.Scalars import graphql.execution.DefaultValueUnboxer import graphql.kickstart.tools.SchemaParserOptions.GenericWrapper -import graphql.kickstart.tools.resolver.ReturnValueUnwrapper +import graphql.kickstart.tools.resolver.TypeShape import graphql.kickstart.tools.util.ParameterizedTypeImpl import graphql.kickstart.tools.util.unwrap import graphql.language.AstPrinter @@ -228,7 +228,6 @@ class NestedGenericWrapperTest { @Test fun `resolves every return type the matcher accepts`() { val options = SchemaParserOptions.newOptions().genericWrappers(tryWrapper, pageWrapper).build() - val typeResolver = GenericTypeResolver(Query::class.java, options) val matcher = TypeClassMatcher(MergedDefinitions(listOf(ObjectTypeDefinition("Payload"), ScalarTypeDefinition("Leaf")))) val method = Query::class.java.getMethod("listOfWrapped") val graphQLTypes = listOf(TypeName("Payload"), TypeName("Leaf")).flatMap { listOf(it, NonNullType(it), ListType(it), NonNullType(ListType(NonNullType(it))), ListType(ListType(it))) } @@ -238,23 +237,20 @@ class NestedGenericWrapperTest { val accepted = graphQLTypes.flatMap { graphQLType -> val shapesOfTheSameListDepth = shapes.filter { shape -> shape.count { it == List::class.java || it == Page::class.java } == graphQLType.listDepth() } shapesOfTheSameListDepth.flatMap { shape -> listOf(false, true).map { shape to javaType(shape, leaf(graphQLType).javaClass, wildcardElements = it) } }.mapNotNull { (shape, javaType) -> - val potentialMatch = TypeClassMatcher.PotentialMatch.returnValue(graphQLType, javaType, typeResolver, ScanDictionary.ReturnValueReference(method)) try { - matcher.match(potentialMatch) - shape to potentialMatch + val typeShape = TypeShape.returnType(graphQLType, javaType, isSubscription = false, Query::class.java, options, ScanDictionary.ReturnValueReference(method)) + matcher.match(typeShape) + Accepted(shape, graphQLType, javaType, typeShape) } catch (e: SchemaClassScannerError) { null } } } - val failures = accepted.mapNotNull { (shape, potentialMatch) -> - val graphQLType = potentialMatch.graphQLType - if (potentialMatch.unwrapping.javaType != leaf(graphQLType).javaClass) { - return@mapNotNull "${TypeUtils.toString(potentialMatch.javaType)} as ${AstPrinter.printAst(graphQLType)}: bound to ${TypeUtils.toString(potentialMatch.unwrapping.javaType)}" + val failures = accepted.mapNotNull { (shape, graphQLType, javaType, typeShape) -> + if (typeShape.leafJavaType != leaf(graphQLType).javaClass) { + return@mapNotNull "${TypeUtils.toString(javaType)} as ${AstPrinter.printAst(graphQLType)}: bound to ${TypeUtils.toString(typeShape.leafJavaType)}" } - val unwrapper = ReturnValueUnwrapper.create( - method, potentialMatch.unwrapping, TypeUtils.getRawType(typeResolver.resolve(potentialMatch.javaType), null), isSuspendFunction = false, isSubscription = false, options.genericWrappers - ) + val unwrapper = TypeShape.Unwrapper(typeShape, method, isSuspendFunction = false) val value = value(shape, leaf(graphQLType)) val result = GraphQL.newGraphQL(fieldSchema(graphQLType) { environment -> unwrapper.unwrap(value) { environment } }).build() .execute(if (leaf(graphQLType) is Payload) "query { field { data } }" else "query { field }") @@ -263,16 +259,17 @@ class NestedGenericWrapperTest { } else { result.errors.isEmpty() && result.getData() == mapOf("field" to expected(graphQLType)) } - "${TypeUtils.toString(potentialMatch.javaType)} as ${AstPrinter.printAst(graphQLType)}: ${result.errors.map { it.message }} ${result.getData()}".takeUnless { resolved } + "${TypeUtils.toString(javaType)} as ${AstPrinter.printAst(graphQLType)}: ${result.errors.map { it.message }} ${result.getData()}".takeUnless { resolved } } - assertTrue(accepted.count { (_, potentialMatch) -> leaf(potentialMatch.graphQLType) is FutureLeaf } > graphQLTypes.size) + assertTrue(accepted.count { leaf(it.graphQLType) is FutureLeaf } > graphQLTypes.size) assertEquals(failures, listOf()) } + private data class Accepted(val shape: List>, val graphQLType: Type<*>, val javaType: java.lang.reflect.Type, val typeShape: TypeShape) + @Test fun `rejects a DataFetcherResult or an Optional twice in one value`() { - val typeResolver = GenericTypeResolver(Query::class.java, SchemaParserOptions.newOptions().build()) val matcher = TypeClassMatcher(MergedDefinitions(listOf(ObjectTypeDefinition("Payload")))) val method = Query::class.java.getMethod("listOfWrapped") val result = DataFetcherResult::class.java @@ -284,9 +281,9 @@ class NestedGenericWrapperTest { ) val messages = rejections.map { (shape, _) -> - val potentialMatch = TypeClassMatcher.PotentialMatch.returnValue(TypeName("Payload"), javaType(shape, Payload::class.java, wildcardElements = false), typeResolver, ScanDictionary.ReturnValueReference(method)) try { - matcher.match(potentialMatch) + val javaType = javaType(shape, Payload::class.java, wildcardElements = false) + matcher.match(TypeShape.returnType(TypeName("Payload"), javaType, isSubscription = false, Query::class.java, SchemaParserOptions.newOptions().build(), ScanDictionary.ReturnValueReference(method))) "accepted" } catch (e: SchemaClassScannerError) { e.message.orEmpty().substringAfterLast("): ") diff --git a/src/test/kotlin/graphql/kickstart/tools/TypeClassMatcherTest.kt b/src/test/kotlin/graphql/kickstart/tools/TypeClassMatcherTest.kt index c8ce6299..deb1adee 100644 --- a/src/test/kotlin/graphql/kickstart/tools/TypeClassMatcherTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/TypeClassMatcherTest.kt @@ -48,11 +48,8 @@ class TypeClassMatcherTest { private val scanner: FieldResolverScanner = FieldResolverScanner(options, GRAPHQL_SCALARS.keys) private val resolver = RootResolverInfo(listOf(QueryMethods()), options) - private fun createPotentialMatch(methodName: String, graphQLType: Type<*>): TypeClassMatcher.PotentialMatch { - return scanner.findFieldResolver(FieldDefinition(methodName, graphQLType), resolver) - .scanForMatches() - .find { it.location == TypeClassMatcher.Location.RETURN_TYPE }!! - } + private fun matchReturnType(methodName: String, graphQLType: Type<*>): TypeClassMatcher.Match = + matcher.match(scanner.findFieldResolver(FieldDefinition(methodName, graphQLType), resolver).scanForMatches().single()) private fun list(other: Type<*> = customType): Type<*> = ListType(other) private fun nonNull(other: Type<*> = customType): Type<*> = NonNullType(other) @@ -63,7 +60,7 @@ class TypeClassMatcherTest { @Test fun `matcher verifies that nested return type matches graphql definition for method`() { - val match = matcher.match(createPotentialMatch(methodName, type)) + val match = matchReturnType(methodName, type) match as TypeClassMatcher.ValidMatch assertEquals(match.type, customDefinition) assertEquals(match.javaType, CustomType::class.java) @@ -98,7 +95,7 @@ class TypeClassMatcherTest { @Test(expected = SchemaClassScannerError::class) fun `matcher verifies that nested return type doesn't match graphql definition for method`() { - matcher.match(createPotentialMatch(methodName, type)) + matchReturnType(methodName, type) } companion object { @@ -118,7 +115,7 @@ class TypeClassMatcherTest { @Test(expected = SchemaClassScannerError::class) fun `matcher verifies return value optionals are used incorrectly for method`() { - matcher.match(createPotentialMatch(methodName, type)) + matchReturnType(methodName, type) } companion object { @@ -137,7 +134,7 @@ class TypeClassMatcherTest { class Suit4 { @Test fun `matcher allows unwrapped parameterized types as root types`() { - val match = matcher.match(createPotentialMatch("genericCustomUnwrappedType", unwrappedCustomType)) + val match = matchReturnType("genericCustomUnwrappedType", unwrappedCustomType) match as TypeClassMatcher.ValidMatch assertEquals(match.type, unwrappedCustomDefinition) val javatype = match.javaType as ParameterizedTypeImpl diff --git a/src/test/kotlin/graphql/kickstart/tools/returnvalue/ReturnValueUnwrappingTest.kt b/src/test/kotlin/graphql/kickstart/tools/returnvalue/ReturnValueUnwrappingTest.kt index aeedf198..6e3da8c0 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.TypeShape import org.junit.Assert.fail import org.junit.Test import java.io.ByteArrayOutputStream @@ -21,7 +22,10 @@ class ReturnValueUnwrappingTest { compile(classpath, dir, copy("Check.java", dir), copy("Fixtures.java", dir)) val args = System.getProperty("unwrapping.args", "").split(" ").filter { it.isNotBlank() } + - listOf("--known", copy("known-findings.txt", dir).toString()) + listOf( + "--known", copy("known-findings.txt", dir).toString(), + "--container-kinds", TypeShape.containerTypes.joinToString(",") { it.canonicalName } + ) val report = ByteArrayOutputStream() val passed = URLClassLoader(arrayOf(dir.toUri().toURL()), javaClass.classLoader).use { loader -> PrintStream(report, true, Charsets.UTF_8).use { out -> diff --git a/src/test/resources/returnvalue/Check.java b/src/test/resources/returnvalue/Check.java index fa7ede26..d445ef3a 100644 --- a/src/test/resources/returnvalue/Check.java +++ b/src/test/resources/returnvalue/Check.java @@ -312,6 +312,17 @@ public static boolean check(String[] args, String classpath, PrintStream out) th int show = intArg(args, "--show", 15); long started = System.nanoTime(); + Set built = Arrays.stream(Container.values()).map(it -> it.javaName).collect(Collectors.toSet()); + List unchecked = Arrays.stream(stringArg(args, "--container-kinds").split(",")) + .filter(it -> !built.contains(it)) + .collect(Collectors.toList()); + if (!unchecked.isEmpty()) { + out.println("MODEL ERROR: TypeShape.kt has a ContainerKind for these classes, but no Container in Check.java builds them, so a pass would prove nothing about them:"); + unchecked.forEach(it -> out.println(" " + it)); + out.println("Add a Container for each to Check.java with its value variants, and its plain value in build."); + return false; + } + List cases = cases(depth); Map> resolvers = compile(cases, classpath);