diff --git a/src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt b/src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt index 44f7c64a..fff48337 100644 --- a/src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt +++ b/src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt @@ -60,36 +60,36 @@ internal class TypeClassMatcher(private val definitionsByName: Map): 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 data = false 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") - } + 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 + } - unwrapped += Unwrapped.Present - realType = typeResolver.resolve(realType.actualTypeArguments.first(), onUnwrap) + 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 + } - if (realType is ParameterizedType && realType.isSubtypeOf(Optional::class.java)) { - throw error("${Optional::class.java.name} cannot be nested within itself") + else -> break } + realType = typeResolver.resolve(realType.actualTypeArguments.first(), onUnwrap) } // Match graphql type to java type. diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/ReturnValueUnwrapper.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/ReturnValueUnwrapper.kt index 705316dd..5a0473d3 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/ReturnValueUnwrapper.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/ReturnValueUnwrapper.kt @@ -88,23 +88,29 @@ private class DataOf(private val data: Step) : Step { } } -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 +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? = - unwrapped.foldRight(null as Step?) { it, next -> - when (it) { - is Unwrapped.Wrapper -> if (it.isFuture) Await(it.wrapper, awaits, next) else Peel(it.wrapper, peels, next) - Unwrapped.Data -> next?.let(::DataOf) - Unwrapped.Present -> next?.let(::ValueOf) - Unwrapped.Elements -> next?.let(::EachElement) - } +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) diff --git a/src/test/kotlin/graphql/kickstart/tools/NestedGenericWrapperTest.kt b/src/test/kotlin/graphql/kickstart/tools/NestedGenericWrapperTest.kt index 79d1bb87..aad0a9d2 100644 --- a/src/test/kotlin/graphql/kickstart/tools/NestedGenericWrapperTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/NestedGenericWrapperTest.kt @@ -93,6 +93,25 @@ class NestedGenericWrapperTest { @Test fun `unwraps wrappers in a list in an Optional`() = assertResolves("optionalListOfWrapped", ab) + @Test + fun `resolves a DataFetcherResult in an Optional and keeps its errors`() { + val result = execute("optionalResult") + + assertEquals(result.errors.map { it.message }, listOf("partial")) + assertEquals(result.getData(), mapOf("optionalResult" to mapOf("data" to "a"))) + } + + @Test + fun `resolves an empty Optional of a DataFetcherResult to null`() = assertResolves("emptyOptionalResult", null) + + @Test + fun `unwraps wrappers in a list in a DataFetcherResult in an Optional and keeps its errors`() { + val result = execute("optionalResultOfListOfWrapped") + + assertEquals(result.errors.map { it.message }, listOf("partial")) + assertEquals(result.getData(), mapOf("optionalResultOfListOfWrapped" to ab)) + } + @Test fun `unwraps wrappers in a list from a suspend function`() = assertResolves("suspendListOfWrapped", ab) @@ -178,6 +197,7 @@ class NestedGenericWrapperTest { assertSame(fetch(schema, "plainList"), query.plainList) assertSame(fetch(schema, "futureOfList"), query.futureOfList) assertSame(fetch(schema, "wrappedFuture"), query.wrappedFuture.get()) + assertSame(fetch(schema, "optionalPayload"), query.optionalPayload) } @Test @@ -202,18 +222,22 @@ class NestedGenericWrapperTest { val shapes = (1..4).fold(listOf(listOf>())) { shapes, _ -> shapes + shapes.filter { it.size == shapes.last().size }.flatMap { shape -> containers.map { shape + it } } } val accepted = graphQLTypes.flatMap { graphQLType -> - shapes.flatMap { shape -> listOf(false, true).map { shape to javaType(shape, leaf(graphQLType).javaClass, wildcardElements = it) } }.mapNotNull { (shape, javaType) -> + 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)) - val match = try { + try { matcher.match(potentialMatch) + shape to potentialMatch } catch (e: SchemaClassScannerError) { null } - (shape to potentialMatch).takeIf { match != null && potentialMatch.unwrapping.javaType == leaf(graphQLType).javaClass } } } 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 unwrapper = ReturnValueUnwrapper.create( method, potentialMatch.unwrapping, TypeUtils.getRawType(typeResolver.resolve(potentialMatch.javaType), null), isSuspendFunction = false, isSubscription = false, options.genericWrappers ) @@ -232,6 +256,38 @@ class NestedGenericWrapperTest { assertEquals(failures, listOf()) } + @Test + fun `rejects a DataFetcherResult or an Optional twice in one value`() { + val typeResolver = GenericTypeResolver(Query::class.java, SchemaParserOptions.newOptions().build()) + val matcher = TypeClassMatcher(mapOf("Payload" to ObjectTypeDefinition("Payload"))) + val method = Query::class.java.getMethod("listOfWrapped") + val result = DataFetcherResult::class.java + val optional = Optional::class.java + val rejections = listOf( + listOf(optional, result, result) to "${result.name} cannot be nested within itself", + listOf(result, optional, result) to "${result.name} cannot be nested within itself", + listOf(optional, result, optional) to "${optional.name} cannot be nested within itself", + ) + + 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) + "accepted" + } catch (e: SchemaClassScannerError) { + e.message.orEmpty().substringAfterLast("): ") + } + } + + assertEquals(messages, rejections.map { it.second }) + } + + private fun Type<*>.listDepth(): Int = when (this) { + is ListType -> 1 + type.listDepth() + is NonNullType -> type.listDepth() + else -> 0 + } + private fun leaf(graphQLType: Type<*>): Any = if (graphQLType.unwrap().name == "Leaf") FutureLeaf("a") else Payload("a") private fun javaType(shape: List>, leafClass: Class<*>, wildcardElements: Boolean): java.lang.reflect.Type = @@ -284,6 +340,10 @@ class NestedGenericWrapperTest { pageOfWrapped: [Payload!]! resultOfListOfWrapped: [Payload!]! listOfResultsOfWrapped: [Payload!]! + optionalResult: Payload + emptyOptionalResult: Payload + optionalResultOfListOfWrapped: [Payload!] + optionalPayload: Payload optionalListOfWrapped: [Payload!] suspendListOfWrapped: [Payload!]! suspendListOfFutures: [Payload!]! @@ -387,6 +447,7 @@ class NestedGenericWrapperTest { val plainList = listOf(Payload("a")) val futureOfList: CompletableFuture> = completedFuture(plainList) val wrappedFuture: Try> = Try.success(completedFuture(Payload("a"))) + val optionalPayload: Optional = Optional.of(Payload("a")) private fun result(data: T, vararg errors: GraphQLError): DataFetcherResult = DataFetcherResult.newResult().data(data).errors(errors.toList()).build() @@ -406,6 +467,9 @@ class NestedGenericWrapperTest { fun resultOfListOfWrapped(): DataFetcherResult>> = result(listOfWrapped(), GraphQLError.newError().message("partial").build()) fun listOfResultsOfWrapped(): List>> = listOfWrapped().map { result(it) } fun optionalListOfWrapped(): Optional>> = Optional.of(listOfWrapped()) + fun optionalResult(): Optional> = Optional.of(result(Payload("a"), GraphQLError.newError().message("partial").build())) + fun emptyOptionalResult(): Optional> = Optional.empty() + fun optionalResultOfListOfWrapped(): Optional>>> = Optional.of(resultOfListOfWrapped()) suspend fun suspendListOfWrapped(): List> = listOfWrapped() suspend fun suspendListOfFutures(): List> = listOfFutures() suspend fun suspendNullableListOfFutures(): List> = listOfFutures()