From cf3ea208312802fd43e5f91e77e3b9e69cb3d9a9 Mon Sep 17 00:00:00 2001 From: Oryan Date: Thu, 22 Oct 2026 19:51:13 -0400 Subject: [PATCH 1/4] Unwrap generic wrappers and futures nested in lists and other wrappers The schema build matches a return type with its generic wrappers unwrapped at every level, so List> builds against [Payload!]!, but the data fetcher only unwrapped the outermost value. Every query then failed, or a [String] field returned the wrapper's toString(). The data fetcher now unwraps the value at the positions where the build unwrapped its type. Lists are mapped, and futures below the outermost value are combined into one, so one failed element fails the whole field. Fields with no nested wrapper, subscriptions and custom scalars keep today's behaviour. Fixes #254 --- .../kickstart/tools/GenericTypeResolver.kt | 7 +- .../kickstart/tools/TypeClassMatcher.kt | 1 + .../tools/resolver/MethodFieldResolver.kt | 68 +---- .../tools/resolver/ReturnValueUnwrapper.kt | 245 +++++++++++++++ .../tools/NestedGenericWrapperTest.kt | 279 ++++++++++++++++++ 5 files changed, 540 insertions(+), 60 deletions(-) create mode 100644 src/main/kotlin/graphql/kickstart/tools/resolver/ReturnValueUnwrapper.kt create mode 100644 src/test/kotlin/graphql/kickstart/tools/NestedGenericWrapperTest.kt diff --git a/src/main/kotlin/graphql/kickstart/tools/GenericTypeResolver.kt b/src/main/kotlin/graphql/kickstart/tools/GenericTypeResolver.kt index 13da3e3f..52f29728 100644 --- a/src/main/kotlin/graphql/kickstart/tools/GenericTypeResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/GenericTypeResolver.kt @@ -28,8 +28,10 @@ internal class GenericTypeResolver(private val containingType: JavaType, private * - 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` + * + * [onUnwrap] is told each generic wrapper that is replaced, outermost first. */ - fun resolve(javaType: JavaType): JavaType { + fun resolve(javaType: JavaType, onUnwrap: ((wrapped: ParameterizedType) -> Unit)? = null): JavaType { return when (val type = resolveTypeVariables(javaType)) { is ParameterizedType -> { val rawType = type.rawType @@ -41,8 +43,9 @@ internal class GenericTypeResolver(private val containingType: JavaType, private throw IndexOutOfBoundsException("Generic type '${TypeUtils.toString(type)}' does not have a type argument at index ${wrapper.index}!") } + onUnwrap?.invoke(type) val unwrapsTo = wrapper.schemaWrapper.invoke(typeArguments[wrapper.index]) - resolve(unwrapsTo) + resolve(unwrapsTo, onUnwrap) } is TypeVariable<*> -> error("Could not resolve type variable '${TypeUtils.toLongString(type)}' relative to ${TypeUtils.toString(containingType)}") is WildcardType -> type.upperBounds.firstOrNull() diff --git a/src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt b/src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt index 1089e5db..778f4f46 100644 --- a/src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt +++ b/src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt @@ -21,6 +21,7 @@ internal class TypeClassMatcher(private val definitionsByName: Map 0 || isSuspendFunction) { // requires arguments and environment or is a suspend function - MethodFieldResolverDataFetcher(createSourceResolver(), this.method, args, options, isSuspendFunction, valueClass) + 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, options, valueClass) + LightMethodFieldResolverDataFetcher(createSourceResolver(), this.method, unwrapper) } } @@ -217,7 +218,7 @@ internal class MethodFieldResolverDataFetcher( private val args: List, private val options: SchemaParserOptions, private val isSuspendFunction: Boolean, - private val valueClass: Class<*>? + private val unwrapper: ReturnValueUnwrapper ) : DataFetcher { override fun get(environment: DataFetchingEnvironment): Any? { @@ -229,12 +230,10 @@ internal class MethodFieldResolverDataFetcher( // 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)?.transformWithGenericWrapper(options.genericWrappers) { environment } - .requireAwaitable(valueClass, method, isSuspendFunction = true) + invokeSuspend(source, method, args)?.let { unwrapper.unwrapSuspending(it) { environment } } } } else { - invoke(method, source, args)?.transformWithGenericWrapper(options.genericWrappers) { environment } - .requireAwaitable(valueClass, method, isSuspendFunction = false) + invoke(method, source, args)?.let { unwrapper.unwrap(it) { environment } } } } @@ -253,15 +252,13 @@ internal class MethodFieldResolverDataFetcher( internal class LightMethodFieldResolverDataFetcher( private val sourceResolver: SourceResolver, private val method: Method, - private val options: SchemaParserOptions, - private val valueClass: Class<*>? + private val unwrapper: ReturnValueUnwrapper ) : LightDataFetcher { override fun get(fieldDefinition: GraphQLFieldDefinition?, sourceObject: Any?, environmentSupplier: Supplier): Any? { val source = sourceResolver.resolve(null, sourceObject) - return invoke(method, source, emptyArray())?.transformWithGenericWrapper(options.genericWrappers, environmentSupplier) - .requireAwaitable(valueClass, method, isSuspendFunction = false) + return invoke(method, source, emptyArray())?.let { unwrapper.unwrap(it, environmentSupplier) } } override fun get(environment: DataFetchingEnvironment): Any? { @@ -277,51 +274,6 @@ internal class LightMethodFieldResolverDataFetcher( } } -private fun Any.transformWithGenericWrapper( - genericWrappers: List, - environmentSupplier: Supplier -): Any? { - val genericWrapper = genericWrappers - .asSequence() - .filter { it.type.isInstance(this) } - .sortedWith(CompareGenericWrappers) - .firstOrNull() - ?: return this - - // returned as is, even if null, so a transformer can map a value to null - return genericWrapper.transformer(this, environmentSupplier.get()) -} - -// 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 fun Any?.requireAwaitable(valueClass: Class<*>?, method: Method, isSuspendFunction: Boolean): Any? { - if (this !is Future<*> || valueClass?.isInstance(this) == true) { - return this - } - - val resolverName = "${method.declaringClass.name}.${method.name}" - if (isSuspendFunction) { - throw ResolverError("Suspend function '$resolverName' resolved to a ${javaClass.name}, which graphql-java can't wait on. Return its value instead.") - } - if (this !is CompletionStage<*>) { - throw ResolverError( - "Resolver method '$resolverName' resolved to a ${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." - ) - } - - return this -} - -private class CompareGenericWrappers { - companion object : Comparator { - override fun compare(w1: GenericWrapper, w2: GenericWrapper): Int = when { - w1.type.isAssignableFrom(w2.type) -> 1 - else -> -1 - } - } -} - private suspend inline fun invokeSuspend(target: Any, resolverMethod: Method, args: Array): Any? { return suspendCoroutineUninterceptedOrReturn { continuation -> invoke(resolverMethod, target, args + continuation) diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/ReturnValueUnwrapper.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/ReturnValueUnwrapper.kt new file mode 100644 index 00000000..f3059850 --- /dev/null +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/ReturnValueUnwrapper.kt @@ -0,0 +1,245 @@ +package graphql.kickstart.tools.resolver + +import graphql.execution.DataFetcherResult +import graphql.kickstart.tools.GenericTypeResolver +import graphql.kickstart.tools.ResolverError +import graphql.kickstart.tools.SchemaParserOptions.GenericWrapper +import graphql.kickstart.tools.util.GraphQLLangType +import graphql.kickstart.tools.util.JavaType +import graphql.kickstart.tools.util.isSubtypeOf +import graphql.kickstart.tools.util.unwrap +import graphql.language.ListType +import graphql.language.NonNullType +import graphql.schema.DataFetchingEnvironment +import graphql.schema.idl.ScalarInfo +import kotlinx.coroutines.future.asDeferred +import java.lang.reflect.Method +import java.lang.reflect.ParameterizedType +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 + +/** + * Turns what a resolver method returned into the value graphql-java completes its field with. + * + * The schema build matches the field's GraphQL type against the return type with generic wrappers unwrapped at every + * level, e.g. `List>` against `[Item]`. graphql-java only waits on a CompletionStage that the data fetcher + * returns and doesn't know generic wrappers, so the value is unwrapped at the same positions, and futures below the + * outermost value are combined into one: one failed element fails the whole field. + */ +internal class ReturnValueUnwrapper private constructor( + private val genericWrappers: List, + private val method: Method, + private val valueClass: Class<*>?, + /** null for most fields: nothing is unwrapped below the outermost value */ + private val nested: Step? +) { + + fun unwrap(value: Any, environmentSupplier: Supplier): Any? = + value.transformWithGenericWrapper(genericWrappers, environmentSupplier) + .requireAwaitable(valueClass, method, isSuspendFunction = false) + .then(nested, environmentSupplier) + + suspend fun unwrapSuspending(value: Any, environmentSupplier: Supplier): Any? { + val unwrapped = value.transformWithGenericWrapper(genericWrappers, environmentSupplier) + .requireAwaitable(valueClass, method, isSuspendFunction = true) + .then(nested, 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 { + /** + * @param returnType the declared return type with type variables resolved, generic wrappers kept + * @param valueClass the class the outermost value unwraps to, see [requireAwaitable] + */ + fun create( + method: Method, + returnType: JavaType, + valueClass: Class<*>?, + fieldType: GraphQLLangType, + isSubscription: Boolean, + typeResolver: GenericTypeResolver, + genericWrappers: List, + scalarNames: Set + ): ReturnValueUnwrapper { + val nested = when { + // graphql-java completes a subscription's events one at a time + isSubscription -> null + !hasGenericWrapperBelow(returnType, genericWrappers) -> null + // a custom scalar's Coercing may take the wrapper itself, e.g. a JSON scalar + fieldType.unwrap().name.let { it in scalarNames && it !in ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS } -> null + else -> nestedSteps(fieldType, returnType, typeResolver, genericWrappers) + } + return ReturnValueUnwrapper(genericWrappers, method, valueClass, nested) + } + } +} + +/** A position below the outermost value where the schema build unwrapped something. */ +private sealed interface Step { + /** The value unwrapped, or a CompletionStage of it. A value of another shape than expected, null included, is kept. */ + fun unwrap(value: Any?, environmentSupplier: Supplier): Any? +} + +/** A generic wrapper, e.g. each `Try` of `List>`. Every chain of steps ends in one. */ +private class Peel(private val genericWrappers: List, private val next: Step?) : Step { + override fun unwrap(value: Any?, environmentSupplier: Supplier): Any? = + value?.transformWithGenericWrapper(genericWrappers, environmentSupplier).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? = + if (value is Optional<*>) present.unwrap(value.orElse(null), environmentSupplier) else value +} + +/** What the schema build unwraps in a return type. */ +private sealed interface Unwrapped { + class Wrapper(rawType: Class<*>) : Unwrapped { + val isFuture = Future::class.java.isAssignableFrom(rawType) || CompletionStage::class.java.isAssignableFrom(rawType) + } + + object Data : Unwrapped + object Present : Unwrapped + object Elements : Unwrapped +} + +private fun nestedSteps(fieldType: GraphQLLangType, returnType: JavaType, typeResolver: GenericTypeResolver, genericWrappers: List): Step? { + val unwrapped = mutableListOf() + val onUnwrap = { wrapped: ParameterizedType -> unwrapped += Unwrapped.Wrapper(wrapped.rawType as Class<*>) } + + // TypeClassMatcher.match without its errors, which the build has thrown already: generic wrappers, then a + // DataFetcherResult, then an Optional, then the GraphQL type: NonNull matches the same Java type again, a list goes + // on with the Iterable's elements, anything else ends the walk + fun addLevel(graphQLType: GraphQLLangType, javaType: JavaType) { + var type = typeResolver.resolve(javaType, onUnwrap) + if (type is ParameterizedType && type.isSubtypeOf(DataFetcherResult::class.java)) { + unwrapped += Unwrapped.Data + type = typeResolver.resolve(type.actualTypeArguments.first(), onUnwrap) + } + if (type is ParameterizedType && type.isSubtypeOf(Optional::class.java)) { + unwrapped += Unwrapped.Present + type = typeResolver.resolve(type.actualTypeArguments.first(), onUnwrap) + } + when (graphQLType) { + is NonNullType -> addLevel(graphQLType.type, type) + is ListType -> if (type is ParameterizedType && type.isSubtypeOf(Iterable::class.java)) { + unwrapped += Unwrapped.Elements + addLevel(graphQLType.type, type.actualTypeArguments.first()) + } + else -> Unit + } + } + + // MethodFieldResolver.scanForMatches matches the resolved return type + addLevel(fieldType, typeResolver.resolve(returnType, onUnwrap)) + val awaited = unwrapped.filterIndexed { i, it -> + // a future returned by the wrapper before it is waited on by whatever takes that wrapper's result, + // e.g. graphql-java for `Try>` + !(it is Unwrapped.Wrapper && it.isFuture && (unwrapped.getOrNull(i - 1) as? Unwrapped.Wrapper)?.isFuture == false) + } + // the outermost value is unwrapped by the data fetcher itself + val belowOutermost = if (awaited.firstOrNull() is Unwrapped.Wrapper) awaited.drop(1) else awaited + // only a generic wrapper needs a step of its own, the others only lead to one + return belowOutermost.foldRight(null as Step?) { it, next -> + when (it) { + is Unwrapped.Wrapper -> Peel(genericWrappers, next) + Unwrapped.Data -> next?.let(::DataOf) + Unwrapped.Present -> next?.let(::ValueOf) + Unwrapped.Elements -> next?.let(::EachElement) + } + } +} + +// without one, nothing below the outermost value can be unwrapped (unless a schemaWrapper adds one), so most fields +// skip the walk +private fun hasGenericWrapperBelow(type: JavaType, genericWrappers: List): Boolean = + type is ParameterizedType && type.actualTypeArguments.any { it.mentionsGenericWrapper(genericWrappers) } + +private fun JavaType.mentionsGenericWrapper(genericWrappers: List): Boolean = when (this) { + is ParameterizedType -> genericWrappers.any { it.type == rawType } || actualTypeArguments.any { it.mentionsGenericWrapper(genericWrappers) } + is WildcardType -> upperBounds.any { it.mentionsGenericWrapper(genericWrappers) } + else -> false +} + +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 Any.transformWithGenericWrapper( + genericWrappers: List, + environmentSupplier: Supplier +): Any? { + val genericWrapper = genericWrappers + .asSequence() + .filter { it.type.isInstance(this) } + .sortedWith(CompareGenericWrappers) + .firstOrNull() + ?: return this + + // returned as is, even if null, so a transformer can map a value to null + return genericWrapper.transformer(this, environmentSupplier.get()) +} + +// 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 fun Any?.requireAwaitable(valueClass: Class<*>?, method: Method, isSuspendFunction: Boolean): Any? { + if (this !is Future<*> || valueClass?.isInstance(this) == true) { + return this + } + + val resolverName = "${method.declaringClass.name}.${method.name}" + if (isSuspendFunction) { + throw ResolverError("Suspend function '$resolverName' resolved to a ${javaClass.name}, which graphql-java can't wait on. Return its value instead.") + } + if (this !is CompletionStage<*>) { + throw ResolverError( + "Resolver method '$resolverName' resolved to a ${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." + ) + } + + return this +} + +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/test/kotlin/graphql/kickstart/tools/NestedGenericWrapperTest.kt b/src/test/kotlin/graphql/kickstart/tools/NestedGenericWrapperTest.kt new file mode 100644 index 00000000..cf48fdbb --- /dev/null +++ b/src/test/kotlin/graphql/kickstart/tools/NestedGenericWrapperTest.kt @@ -0,0 +1,279 @@ +package graphql.kickstart.tools + +import graphql.ExecutionInput +import graphql.ExecutionResult +import graphql.GraphQL +import graphql.GraphQLContext +import graphql.GraphQLError +import graphql.execution.DataFetcherResult +import graphql.execution.DefaultValueUnboxer +import graphql.kickstart.tools.SchemaParserOptions.GenericWrapper +import graphql.schema.Coercing +import graphql.schema.DataFetchingEnvironment +import graphql.schema.DataFetchingEnvironmentImpl +import graphql.schema.FieldCoordinates +import graphql.schema.GraphQLScalarType +import graphql.schema.GraphQLSchema +import org.dataloader.BatchLoader +import org.dataloader.DataLoaderFactory +import org.dataloader.DataLoaderRegistry +import org.junit.Assert.assertSame +import org.junit.Test +import java.util.* +import java.util.concurrent.CompletableFuture +import java.util.concurrent.CompletableFuture.completedFuture +import java.util.concurrent.Future +import java.util.concurrent.FutureTask + +class NestedGenericWrapperTest { + + private val schema = schema(Query()) + + private val ab = listOf(mapOf("data" to "a"), mapOf("data" to "b")) + + @Test + fun `unwraps wrappers in a list`() = assertResolves("listOfWrapped", ab) + + @Test + fun `waits on futures in a list`() = assertResolves("listOfFutures", ab) + + @Test + fun `unwraps wrappers in a list in a future`() = assertResolves("futureOfListOfWrapped", ab) + + @Test + fun `unwraps scalar wrappers in a list`() = assertResolves("listOfWrappedStrings", listOf("a", "b"), selection = "") + + @Test + fun `waits on scalar futures in a list`() = assertResolves("listOfFutureStrings", listOf("a", "b"), selection = "") + + @Test + fun `keeps the order of futures that complete in another order`() = assertResolves("futuresCompletingInReverse", listOf("1", "2", "3"), selection = "") + + @Test + fun `unwraps wrappers in nested lists`() = assertResolves("listOfListsOfWrapped", listOf(listOf(mapOf("data" to "a")), listOf(mapOf("data" to "b")))) + + @Test + fun `unwraps wrappers in a list in a wrapper`() = assertResolves("wrappedListOfWrapped", ab) + + @Test + fun `unwraps wrappers in a list collection wrapper`() = assertResolves("pageOfWrapped", ab) + + @Test + fun `unwraps wrappers in a list in a DataFetcherResult and keeps its errors`() { + val result = execute("resultOfListOfWrapped") + + assertEquals(result.errors.map { it.message }, listOf("partial")) + assertEquals(result.getData(), mapOf("resultOfListOfWrapped" to ab)) + } + + @Test + fun `unwraps wrappers in DataFetcherResults in a list`() = assertResolves("listOfResultsOfWrapped", ab) + + @Test + fun `unwraps wrappers in a list in an Optional`() = assertResolves("optionalListOfWrapped", ab) + + @Test + fun `unwraps wrappers in a list from a suspend function`() = assertResolves("suspendListOfWrapped", ab) + + @Test + fun `waits on futures in a list from a suspend function`() = assertResolves("suspendListOfFutures", ab) + + @Test + fun `keeps null elements and null transformer results in a list`() = assertResolves("listOfNullableWrapped", listOf(mapOf("data" to "a"), null, null)) + + @Test + fun `waits on a future in a future`() = assertResolves("futureOfFuture", mapOf("data" to "a")) + + @Test + fun `waits on a future in an Optional`() = assertResolves("optionalFuture", mapOf("data" to "a")) + + @Test + fun `waits on futures in wrappers in a list`() = assertResolves("listOfWrappedFutures", ab) + + @Test + fun `waits on DataLoader futures in a list`() = assertResolves("loadedPayloads", ab) + + @Test + fun `fails the list when one of its futures fails`() = assertFails("listWithFailedFuture", "failed future") + + @Test + fun `fails the list when one of its wrappers fails`() = assertFails("listWithFailedWrapper", "failed wrapper") + + @Test + fun `fails the list when one of its wrappers in a future fails`() = assertFails("futureOfListWithFailedWrapper", "failed wrapper") + + @Test + fun `leaves futures in a list that graphql-java can't wait on as they are`() { + val result = execute("listOfPlainFutures") + + assertEquals(result.errors.map { it.message }, listOf( + "Exception while fetching data (/listOfPlainFutures[0]/data) : " + + "Expected source object to be an instance of '${Payload::class.java.name}' but instead got '${FutureTask::class.java.name}'" + )) + } + + @Test + fun `gives a custom scalar the wrappers as they are`() { + val json = mutableListOf() + val schema = schema(Query(), jsonScalar(json)) + + assertEquals(execute("json", schema = schema, selection = "").errors, listOf()) + assertEquals(execute("futureJson", schema = schema, selection = "").errors, listOf()) + assertEquals(json.map { it?.javaClass }, listOf(Try::class.java, Try::class.java, Try::class.java)) + } + + @Test + fun `returns the resolver's own value when nothing is nested`() { + val query = Query() + val schema = schema(query) + + assertSame(fetch(schema, "plainList"), query.plainList) + assertSame(fetch(schema, "futureOfList"), query.futureOfList) + assertSame(fetch(schema, "wrappedFuture"), query.wrappedFuture.get()) + } + + @Test + fun `gives a ValueUnboxer the unwrapped values`() { + val gql = GraphQL.newGraphQL(schema) + .valueUnboxer { value -> (if (value is Try<*>) value.get() else value)?.let { DefaultValueUnboxer.unboxValue(it) } } + .build() + + assertEquals(execute("listOfWrapped", gql = gql).getData(), mapOf("listOfWrapped" to ab)) + assertEquals(execute("listOfWrappedStrings", gql = gql, selection = "").getData(), mapOf("listOfWrappedStrings" to listOf("a", "b"))) + assertEquals(execute("listWithFailedWrapper", gql = gql).errors.map { it.message }, listOf("Exception while fetching data (/listWithFailedWrapper) : failed wrapper")) + } + + private fun schema(query: Query, vararg scalars: GraphQLScalarType): GraphQLSchema = SchemaParser.newParser() + .schemaString( + """ + type Query { + listOfWrapped: [Payload!]! + listOfFutures: [Payload!]! + futureOfListOfWrapped: [Payload!]! + listOfWrappedStrings: [String!]! + listOfFutureStrings: [String] + futuresCompletingInReverse: [String!]! + listOfListsOfWrapped: [[Payload!]!]! + wrappedListOfWrapped: [Payload!]! + pageOfWrapped: [Payload!]! + resultOfListOfWrapped: [Payload!]! + listOfResultsOfWrapped: [Payload!]! + optionalListOfWrapped: [Payload!] + suspendListOfWrapped: [Payload!]! + suspendListOfFutures: [Payload!]! + listOfNullableWrapped: [Payload]! + futureOfFuture: Payload! + optionalFuture: Payload + listOfWrappedFutures: [Payload!]! + loadedPayloads: [Payload!]! + listWithFailedFuture: [Payload!] + listWithFailedWrapper: [Payload] + futureOfListWithFailedWrapper: [Payload] + listOfPlainFutures: [Payload] + plainList: [Payload!]! + futureOfList: [Payload!]! + wrappedFuture: Payload! + ${if (scalars.isEmpty()) "" else "json: [JSON] futureJson: JSON"} + } + + type Payload { + data: String! + } + + ${if (scalars.isEmpty()) "" else "scalar JSON"} + """) + .resolvers(query) + .scalars(*scalars) + .options(SchemaParserOptions.newOptions().genericWrappers(tryWrapper, pageWrapper).build()) + .build() + .makeExecutableSchema() + + private fun execute(field: String, selection: String = "{ data }", schema: GraphQLSchema = this.schema, gql: GraphQL = GraphQL.newGraphQL(schema).build()): ExecutionResult { + val registry = DataLoaderRegistry().apply { + register("payloads", DataLoaderFactory.newDataLoader(BatchLoader { keys -> completedFuture(keys.map { Payload(it) }) })) + } + return gql.execute(ExecutionInput.newExecutionInput().query("query { $field $selection }").dataLoaderRegistry(registry)) + } + + private fun assertResolves(field: String, expected: Any?, selection: String = "{ data }") { + val result = execute(field, selection) + assertEquals(result.errors, listOf()) + assertEquals(result.getData(), mapOf(field to expected)) + } + + private fun assertFails(field: String, message: String) { + val result = execute(field) + assertEquals(result.errors.map { it.message }, listOf("Exception while fetching data (/$field) : $message")) + assertEquals(result.getData(), mapOf(field to null)) + } + + private fun fetch(schema: GraphQLSchema, field: String): Any? { + val definition = schema.queryType.getFieldDefinition(field) + val environment = DataFetchingEnvironmentImpl.newDataFetchingEnvironment().fieldDefinition(definition).build() + return schema.codeRegistry.getDataFetcher(FieldCoordinates.coordinates("Query", field), definition).get(environment) + } + + private fun jsonScalar(serialized: MutableList): GraphQLScalarType = GraphQLScalarType.newScalar() + .name("JSON") + .coercing(object : Coercing { + override fun serialize(dataFetcherResult: Any, graphQLContext: GraphQLContext, locale: Locale): Any = dataFetcherResult.also { serialized += it } + }) + .build() + + class Try private constructor(private val value: T?, private val failure: Throwable?) { + fun get(): T? = if (failure != null) throw failure else value + + companion object { + fun success(value: T?) = Try(value, null) + fun failure(message: String) = Try(null, IllegalStateException(message)) + } + } + + class Page(val content: List) + + data class Payload(val data: String) + + class Query : GraphQLQueryResolver { + val plainList = listOf(Payload("a")) + val futureOfList: CompletableFuture> = completedFuture(plainList) + val wrappedFuture: Try> = Try.success(completedFuture(Payload("a"))) + + private fun result(data: T, vararg errors: GraphQLError): DataFetcherResult = DataFetcherResult.newResult().data(data).errors(errors.toList()).build() + + fun listOfWrapped(): List> = listOf(Try.success(Payload("a")), Try.success(Payload("b"))) + fun listOfFutures(): List> = listOf(completedFuture(Payload("a")), CompletableFuture.supplyAsync { Payload("b") }) + fun futureOfListOfWrapped(): CompletableFuture>> = CompletableFuture.supplyAsync { listOfWrapped() } + fun listOfWrappedStrings(): List> = listOf(Try.success("a"), Try.success("b")) + fun listOfFutureStrings(): List> = listOf(completedFuture("a"), CompletableFuture.supplyAsync { "b" }) + fun futuresCompletingInReverse(): List> { + val futures = List(3) { CompletableFuture() } + Thread { futures.indices.reversed().forEach { futures[it].complete("${it + 1}") } }.start() + return futures + } + fun listOfListsOfWrapped(): List>> = listOf(listOf(Try.success(Payload("a"))), listOf(Try.success(Payload("b")))) + fun wrappedListOfWrapped(): Try>> = Try.success(listOfWrapped()) + fun pageOfWrapped(): Page> = Page(listOfWrapped()) + fun resultOfListOfWrapped(): DataFetcherResult>> = result(listOfWrapped(), GraphQLError.newError().message("partial").build()) + fun listOfResultsOfWrapped(): List>> = listOfWrapped().map { result(it) } + fun optionalListOfWrapped(): Optional>> = Optional.of(listOfWrapped()) + suspend fun suspendListOfWrapped(): List> = listOfWrapped() + suspend fun suspendListOfFutures(): List> = listOfFutures() + fun listOfNullableWrapped(): List?> = listOf(Try.success(Payload("a")), Try.success(null), null) + fun futureOfFuture(): CompletableFuture> = CompletableFuture.supplyAsync { completedFuture(Payload("a")) } + fun optionalFuture(): Optional> = Optional.of(CompletableFuture.supplyAsync { Payload("a") }) + fun listOfWrappedFutures(): List>> = listOfFutures().map { Try.success(it) } + fun loadedPayloads(env: DataFetchingEnvironment): List> = + listOf("a", "b").map { env.getDataLoader("payloads")!!.load(it) } + fun listWithFailedFuture(): List> = listOf(completedFuture(Payload("a")), CompletableFuture.failedFuture(IllegalStateException("failed future"))) + fun listWithFailedWrapper(): List> = listOf(Try.success(Payload("a")), Try.failure("failed wrapper")) + fun futureOfListWithFailedWrapper(): CompletableFuture>> = CompletableFuture.supplyAsync { listWithFailedWrapper() } + fun listOfPlainFutures(): List> = listOf(FutureTask { Payload("a") }.apply { run() }) + fun json(): List>> = listOf(Try.success(mapOf("a" to 1)), Try.success(mapOf("b" to 2))) + fun futureJson(): CompletableFuture>> = completedFuture(Try.success(mapOf("c" to 3))) + } + + companion object { + private val tryWrapper = GenericWrapper.withTransformer(Try::class, 0, { wrapper -> wrapper.get() }) + private val pageWrapper = GenericWrapper.listCollectionWithTransformer(Page::class, 0, { page -> page.content }) + } +} From f812c12d228ac0b4e5edd2e85dcb76c585b6e8e2 Mon Sep 17 00:00:00 2001 From: Oryan Date: Thu, 22 Oct 2026 20:38:46 -0400 Subject: [PATCH 2/4] Return from the wrapper search before it allocates when no wrapper matches Behind ReturnValueUnwrapper the JIT stopped optimizing away the search's sequence, which cost about 40 bytes per fetch. Also shares the outermost unwrap between the suspend and non-suspend paths and drops comments the code already says. --- .../kickstart/tools/GenericTypeResolver.kt | 2 - .../kickstart/tools/TypeClassMatcher.kt | 1 - .../tools/resolver/ReturnValueUnwrapper.kt | 53 ++++++------------- 3 files changed, 16 insertions(+), 40 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/GenericTypeResolver.kt b/src/main/kotlin/graphql/kickstart/tools/GenericTypeResolver.kt index 52f29728..b519624e 100644 --- a/src/main/kotlin/graphql/kickstart/tools/GenericTypeResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/GenericTypeResolver.kt @@ -28,8 +28,6 @@ internal class GenericTypeResolver(private val containingType: JavaType, private * - 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` - * - * [onUnwrap] is told each generic wrapper that is replaced, outermost first. */ fun resolve(javaType: JavaType, onUnwrap: ((wrapped: ParameterizedType) -> Unit)? = null): JavaType { return when (val type = resolveTypeVariables(javaType)) { diff --git a/src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt b/src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt index 778f4f46..1089e5db 100644 --- a/src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt +++ b/src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt @@ -21,7 +21,6 @@ internal class TypeClassMatcher(private val definitionsByName: Map>` against `[Item]`. graphql-java only waits on a CompletionStage that the data fetcher - * returns and doesn't know generic wrappers, so the value is unwrapped at the same positions, and futures below the - * outermost value are combined into one: one failed element fails the whole field. - */ internal class ReturnValueUnwrapper private constructor( private val genericWrappers: List, private val method: Method, private val valueClass: Class<*>?, - /** null for most fields: nothing is unwrapped below the outermost value */ private val nested: Step? ) { fun unwrap(value: Any, environmentSupplier: Supplier): Any? = - value.transformWithGenericWrapper(genericWrappers, environmentSupplier) - .requireAwaitable(valueClass, method, isSuspendFunction = false) - .then(nested, environmentSupplier) + unwrap(value, environmentSupplier, isSuspendFunction = false) suspend fun unwrapSuspending(value: Any, environmentSupplier: Supplier): Any? { - val unwrapped = value.transformWithGenericWrapper(genericWrappers, environmentSupplier) - .requireAwaitable(valueClass, method, isSuspendFunction = true) - .then(nested, environmentSupplier) + val unwrapped = unwrap(value, environmentSupplier, isSuspendFunction = true) // 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 } + private fun unwrap(value: Any, environmentSupplier: Supplier, isSuspendFunction: Boolean): Any? = + value.transformWithGenericWrapper(genericWrappers, environmentSupplier) + .requireAwaitable(valueClass, method, isSuspendFunction) + .then(nested, environmentSupplier) + companion object { - /** - * @param returnType the declared return type with type variables resolved, generic wrappers kept - * @param valueClass the class the outermost value unwraps to, see [requireAwaitable] - */ fun create( method: Method, returnType: JavaType, @@ -71,7 +59,6 @@ internal class ReturnValueUnwrapper private constructor( // graphql-java completes a subscription's events one at a time isSubscription -> null !hasGenericWrapperBelow(returnType, genericWrappers) -> null - // a custom scalar's Coercing may take the wrapper itself, e.g. a JSON scalar fieldType.unwrap().name.let { it in scalarNames && it !in ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS } -> null else -> nestedSteps(fieldType, returnType, typeResolver, genericWrappers) } @@ -80,13 +67,10 @@ internal class ReturnValueUnwrapper private constructor( } } -/** A position below the outermost value where the schema build unwrapped something. */ private sealed interface Step { - /** The value unwrapped, or a CompletionStage of it. A value of another shape than expected, null included, is kept. */ fun unwrap(value: Any?, environmentSupplier: Supplier): Any? } -/** A generic wrapper, e.g. each `Try` of `List>`. Every chain of steps ends in one. */ private class Peel(private val genericWrappers: List, private val next: Step?) : Step { override fun unwrap(value: Any?, environmentSupplier: Supplier): Any? = value?.transformWithGenericWrapper(genericWrappers, environmentSupplier).then(next, environmentSupplier) @@ -112,7 +96,6 @@ private class ValueOf(private val present: Step) : Step { if (value is Optional<*>) present.unwrap(value.orElse(null), environmentSupplier) else value } -/** What the schema build unwraps in a return type. */ private sealed interface Unwrapped { class Wrapper(rawType: Class<*>) : Unwrapped { val isFuture = Future::class.java.isAssignableFrom(rawType) || CompletionStage::class.java.isAssignableFrom(rawType) @@ -127,9 +110,6 @@ private fun nestedSteps(fieldType: GraphQLLangType, returnType: JavaType, typeRe val unwrapped = mutableListOf() val onUnwrap = { wrapped: ParameterizedType -> unwrapped += Unwrapped.Wrapper(wrapped.rawType as Class<*>) } - // TypeClassMatcher.match without its errors, which the build has thrown already: generic wrappers, then a - // DataFetcherResult, then an Optional, then the GraphQL type: NonNull matches the same Java type again, a list goes - // on with the Iterable's elements, anything else ends the walk fun addLevel(graphQLType: GraphQLLangType, javaType: JavaType) { var type = typeResolver.resolve(javaType, onUnwrap) if (type is ParameterizedType && type.isSubtypeOf(DataFetcherResult::class.java)) { @@ -150,16 +130,12 @@ private fun nestedSteps(fieldType: GraphQLLangType, returnType: JavaType, typeRe } } - // MethodFieldResolver.scanForMatches matches the resolved return type + fun isFutureReturnedByWrapperBefore(i: Int) = + (unwrapped[i] as? Unwrapped.Wrapper)?.isFuture == true && (unwrapped.getOrNull(i - 1) as? Unwrapped.Wrapper)?.isFuture == false + addLevel(fieldType, typeResolver.resolve(returnType, onUnwrap)) - val awaited = unwrapped.filterIndexed { i, it -> - // a future returned by the wrapper before it is waited on by whatever takes that wrapper's result, - // e.g. graphql-java for `Try>` - !(it is Unwrapped.Wrapper && it.isFuture && (unwrapped.getOrNull(i - 1) as? Unwrapped.Wrapper)?.isFuture == false) - } - // the outermost value is unwrapped by the data fetcher itself + val awaited = unwrapped.filterIndexed { i, _ -> !isFutureReturnedByWrapperBefore(i) } val belowOutermost = if (awaited.firstOrNull() is Unwrapped.Wrapper) awaited.drop(1) else awaited - // only a generic wrapper needs a step of its own, the others only lead to one return belowOutermost.foldRight(null as Step?) { it, next -> when (it) { is Unwrapped.Wrapper -> Peel(genericWrappers, next) @@ -170,8 +146,6 @@ private fun nestedSteps(fieldType: GraphQLLangType, returnType: JavaType, typeRe } } -// without one, nothing below the outermost value can be unwrapped (unless a schemaWrapper adds one), so most fields -// skip the walk private fun hasGenericWrapperBelow(type: JavaType, genericWrappers: List): Boolean = type is ParameterizedType && type.actualTypeArguments.any { it.mentionsGenericWrapper(genericWrappers) } @@ -203,6 +177,11 @@ private fun Any.transformWithGenericWrapper( genericWrappers: List, environmentSupplier: Supplier ): Any? { + // 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 (genericWrappers.none { it.type.isInstance(this) }) { + return this + } val genericWrapper = genericWrappers .asSequence() .filter { it.type.isInstance(this) } From 77e8f5d4f09e432af64bc3134435af08b582a226 Mon Sep 17 00:00:00 2001 From: Oryan Date: Fri, 23 Oct 2026 07:16:17 -0400 Subject: [PATCH 3/4] Test that every return type the matcher accepts resolves at runtime --- .../tools/NestedGenericWrapperTest.kt | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/src/test/kotlin/graphql/kickstart/tools/NestedGenericWrapperTest.kt b/src/test/kotlin/graphql/kickstart/tools/NestedGenericWrapperTest.kt index cf48fdbb..fb9e819a 100644 --- a/src/test/kotlin/graphql/kickstart/tools/NestedGenericWrapperTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/NestedGenericWrapperTest.kt @@ -6,18 +6,36 @@ import graphql.GraphQL import graphql.GraphQLContext import graphql.GraphQLError 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.util.ParameterizedTypeImpl +import graphql.language.AstPrinter +import graphql.language.ListType +import graphql.language.NonNullType +import graphql.language.ObjectTypeDefinition +import graphql.language.Type +import graphql.language.TypeName import graphql.schema.Coercing +import graphql.schema.DataFetcher import graphql.schema.DataFetchingEnvironment import graphql.schema.DataFetchingEnvironmentImpl import graphql.schema.FieldCoordinates +import graphql.schema.GraphQLCodeRegistry +import graphql.schema.GraphQLFieldDefinition +import graphql.schema.GraphQLList +import graphql.schema.GraphQLNonNull +import graphql.schema.GraphQLObjectType +import graphql.schema.GraphQLOutputType import graphql.schema.GraphQLScalarType import graphql.schema.GraphQLSchema import org.dataloader.BatchLoader import org.dataloader.DataLoaderFactory import org.dataloader.DataLoaderRegistry +import org.apache.commons.lang3.reflect.TypeUtils import org.junit.Assert.assertSame +import org.junit.Assert.assertTrue import org.junit.Test import java.util.* import java.util.concurrent.CompletableFuture @@ -143,6 +161,69 @@ class NestedGenericWrapperTest { assertEquals(execute("listWithFailedWrapper", gql = gql).errors.map { it.message }, listOf("Exception while fetching data (/listWithFailedWrapper) : failed wrapper")) } + @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(mapOf("Payload" to ObjectTypeDefinition("Payload"))) + val method = Query::class.java.getMethod("listOfWrapped") + val payload = TypeName("Payload") + val graphQLTypes = listOf(payload, NonNullType(payload), ListType(payload), NonNullType(ListType(NonNullType(payload))), ListType(ListType(payload))) + val containers = listOf(List::class.java, CompletableFuture::class.java, Try::class.java, Page::class.java, DataFetcherResult::class.java, Optional::class.java) + val shapes = (1..4).fold(listOf(listOf>())) { shapes, _ -> shapes + shapes.filter { it.size == shapes.last().size }.flatMap { shape -> containers.map { shape + it } } } + + val accepted = shapes.flatMap { shape -> graphQLTypes.map { shape to it } }.filter { (shape, graphQLType) -> + val match = try { + matcher.match(TypeClassMatcher.PotentialMatch.returnValue(graphQLType, typeResolver.resolve(javaType(shape)), typeResolver, SchemaClassScanner.ReturnValueReference(method))) + } catch (e: SchemaClassScannerError) { + null + } + (match as? TypeClassMatcher.ValidMatch)?.javaType == Payload::class.java + } + val failures = accepted.mapNotNull { (shape, graphQLType) -> + val unwrapper = ReturnValueUnwrapper.create( + method, javaType(shape), TypeUtils.getRawType(typeResolver.resolve(javaType(shape)), null), graphQLType, false, typeResolver, options.genericWrappers, emptySet() + ) + val result = GraphQL.newGraphQL(fieldSchema(graphQLType) { environment -> unwrapper.unwrap(value(shape)) { environment } }).build().execute("query { field { data } }") + "${TypeUtils.toString(javaType(shape))} as ${AstPrinter.printAst(graphQLType)}: ${result.errors.map { it.message }} ${result.getData()}" + .takeUnless { result.errors.isEmpty() && result.getData() == mapOf("field" to expected(graphQLType)) } + } + + assertTrue(accepted.size > graphQLTypes.size) + assertEquals(failures, listOf()) + } + + private fun javaType(shape: List>): java.lang.reflect.Type = + shape.foldRight(Payload::class.java as java.lang.reflect.Type) { container, inner -> ParameterizedTypeImpl(container, arrayOf(inner), null) } + + private fun value(shape: List>): Any = shape.foldRight(Payload("a") as Any) { container, inner -> + when (container) { + List::class.java -> listOf(inner, inner) + CompletableFuture::class.java -> CompletableFuture.supplyAsync { inner } + Try::class.java -> Try.success(inner) + Page::class.java -> Page(listOf(inner, inner)) + DataFetcherResult::class.java -> DataFetcherResult.newResult().data(inner).build() + else -> Optional.of(inner) + } + } + + private fun expected(graphQLType: Type<*>): Any = when (graphQLType) { + is ListType -> List(2) { expected(graphQLType.type) } + is NonNullType -> expected(graphQLType.type) + else -> mapOf("data" to "a") + } + + private fun fieldSchema(graphQLType: Type<*>, dataFetcher: DataFetcher<*>): GraphQLSchema = GraphQLSchema.newSchema() + .query(GraphQLObjectType.newObject().name("Query").field(GraphQLFieldDefinition.newFieldDefinition().name("field").type(outputType(graphQLType)))) + .codeRegistry(GraphQLCodeRegistry.newCodeRegistry().dataFetcher(FieldCoordinates.coordinates("Query", "field"), dataFetcher).build()) + .build() + + private fun outputType(graphQLType: Type<*>): GraphQLOutputType = when (graphQLType) { + is ListType -> GraphQLList.list(outputType(graphQLType.type)) + is NonNullType -> GraphQLNonNull.nonNull(outputType(graphQLType.type)) + else -> payloadType + } + private fun schema(query: Query, vararg scalars: GraphQLScalarType): GraphQLSchema = SchemaParser.newParser() .schemaString( """ @@ -275,5 +356,9 @@ class NestedGenericWrapperTest { companion object { private val tryWrapper = GenericWrapper.withTransformer(Try::class, 0, { wrapper -> wrapper.get() }) private val pageWrapper = GenericWrapper.listCollectionWithTransformer(Page::class, 0, { page -> page.content }) + private val payloadType = GraphQLObjectType.newObject() + .name("Payload") + .field(GraphQLFieldDefinition.newFieldDefinition().name("data").type(GraphQLNonNull.nonNull(Scalars.GraphQLString))) + .build() } } From 1444ba4b828e762f872d81bb75de1f5545d7fe80 Mon Sep 17 00:00:00 2001 From: Oryan Date: Fri, 23 Oct 2026 09:16:47 -0400 Subject: [PATCH 4/4] Unwrap generic wrappers under a wildcard's upper bound --- .../kickstart/tools/GenericTypeResolver.kt | 8 +++-- .../tools/NestedGenericWrapperTest.kt | 29 ++++++++++++++----- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/GenericTypeResolver.kt b/src/main/kotlin/graphql/kickstart/tools/GenericTypeResolver.kt index b519624e..41e2fde9 100644 --- a/src/main/kotlin/graphql/kickstart/tools/GenericTypeResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/GenericTypeResolver.kt @@ -46,8 +46,12 @@ internal class GenericTypeResolver(private val containingType: JavaType, private resolve(unwrapsTo, onUnwrap) } is TypeVariable<*> -> error("Could not resolve type variable '${TypeUtils.toLongString(type)}' relative to ${TypeUtils.toString(containingType)}") - is WildcardType -> type.upperBounds.firstOrNull() - ?: error("Unable to unwrap type, wildcard has no upper bound: $type") + // 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") } diff --git a/src/test/kotlin/graphql/kickstart/tools/NestedGenericWrapperTest.kt b/src/test/kotlin/graphql/kickstart/tools/NestedGenericWrapperTest.kt index fb9e819a..2a935dfb 100644 --- a/src/test/kotlin/graphql/kickstart/tools/NestedGenericWrapperTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/NestedGenericWrapperTest.kt @@ -96,6 +96,12 @@ class NestedGenericWrapperTest { @Test fun `waits on futures in a list from a suspend function`() = assertResolves("suspendListOfFutures", ab) + @Test + fun `waits on futures in a list of nullable elements from a suspend function`() = assertResolves("suspendNullableListOfFutures", ab) + + @Test + fun `resolves a list of scalars typed by a suspend function's own type variable`() = assertResolves("suspendListOfTypeVariable", listOf("a", "b"), selection = "") + @Test fun `keeps null elements and null transformer results in a list`() = assertResolves("listOfNullableWrapped", listOf(mapOf("data" to "a"), null, null)) @@ -171,21 +177,22 @@ class NestedGenericWrapperTest { val graphQLTypes = listOf(payload, NonNullType(payload), ListType(payload), NonNullType(ListType(NonNullType(payload))), ListType(ListType(payload))) val containers = listOf(List::class.java, CompletableFuture::class.java, Try::class.java, Page::class.java, DataFetcherResult::class.java, Optional::class.java) val shapes = (1..4).fold(listOf(listOf>())) { shapes, _ -> shapes + shapes.filter { it.size == shapes.last().size }.flatMap { shape -> containers.map { shape + it } } } + val javaTypes = shapes.flatMap { shape -> listOf(shape to javaType(shape, wildcardElements = false), shape to javaType(shape, wildcardElements = true)) } - val accepted = shapes.flatMap { shape -> graphQLTypes.map { shape to it } }.filter { (shape, graphQLType) -> + val accepted = javaTypes.flatMap { (shape, javaType) -> graphQLTypes.map { Triple(shape, javaType, it) } }.filter { (_, javaType, graphQLType) -> val match = try { - matcher.match(TypeClassMatcher.PotentialMatch.returnValue(graphQLType, typeResolver.resolve(javaType(shape)), typeResolver, SchemaClassScanner.ReturnValueReference(method))) + matcher.match(TypeClassMatcher.PotentialMatch.returnValue(graphQLType, typeResolver.resolve(javaType), typeResolver, SchemaClassScanner.ReturnValueReference(method))) } catch (e: SchemaClassScannerError) { null } (match as? TypeClassMatcher.ValidMatch)?.javaType == Payload::class.java } - val failures = accepted.mapNotNull { (shape, graphQLType) -> + val failures = accepted.mapNotNull { (shape, javaType, graphQLType) -> val unwrapper = ReturnValueUnwrapper.create( - method, javaType(shape), TypeUtils.getRawType(typeResolver.resolve(javaType(shape)), null), graphQLType, false, typeResolver, options.genericWrappers, emptySet() + method, javaType, TypeUtils.getRawType(typeResolver.resolve(javaType), null), graphQLType, false, typeResolver, options.genericWrappers, emptySet() ) val result = GraphQL.newGraphQL(fieldSchema(graphQLType) { environment -> unwrapper.unwrap(value(shape)) { environment } }).build().execute("query { field { data } }") - "${TypeUtils.toString(javaType(shape))} as ${AstPrinter.printAst(graphQLType)}: ${result.errors.map { it.message }} ${result.getData()}" + "${TypeUtils.toString(javaType)} as ${AstPrinter.printAst(graphQLType)}: ${result.errors.map { it.message }} ${result.getData()}" .takeUnless { result.errors.isEmpty() && result.getData() == mapOf("field" to expected(graphQLType)) } } @@ -193,8 +200,11 @@ class NestedGenericWrapperTest { assertEquals(failures, listOf()) } - private fun javaType(shape: List>): java.lang.reflect.Type = - shape.foldRight(Payload::class.java as java.lang.reflect.Type) { container, inner -> ParameterizedTypeImpl(container, arrayOf(inner), null) } + private fun javaType(shape: List>, wildcardElements: Boolean): java.lang.reflect.Type = + shape.foldRight(Payload::class.java as java.lang.reflect.Type) { container, inner -> + val element = if (wildcardElements && container == List::class.java) TypeUtils.wildcardType().withUpperBounds(inner).build() else inner + ParameterizedTypeImpl(container, arrayOf(element), null) + } private fun value(shape: List>): Any = shape.foldRight(Payload("a") as Any) { container, inner -> when (container) { @@ -242,6 +252,8 @@ class NestedGenericWrapperTest { optionalListOfWrapped: [Payload!] suspendListOfWrapped: [Payload!]! suspendListOfFutures: [Payload!]! + suspendNullableListOfFutures: [Payload] + suspendListOfTypeVariable: [String] listOfNullableWrapped: [Payload]! futureOfFuture: Payload! optionalFuture: Payload @@ -339,6 +351,9 @@ class NestedGenericWrapperTest { fun optionalListOfWrapped(): Optional>> = Optional.of(listOfWrapped()) suspend fun suspendListOfWrapped(): List> = listOfWrapped() suspend fun suspendListOfFutures(): List> = listOfFutures() + suspend fun suspendNullableListOfFutures(): List> = listOfFutures() + @Suppress("UNCHECKED_CAST") + suspend fun suspendListOfTypeVariable(): List = listOf("a", "b") as List fun listOfNullableWrapped(): List?> = listOf(Try.success(Payload("a")), Try.success(null), null) fun futureOfFuture(): CompletableFuture> = CompletableFuture.supplyAsync { completedFuture(Payload("a")) } fun optionalFuture(): Optional> = Optional.of(CompletableFuture.supplyAsync { Payload("a") })