Skip to content

Commit 095b056

Browse files
authored
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
1 parent 2bc633c commit 095b056

File tree

3 files changed

+28
-4
lines changed

3 files changed

+28
-4
lines changed

generator/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/execution/convertArgumentValue.kt

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import com.expediagroup.graphql.generator.internal.extensions.getGraphQLName
2222
import com.expediagroup.graphql.generator.internal.extensions.getKClass
2323
import com.expediagroup.graphql.generator.internal.extensions.getName
2424
import com.expediagroup.graphql.generator.internal.extensions.getTypeOfFirstArgument
25+
import com.expediagroup.graphql.generator.internal.extensions.isNotOptionalNullable
2526
import com.expediagroup.graphql.generator.internal.extensions.isOptionalInputType
2627
import com.expediagroup.graphql.generator.internal.extensions.isSubclassOf
2728
import kotlin.reflect.KClass
@@ -105,13 +106,16 @@ private fun <T : Any> mapToKotlinObject(input: Map<String, *>, targetClass: KCla
105106
}
106107
}
107108

108-
val constructorParameters = targetConstructor.parameters
109109
// filter parameters that are actually in the input in order to rely on parameters default values
110110
// in target constructor
111-
val constructorParametersInInput = constructorParameters.filter { parameter ->
112-
input.containsKey(parameter.getName()) || parameter.type.isOptionalInputType()
111+
val constructorParameters = targetConstructor.parameters.filter { parameter ->
112+
input.containsKey(parameter.getName()) ||
113+
parameter.type.isOptionalInputType() ||
114+
115+
// for nullable parameters that have no explicit default, we pass in null if not in input
116+
parameter.isNotOptionalNullable()
113117
}
114-
val constructorArguments = constructorParametersInInput.associateWith { parameter ->
118+
val constructorArguments = constructorParameters.associateWith { parameter ->
115119
convertArgumentValue(parameter.getName(), parameter, input)
116120
}
117121
return targetConstructor.callBy(constructorArguments)

generator/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/internal/extensions/kParameterExtensions.kt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ internal fun KParameter.isGraphQLContext() = this.type.getKClass().isSubclassOf(
2929

3030
internal fun KParameter.isDataFetchingEnvironment() = this.type.classifier == DataFetchingEnvironment::class
3131

32+
internal fun KParameter.isNotOptionalNullable() = type.isMarkedNullable && !isOptional
33+
3234
@Throws(CouldNotGetNameOfKParameterException::class)
3335
internal fun KParameter.getName(): String =
3436
this.getGraphQLName() ?: this.name ?: throw CouldNotGetNameOfKParameterException(this)

generator/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/execution/ConvertArgumentValueTest.kt

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,22 @@ class ConvertArgumentValueTest {
118118
assertEquals("nested default value", castResult.nested?.value)
119119
}
120120

121+
@Test
122+
fun `generic map object is parsed and correct defaults are used for nullables`() {
123+
val kParam = assertNotNull(TestFunctions::inputObjectNullableWithAndWithoutDefaults.findParameterByName("input"))
124+
val result = convertArgumentValue(
125+
"input",
126+
kParam,
127+
mapOf(
128+
"input" to emptyMap<String, Any>()
129+
)
130+
)
131+
132+
val castResult = assertIs<TestInputNullableWithAndWithoutDefaults>(result)
133+
assertEquals(null, castResult.iHaveNoDefault)
134+
assertEquals("DEFAULT", castResult.iHaveADefault)
135+
}
136+
121137
@Test
122138
fun `generic map object is parsed without using primary constructor`() {
123139
val kParam = assertNotNull(TestFunctions::inputObjectNoPrimaryConstructor.findParameterByName("input"))
@@ -278,6 +294,7 @@ class ConvertArgumentValueTest {
278294
fun inputObjectNoPrimaryConstructor(input: TestInputNoPrimaryConstructor): String = TODO()
279295
fun inputObjectMultipleConstructors(input: TestInputMultipleConstructors): String = TODO()
280296
fun inputObjectNested(input: TestInputNested): String = TODO()
297+
fun inputObjectNullableWithAndWithoutDefaults(input: TestInputNullableWithAndWithoutDefaults): String = TODO()
281298
fun inputObjectNullableScalar(input: TestInputNullableScalar): String = TODO()
282299
fun inputObjectNotNullableScalar(input: TestInputNotNullableScalar): String = TODO()
283300
fun listStringInput(input: List<String>): String = TODO()
@@ -291,6 +308,7 @@ class ConvertArgumentValueTest {
291308
class TestInput(val foo: String, val bar: String? = null, val baz: List<String>? = null, val qux: String? = null)
292309
class TestInputNested(val foo: String? = "foo", val bar: String? = "bar", val nested: TestInputNestedType? = TestInputNestedType())
293310
class TestInputNestedType(val value: String = "nested default value")
311+
class TestInputNullableWithAndWithoutDefaults(val iHaveNoDefault: String?, val iHaveADefault: String? = "DEFAULT")
294312
class TestInputNullableScalar(val foo: String? = null, val id: ID? = null)
295313
class TestInputNotNullableScalar(val foo: String, val id: ID = ID("1234"))
296314
class TestInputRenamed(@GraphQLName("bar") val foo: String)

0 commit comments

Comments
 (0)