From fe52c506f692becb01f97fc81d7441f36c504f6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bernd=20St=C3=BCbinger?= <41049452+stuebingerb@users.noreply.github.com> Date: Mon, 16 Dec 2024 16:15:06 +0100 Subject: [PATCH] fix: Fix non-nullable union types Resolves #135 --- .../DataLoaderPreparedRequestExecutor.kt | 11 +++-- .../execution/ParallelRequestExecutor.kt | 11 +++-- .../kgraphql/schema/structure/Field.kt | 3 +- .../schema/structure/RequestInterpreter.kt | 2 +- .../schema/structure/SchemaCompilation.kt | 14 +++--- .../kgraphql/schema/structure/Validation.kt | 2 +- .../kgraphql/schema/SchemaPrinterTest.kt | 47 ++++++++++++++++++- .../typesystem/UnionsSpecificationTest.kt | 2 +- 8 files changed, 69 insertions(+), 23 deletions(-) diff --git a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/DataLoaderPreparedRequestExecutor.kt b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/DataLoaderPreparedRequestExecutor.kt index 74406a31..16440681 100644 --- a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/DataLoaderPreparedRequestExecutor.kt +++ b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/DataLoaderPreparedRequestExecutor.kt @@ -267,13 +267,14 @@ class DataLoaderPreparedRequestExecutor(val schema: DefaultSchema) : RequestExec ctx = ctx ) - val returnType = unionProperty.returnType.possibleTypes.find { it.isInstance(operationResult) } + val possibleTypes = (unionProperty.returnType.unwrapped() as Type.Union).possibleTypes + val returnType = possibleTypes.find { it.isInstance(operationResult) } - if (returnType == null && !unionProperty.nullable) { - val expectedOneOf = unionProperty.type.possibleTypes!!.joinToString { it.name.toString() } + if (returnType == null && unionProperty.returnType.isNotNullable()) { + val expectedOneOf = possibleTypes.joinToString { it.name.toString() } throw ExecutionException( - "Unexpected type of union property value, expected one of: [$expectedOneOf]." + - " value was $operationResult", node + "Unexpected type of union property value, expected one of [$expectedOneOf] but was $operationResult", + node ) } diff --git a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ParallelRequestExecutor.kt b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ParallelRequestExecutor.kt index ce20096e..9bf36977 100644 --- a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ParallelRequestExecutor.kt +++ b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/execution/ParallelRequestExecutor.kt @@ -111,13 +111,14 @@ class ParallelRequestExecutor(val schema: DefaultSchema) : RequestExecutor { ctx = ctx ) - val returnType = unionProperty.returnType.possibleTypes.find { it.isInstance(operationResult) } + val possibleTypes = (unionProperty.returnType.unwrapped() as Type.Union).possibleTypes + val returnType = possibleTypes.find { it.isInstance(operationResult) } - if (returnType == null && !unionProperty.nullable) { - val expectedOneOf = unionProperty.type.possibleTypes!!.joinToString { it.name.toString() } + if (returnType == null && unionProperty.returnType.isNotNullable()) { + val expectedOneOf = possibleTypes.joinToString { it.name.toString() } throw ExecutionException( - "Unexpected type of union property value, expected one of: [$expectedOneOf]." + - " value was $operationResult", node + "Unexpected type of union property value, expected one of [$expectedOneOf] but was $operationResult", + node ) } diff --git a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/Field.kt b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/Field.kt index ba5d89fe..d11bdd3d 100644 --- a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/Field.kt +++ b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/Field.kt @@ -90,8 +90,7 @@ sealed class Field : __Field { class Union( kql: PropertyDef.Union, - val nullable: Boolean, - override val returnType: Type.Union, + override val returnType: Type, override val arguments: List> ) : Field(), FunctionWrapper by kql { diff --git a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/RequestInterpreter.kt b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/RequestInterpreter.kt index 1f4adf1d..ac92fff7 100644 --- a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/RequestInterpreter.kt +++ b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/RequestInterpreter.kt @@ -216,7 +216,7 @@ class RequestInterpreter(private val schemaModel: SchemaModel) { // do not define any fields, so *no* fields may be queried on this type without the use of type refining // fragments or inline fragments (with the exception of the meta-field `__typename`)." val unionMembersChildren: Map> = - field.returnType.possibleTypes.associateWith { possibleType -> + (field.returnType.unwrapped() as Type.Union).possibleTypes.associateWith { possibleType -> val mergedSelectionsForType = selectionNode.selectionSet?.selections?.flatMap { when { // Only __typename is allowed as field selection diff --git a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/SchemaCompilation.kt b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/SchemaCompilation.kt index 4a1cb160..b269bf98 100644 --- a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/SchemaCompilation.kt +++ b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/SchemaCompilation.kt @@ -184,8 +184,8 @@ class SchemaCompilation( private suspend fun handleUnionProperty(unionProperty: PropertyDef.Union<*>): Field { val inputValues = handleInputValues(unionProperty.name, unionProperty, unionProperty.inputValues) - val type = handleUnionType(unionProperty.union) - return Field.Union(unionProperty, unionProperty.nullable, type, inputValues) + val type = applyNullability(unionProperty.nullable, handleUnionType(unionProperty.union)) + return Field.Union(unionProperty, type, inputValues) } private suspend fun handlePossiblyWrappedType(kType: KType, typeCategory: TypeCategory): Type = try { @@ -201,7 +201,7 @@ class SchemaCompilation( name = kType.jvmErasure.simpleName!!, members = kType.jvmErasure.sealedSubclasses.toSet(), description = null - ).let { handleUnionType(it) } + ).let { applyNullability(kType.isMarkedNullable, handleUnionType(it)) } else -> handleSimpleType(kType, typeCategory) } @@ -216,16 +216,16 @@ class SchemaCompilation( private suspend fun handleCollectionType(kType: KType, typeCategory: TypeCategory): Type { val type = kType.getIterableElementType() val nullableListType = Type.AList(handlePossiblyWrappedType(type, typeCategory)) - return applyNullability(kType, nullableListType) + return applyNullability(kType.isMarkedNullable, nullableListType) } private suspend fun handleSimpleType(kType: KType, typeCategory: TypeCategory): Type { val simpleType = handleRawType(kType.jvmErasure, typeCategory) - return applyNullability(kType, simpleType) + return applyNullability(kType.isMarkedNullable, simpleType) } - private fun applyNullability(kType: KType, simpleType: Type): Type { - return if (!kType.isMarkedNullable) { + private fun applyNullability(isNullable: Boolean, simpleType: Type): Type { + return if (!isNullable) { Type.NonNull(simpleType) } else { simpleType diff --git a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/Validation.kt b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/Validation.kt index 33a0b716..81a1e0cd 100644 --- a/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/Validation.kt +++ b/kgraphql/src/main/kotlin/com/apurebase/kgraphql/schema/structure/Validation.kt @@ -68,7 +68,7 @@ fun validateUnionRequest(field: Field.Union<*>, selectionNode: FieldNode) { illegalChildren.joinToString(prefix = "[", postfix = "]") { it.aliasOrName.value } - } on union type property ${field.name} : ${field.returnType.possibleTypes.map { it.name }}", + } on union type property ${field.name} : ${(field.returnType.unwrapped() as Type.Union).possibleTypes.map { it.name }}", nodes = illegalChildren ) } diff --git a/kgraphql/src/test/kotlin/com/apurebase/kgraphql/schema/SchemaPrinterTest.kt b/kgraphql/src/test/kotlin/com/apurebase/kgraphql/schema/SchemaPrinterTest.kt index c76792d7..ce143594 100644 --- a/kgraphql/src/test/kotlin/com/apurebase/kgraphql/schema/SchemaPrinterTest.kt +++ b/kgraphql/src/test/kotlin/com/apurebase/kgraphql/schema/SchemaPrinterTest.kt @@ -112,6 +112,12 @@ class SchemaPrinterTest { } } } + unionProperty("nullablePdf") { + returnType = linked + nullable = true + description = "link to pdf representation of scenario" + resolver { scenario: Scenario -> null } + } } } @@ -133,7 +139,8 @@ class SchemaPrinterTest { type Scenario { author: String! content: String! - pdf: Linked + nullablePdf: Linked + pdf: Linked! } union Linked = Author | Scenario @@ -141,6 +148,44 @@ class SchemaPrinterTest { """.trimIndent() } + sealed class Child + data class Child1(val one: String): Child() + data class Child2(val two: String?): Child() + + @Test + fun `schema with union types out of sealed classes should be printed as expected`() { + val schema = KGraphQL.schema { + query("child") { + resolver { -> Child1("one") } + } + query("childs") { + resolver> { -> listOf(Child2("one")) } + } + query("nullchilds") { + resolver?> { -> null } + } + } + + SchemaPrinter().print(schema) shouldBeEqualTo """ + type Child1 { + one: String! + } + + type Child2 { + two: String + } + + type Query { + child: Child! + childs: [Child!]! + nullchilds: [Child] + } + + union Child = Child1 | Child2 + + """.trimIndent() + } + @Test fun `schema with interfaces should be printed as expected`() { val schema = KGraphQL.schema { diff --git a/kgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/typesystem/UnionsSpecificationTest.kt b/kgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/typesystem/UnionsSpecificationTest.kt index 5b0ee65e..dea01e8b 100644 --- a/kgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/typesystem/UnionsSpecificationTest.kt +++ b/kgraphql/src/test/kotlin/com/apurebase/kgraphql/specification/typesystem/UnionsSpecificationTest.kt @@ -244,7 +244,7 @@ class UnionsSpecificationTest : BaseSchemaTest() { }""".trimIndent() ) } shouldThrow ExecutionException::class with { - message shouldBeEqualTo "Unexpected type of union property value, expected one of: [Actor, Scenario, Director]. value was null" + message shouldBeEqualTo "Unexpected type of union property value, expected one of [Actor, Scenario, Director] but was null" } }