Make nullable parameters always optional (#1528)

* Make nullable parameters always optional Treat nullable parameters that have no default value as if their default was set to null. Nullable parameters that have a default value explicitly set will continue to use that as their default. * Apply review feedback - add KParameter.isNotOptionalNullable() - minor cleanups in mapToKotlinObject
ExpediaGroup · Aug 24, 2022 · 095b056 · 095b056
1 parent 2bc633c
commit 095b056
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 4 deletions.
diff --git a/...ator/src/main/kotlin/com/expediagroup/graphql/generator/execution/convertArgumentValue.kt b/...ator/src/main/kotlin/com/expediagroup/graphql/generator/execution/convertArgumentValue.kt
@@ -22,6 +22,7 @@ import com.expediagroup.graphql.generator.internal.extensions.getGraphQLName
 import com.expediagroup.graphql.generator.internal.extensions.getKClass
 import com.expediagroup.graphql.generator.internal.extensions.getName
 import com.expediagroup.graphql.generator.internal.extensions.getTypeOfFirstArgument
+import com.expediagroup.graphql.generator.internal.extensions.isNotOptionalNullable
 import com.expediagroup.graphql.generator.internal.extensions.isOptionalInputType
 import com.expediagroup.graphql.generator.internal.extensions.isSubclassOf
 import kotlin.reflect.KClass
@@ -105,13 +106,16 @@ private fun <T : Any> mapToKotlinObject(input: Map<String, *>, targetClass: KCla
         }
     }
 
-    val constructorParameters = targetConstructor.parameters
     // filter parameters that are actually in the input in order to rely on parameters default values
     // in target constructor
-    val constructorParametersInInput = constructorParameters.filter { parameter ->
-        input.containsKey(parameter.getName()) || parameter.type.isOptionalInputType()
+    val constructorParameters = targetConstructor.parameters.filter { parameter ->
+        input.containsKey(parameter.getName()) ||
+            parameter.type.isOptionalInputType() ||
+
+            // for nullable parameters that have no explicit default, we pass in null if not in input
+            parameter.isNotOptionalNullable()
     }
-    val constructorArguments = constructorParametersInInput.associateWith { parameter ->
+    val constructorArguments = constructorParameters.associateWith { parameter ->
         convertArgumentValue(parameter.getName(), parameter, input)
     }
     return targetConstructor.callBy(constructorArguments)

diff --git a/...ain/kotlin/com/expediagroup/graphql/generator/internal/extensions/kParameterExtensions.kt b/...ain/kotlin/com/expediagroup/graphql/generator/internal/extensions/kParameterExtensions.kt
@@ -29,6 +29,8 @@ internal fun KParameter.isGraphQLContext() = this.type.getKClass().isSubclassOf(
 
 internal fun KParameter.isDataFetchingEnvironment() = this.type.classifier == DataFetchingEnvironment::class
 
+internal fun KParameter.isNotOptionalNullable() = type.isMarkedNullable && !isOptional
+
 @Throws(CouldNotGetNameOfKParameterException::class)
 internal fun KParameter.getName(): String =
     this.getGraphQLName() ?: this.name ?: throw CouldNotGetNameOfKParameterException(this)
diff --git a/.../src/test/kotlin/com/expediagroup/graphql/generator/execution/ConvertArgumentValueTest.kt b/.../src/test/kotlin/com/expediagroup/graphql/generator/execution/ConvertArgumentValueTest.kt
@@ -118,6 +118,22 @@ class ConvertArgumentValueTest {
         assertEquals("nested default value", castResult.nested?.value)
     }
 
+    @Test
+    fun `generic map object is parsed and correct defaults are used for nullables`() {
+        val kParam = assertNotNull(TestFunctions::inputObjectNullableWithAndWithoutDefaults.findParameterByName("input"))
+        val result = convertArgumentValue(
+            "input",
+            kParam,
+            mapOf(
+                "input" to emptyMap<String, Any>()
+            )
+        )
+
+        val castResult = assertIs<TestInputNullableWithAndWithoutDefaults>(result)
+        assertEquals(null, castResult.iHaveNoDefault)
+        assertEquals("DEFAULT", castResult.iHaveADefault)
+    }
+
     @Test
     fun `generic map object is parsed without using primary constructor`() {
         val kParam = assertNotNull(TestFunctions::inputObjectNoPrimaryConstructor.findParameterByName("input"))
@@ -278,6 +294,7 @@ class ConvertArgumentValueTest {
         fun inputObjectNoPrimaryConstructor(input: TestInputNoPrimaryConstructor): String = TODO()
         fun inputObjectMultipleConstructors(input: TestInputMultipleConstructors): String = TODO()
         fun inputObjectNested(input: TestInputNested): String = TODO()
+        fun inputObjectNullableWithAndWithoutDefaults(input: TestInputNullableWithAndWithoutDefaults): String = TODO()
         fun inputObjectNullableScalar(input: TestInputNullableScalar): String = TODO()
         fun inputObjectNotNullableScalar(input: TestInputNotNullableScalar): String = TODO()
         fun listStringInput(input: List<String>): String = TODO()
@@ -291,6 +308,7 @@ class ConvertArgumentValueTest {
     class TestInput(val foo: String, val bar: String? = null, val baz: List<String>? = null, val qux: String? = null)
     class TestInputNested(val foo: String? = "foo", val bar: String? = "bar", val nested: TestInputNestedType? = TestInputNestedType())
     class TestInputNestedType(val value: String = "nested default value")
+    class TestInputNullableWithAndWithoutDefaults(val iHaveNoDefault: String?, val iHaveADefault: String? = "DEFAULT")
     class TestInputNullableScalar(val foo: String? = null, val id: ID? = null)
     class TestInputNotNullableScalar(val foo: String, val id: ID = ID("1234"))
     class TestInputRenamed(@GraphQLName("bar") val foo: String)