Unwrap nested return values the way the schema build matched them

src/main/kotlin/graphql/kickstart/tools/GenericTypeResolver.kt

@@ -29,7 +29,7 @@ internal class GenericTypeResolver(private val containingType: JavaType, private
      * - 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: ((wrapped: ParameterizedType) -> Unit)? = null): JavaType {
+    fun resolve(javaType: JavaType, onUnwrap: ((SchemaParserOptions.GenericWrapper) -> Unit)? = null): JavaType {
         return when (val type = resolveTypeVariables(javaType)) {
             is ParameterizedType -> {
                 val rawType = type.rawType
@@ -41,7 +41,7 @@ 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)
+                onUnwrap?.invoke(wrapper)
                 val unwrapsTo = wrapper.schemaWrapper.invoke(typeArguments[wrapper.index])
                 resolve(unwrapsTo, onUnwrap)
             }

src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt

@@ -1,6 +1,7 @@
 package graphql.kickstart.tools
 
 import graphql.execution.DataFetcherResult
+import graphql.kickstart.tools.SchemaParserOptions.GenericWrapper
 import graphql.kickstart.tools.util.GraphQLLangType
 import graphql.kickstart.tools.util.JavaType
 import graphql.kickstart.tools.util.isSubtypeOf
@@ -15,79 +16,22 @@
  */
 internal class TypeClassMatcher(private val definitionsByName: Map<String, TypeDefinition<*>>) {
 
-    private fun error(potentialMatch: PotentialMatch, msg: String) = SchemaClassScannerError("Unable to match type definition (${TypeUtil.simplePrint(potentialMatch.graphQLType)}) for reference ${potentialMatch.reference} with java type (${potentialMatch.javaType}): $msg")
-
     fun match(potentialMatch: PotentialMatch): Match {
-        return match(potentialMatch, potentialMatch.graphQLType, potentialMatch.javaType, true)
-    }
-
-    private fun match(potentialMatch: PotentialMatch, graphQLType: GraphQLLangType, javaType: JavaType, root: Boolean = false): Match {
-
-        var realType = potentialMatch.typeResolver.resolve(javaType)
-
-        if (realType is ParameterizedType && realType.isSubtypeOf(DataFetcherResult::class.java)) {
-            if (potentialMatch.location != Location.RETURN_TYPE) {
-                throw error(potentialMatch, "${DataFetcherResult::class.java.name} can only be used as a return type")
-            }
-
-            realType = potentialMatch.typeResolver.resolve(realType.actualTypeArguments.first())
-
-            if (realType is ParameterizedType && realType.isSubtypeOf(DataFetcherResult::class.java)) {
-                throw error(potentialMatch, "${DataFetcherResult::class.java.name} cannot be nested within itself")
-            }
-        }
-
-        var optional = false
-
-        // Handle jdk8 Optionals
-        if (realType is ParameterizedType && realType.isSubtypeOf(Optional::class.java)) {
-            optional = true
-
-            if (potentialMatch.location == Location.RETURN_TYPE && !root) {
-                throw error(potentialMatch, "${Optional::class.java.name} can only be used at the top level of a return type")
-            }
-
-            realType = potentialMatch.typeResolver.resolve(realType.actualTypeArguments.first())
-
-            if (realType is ParameterizedType && realType.isSubtypeOf(Optional::class.java)) {
-                throw error(potentialMatch, "${Optional::class.java.name} cannot be nested within itself")
-            }
-        }
-
-        // Match graphql type to java type.
-        return when (graphQLType) {
-            is NonNullType -> {
-                if (optional) {
-                    throw error(potentialMatch, "graphql type is marked as nonnull but ${Optional::class.java.name} was used")
-                }
-                match(potentialMatch, graphQLType.type, realType)
-            }
-
-            is ListType -> when {
-                realType is ParameterizedType && realType.isSubtypeOf(Iterable::class.java) ->
-                    match(potentialMatch, graphQLType.type, realType.actualTypeArguments.first())
-                realType is Class<*> && realType.isArray ->
-                    match(potentialMatch, graphQLType.type, realType.componentType)
-                // e.g. a property map value: the element type is unknown, so match it as Object too
-                realType == Any::class.java ->
-                    match(potentialMatch, graphQLType.type, realType)
-                else ->
-                    throw error(potentialMatch, "Java class is not a List or generic type information was lost: $realType")
-            }
-
+        val unwrapping = potentialMatch.unwrapping
+        return when (val graphQLType = unwrapping.graphQLType) {
             is TypeName -> {
                 val typeDefinition = ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS[graphQLType.name]
                     ?: definitionsByName[graphQLType.name]
-                    ?: throw error(potentialMatch, "No ${TypeDefinition::class.java.simpleName} for type name ${graphQLType.name}")
+                    ?: throw potentialMatch.error("No ${TypeDefinition::class.java.simpleName} for type name ${graphQLType.name}")
                 if (typeDefinition is ScalarTypeDefinition) {
                     ScalarMatch(typeDefinition)
                 } else {
-                    ValidMatch(typeDefinition, realType, potentialMatch.reference)
+                    ValidMatch(typeDefinition, unwrapping.javaType, potentialMatch.reference)
                 }
             }
 
-            is TypeDefinition<*> -> ValidMatch(graphQLType, realType, potentialMatch.reference)
-            else -> throw error(potentialMatch, "Unknown type: ${realType.javaClass.name}")
+            is TypeDefinition<*> -> ValidMatch(graphQLType, unwrapping.javaType, potentialMatch.reference)
+            else -> throw potentialMatch.error("Unknown type: ${unwrapping.javaType.javaClass.name}")
         }
     }
 
@@ -109,6 +53,72 @@
         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<Unwrapped>): Unwrapping {
+            val onUnwrap = { wrapper: GenericWrapper -> unwrapped += Unwrapped.Wrapper(wrapper) }
+            var realType = typeResolver.resolve(javaType, onUnwrap)
+
+            if (realType is ParameterizedType && realType.isSubtypeOf(DataFetcherResult::class.java)) {
+                if (location != Location.RETURN_TYPE) {
+                    throw error("${DataFetcherResult::class.java.name} can only be used as a return type")
+                }
+
+                unwrapped += Unwrapped.Data
+                realType = typeResolver.resolve(realType.actualTypeArguments.first(), onUnwrap)
+
+                if (realType is ParameterizedType && realType.isSubtypeOf(DataFetcherResult::class.java)) {
+                    throw error("${DataFetcherResult::class.java.name} cannot be nested within itself")
+                }
+            }
+
+            var optional = false
+
+            // Handle jdk8 Optionals
+            if (realType is ParameterizedType && realType.isSubtypeOf(Optional::class.java)) {
+                optional = true
+
+                if (location == Location.RETURN_TYPE && !root) {
+                    throw error("${Optional::class.java.name} can only be used at the top level of a return type")
+                }
+
+                unwrapped += Unwrapped.Present
+                realType = typeResolver.resolve(realType.actualTypeArguments.first(), onUnwrap)
+
+                if (realType is ParameterizedType && realType.isSubtypeOf(Optional::class.java)) {
+                    throw error("${Optional::class.java.name} cannot be nested within itself")
+                }
+            }
+
+            // 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)
@@ -117,4 +127,13 @@
                 PotentialMatch(graphQLType, javaType, typeResolver, reference, Location.PARAMETER_TYPE)
         }
     }
+
+    internal class Unwrapping(val unwrapped: List<Unwrapped>, 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
+    }
 }

src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt

@@ -130,9 +130,7 @@
         // 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, returnType, valueClass, field.type, search.isSubscription, typeResolver, options.genericWrappers, scalarNames
-        )
+        val unwrapper = ReturnValueUnwrapper.create(method, returnValueMatch.unwrapping, valueClass, isSuspendFunction, search.isSubscription, options.genericWrappers)
 
         return if (numberOfParameters > 0 || isSuspendFunction) {
             // requires arguments and environment or is a suspend function
@@ -160,8 +158,6 @@
     }
 
     override fun scanForMatches(): List<TypeClassMatcher.PotentialMatch> {
-        val returnValueMatch = TypeClassMatcher.PotentialMatch.returnValue(field.type, getResolvedReturnType(), typeResolver, ScanDictionary.ReturnValueReference(method))
-
         return field.inputValueDefinitions.mapIndexed { i, inputDefinition ->
             TypeClassMatcher.PotentialMatch.parameterType(inputDefinition.type, getMethodParameterType(i)!!, typeResolver, ScanDictionary.MethodParameterReference(method, i))
         } + listOf(returnValueMatch)
@@ -184,8 +180,13 @@
         typeResolver.resolveTypeVariables(declaredType)
     }
 
-    private fun getResolvedReturnType(): JavaType =
-        typeResolver.resolve(if (search.isSubscription) returnType.asPublisher() else returnType)
+    private val returnValueMatch by lazy {
+        TypeClassMatcher.PotentialMatch.returnValue(field.type, matchedReturnType(), typeResolver, 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
 
@@ -219,7 +220,7 @@
         return if (isSuspendFunction) {
             // start undispatched so DataLoader loads are queued before graphql-java dispatches them,
             // which runs the block even if the context is already cancelled, hence ensureActive
             environment.coroutineScope().future(options.coroutineContextProvider.provide(), CoroutineStart.UNDISPATCHED) {
                 ensureActive()
                 invokeSuspend(source, method, args)?.let { unwrapper.unwrapSuspending(it) { environment } }
             }