fix: Fix non-nullable union types

Resolves #135
stuebingerb · Dec 16, 2024 · 3aed18f · 3aed18f
1 parent 156ce61
commit 3aed18f
Show file tree

Hide file tree

Showing 8 changed files with 69 additions and 23 deletions.
diff --git a/.../main/kotlin/com/apurebase/kgraphql/schema/execution/DataLoaderPreparedRequestExecutor.kt b/.../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
@@ -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
@@ -90,8 +90,7 @@ sealed class Field : __Field {
 
     class Union<T>(
         kql: PropertyDef.Union<T>,
-        val nullable: Boolean,
-        override val returnType: Type.Union,
+        override val returnType: Type,
         override val arguments: List<InputValue<*>>
     ) : Field(), FunctionWrapper<Any?> 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
@@ -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<Type, List<Execution>> =
-            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
@@ -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
@@ -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
@@ -112,6 +112,12 @@ class SchemaPrinterTest {
                         }
                     }
                 }
+                unionProperty("nullablePdf") {
+                    returnType = linked
+                    nullable = true
+                    description = "link to pdf representation of scenario"
+                    resolver { scenario: Scenario -> null }
+                }
             }
         }
 
@@ -133,14 +139,53 @@ class SchemaPrinterTest {
             type Scenario {
               author: String!
               content: String!
-              pdf: Linked
+              nullablePdf: Linked
+              pdf: Linked!
             }
             
             union Linked = Author | Scenario
             
         """.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<Child> { -> Child1("one") }
+            }
+            query("childs") {
+                resolver<List<Child>> { -> listOf(Child2("one")) }
+            }
+            query("nullchilds") {
+                resolver<List<Child?>?> { -> 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/...rc/test/kotlin/com/apurebase/kgraphql/specification/typesystem/UnionsSpecificationTest.kt b/...rc/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"
         }
     }