[client] fix handling of custom scalar inputs (#1266)

Update client generation logic to correctly annotate custom scalars when they are used as input fields. Due to the extra wrapper layer, optional input functionality is not supported with custom scalars at this point.

This PR also fixes client generation logic to map GraphQL Floats to Kotlin Double (instead of a Float).

Related:

* partially resolves #1263

Author: dariuszkuc

examples/build.gradle.kts

@@ -36,8 +36,9 @@ subprojects {
         this.ext[key.toString()] = value
     }
 
-    val kotlinVersion: String by project
+    val icuVersion: String by project
     val junitVersion: String by project
+    val kotlinVersion: String by project
     val kotlinCoroutinesVersion: String by project
 
     val detektVersion: String by project
@@ -51,7 +52,7 @@ subprojects {
         implementation(kotlin("stdlib", kotlinVersion))
         implementation(kotlin("reflect", kotlinVersion))
         implementation("org.jetbrains.kotlinx:kotlinx-coroutines-jdk8:$kotlinCoroutinesVersion")
-        implementation("com.ibm.icu:icu4j:69.1")
+        implementation("com.ibm.icu:icu4j:$icuVersion")
         testImplementation(kotlin("test-junit5", kotlinVersion))
         testImplementation("org.junit.jupiter:junit-jupiter-api:$junitVersion")
         testImplementation("org.junit.jupiter:junit-jupiter-engine:$junitVersion")

examples/client/gradle-client/src/main/kotlin/com/expediagroup/graphql/examples/client/gradle/Application.kt

@@ -87,7 +87,7 @@ fun main() {
 
     println("additional examples")
     runBlocking {
-        val exampleData = client.execute(ExampleQuery(variables = ExampleQuery.Variables(simpleCriteria = SimpleArgumentInput(max = 1.0f))))
+        val exampleData = client.execute(ExampleQuery(variables = ExampleQuery.Variables(simpleCriteria = SimpleArgumentInput(max = 1.0))))
         println("\tretrieved interface: ${exampleData.data?.interfaceQuery} ")
         println("\tretrieved union: ${exampleData.data?.unionQuery} ")
         println("\tretrieved enum: ${exampleData.data?.enumQuery} ")

examples/client/maven-client/src/main/kotlin/com/expediagroup/graphql/examples/client/maven/Application.kt

@@ -76,7 +76,7 @@ fun main() {
 
     println("additional examples")
     runBlocking {
-        val exampleData = client.execute(ExampleQuery(variables = ExampleQuery.Variables(simpleCriteria = OptionalInput.Defined(SimpleArgumentInput(max = OptionalInput.Defined(1.0f))))))
+        val exampleData = client.execute(ExampleQuery(variables = ExampleQuery.Variables(simpleCriteria = OptionalInput.Defined(SimpleArgumentInput(max = OptionalInput.Defined(1.0))))))
         println("\tretrieved interface: ${exampleData.data?.interfaceQuery} ")
         println("\tretrieved union: ${exampleData.data?.unionQuery} ")
         println("\tretrieved enum: ${exampleData.data?.enumQuery} ")

gradle.properties

@@ -34,6 +34,7 @@ springVersion = 5.3.9
 
 # test dependency versions
 compileTestingVersion = 1.4.3
+icuVersion=69.1
 junitVersion = 5.7.2
 mockkVersion = 1.12.0
 mustacheVersion = 0.9.10

plugins/client/graphql-kotlin-client-generator/build.gradle.kts

@@ -2,6 +2,7 @@ description = "GraphQL Kotlin common utilities to generate a client."
 
 val compileTestingVersion: String by project
 val graphQLJavaVersion: String by project
+val icuVersion: String by project
 val jacksonVersion: String by project
 val junitVersion: String by project
 val kotlinPoetVersion: String by project
@@ -26,6 +27,7 @@ dependencies {
     testImplementation("com.github.tomakehurst:wiremock-jre8:$wireMockVersion")
     testImplementation("com.github.tschuchortdev:kotlin-compile-testing:$compileTestingVersion")
     testImplementation("org.junit.jupiter:junit-jupiter-params:$junitVersion")
+    testImplementation("com.ibm.icu:icu4j:$icuVersion")
 }
 
 tasks {

plugins/client/graphql-kotlin-client-generator/src/main/kotlin/com/expediagroup/graphql/plugin/client/generator/GraphQLClientGenerator.kt

@@ -37,9 +37,11 @@ import graphql.schema.idl.SchemaParser
 import graphql.schema.idl.TypeDefinitionRegistry
 import kotlinx.serialization.Required
 import kotlinx.serialization.Serializable
+import org.slf4j.LoggerFactory
 import java.io.File
 
 private const val CORE_TYPES_PACKAGE = "com.expediagroup.graphql.client.types"
+internal val LOGGER = LoggerFactory.getLogger(GraphQLClientGenerator::class.java)
 
 /**
  * GraphQL client code generator that uses [KotlinPoet](https://github.com/square/kotlinpoet) to generate Kotlin classes based on the specified GraphQL queries.

plugins/client/graphql-kotlin-client-generator/src/main/kotlin/com/expediagroup/graphql/plugin/client/generator/types/generateGraphQLInputObjectTypeSpec.kt

@@ -19,6 +19,7 @@ package com.expediagroup.graphql.plugin.client.generator.types
 import com.expediagroup.graphql.client.Generated
 import com.expediagroup.graphql.plugin.client.generator.GraphQLClientGeneratorContext
 import com.expediagroup.graphql.plugin.client.generator.GraphQLSerializer
+import com.expediagroup.graphql.plugin.client.generator.LOGGER
 import com.squareup.kotlinpoet.ClassName
 import com.squareup.kotlinpoet.CodeBlock
 import com.squareup.kotlinpoet.FunSpec
@@ -36,7 +37,8 @@ import kotlinx.serialization.Serializable
  * Generate [TypeSpec] data class from the specified input object definition where are fields are mapped to corresponding Kotlin property.
  */
 internal fun generateGraphQLInputObjectTypeSpec(context: GraphQLClientGeneratorContext, inputObjectDefinition: InputObjectTypeDefinition): TypeSpec {
-    val inputObjectTypeSpecBuilder = TypeSpec.classBuilder(inputObjectDefinition.name)
+    val inputTypeName = inputObjectDefinition.name
+    val inputObjectTypeSpecBuilder = TypeSpec.classBuilder(inputTypeName)
         .addModifiers(KModifier.DATA)
         .addAnnotation(Generated::class)
     inputObjectDefinition.description?.content?.let { kdoc ->
@@ -49,43 +51,67 @@ internal fun generateGraphQLInputObjectTypeSpec(context: GraphQLClientGeneratorC
 
     val constructorBuilder = FunSpec.constructorBuilder()
     inputObjectDefinition.inputValueDefinitions.forEach { fieldDefinition ->
-        val kotlinFieldType = generateTypeName(context, fieldDefinition.type)
-        val fieldName = fieldDefinition.name
+        val kotlinFieldTypeName = generateTypeName(context, fieldDefinition.type)
 
-        val inputFieldType = kotlinFieldType.wrapOptionalInputType(context)
-        val inputPropertySpecBuilder = PropertySpec.builder(fieldName, inputFieldType)
-            .initializer(fieldName)
-        fieldDefinition.description?.content?.let { kdoc ->
-            inputPropertySpecBuilder.addKdoc("%L", kdoc)
+        val (rawType, isList) = unwrapRawType(kotlinFieldTypeName)
+        val isCustomScalar = context.isCustomScalar(rawType)
+        val shouldWrapInOptional = shouldWrapInOptional(kotlinFieldTypeName, context)
+
+        val inputFieldType = if (!isCustomScalar && shouldWrapInOptional) {
+            kotlinFieldTypeName.wrapOptionalInputType(context)
+        } else {
+            kotlinFieldTypeName
         }
 
-        val inputPropertySpec = inputPropertySpecBuilder.build()
+        val inputPropertySpec = PropertySpec.builder(fieldDefinition.name, inputFieldType)
+            .initializer(fieldDefinition.name)
+            .also { builder ->
+                fieldDefinition.description?.content?.let { kdoc ->
+                    builder.addKdoc("%L", kdoc)
+                }
+
+                if (isCustomScalar) {
+                    builder.addAnnotations(generateCustomScalarPropertyAnnotations(context, rawType, isList))
+
+                    if (shouldWrapInOptional) {
+                        LOGGER.warn(
+                            "Operation ${context.operationName} specifies optional custom scalar as input - ${fieldDefinition.name} in Variables. " +
+                                "Currently custom scalars do not work with optional wrappers."
+                        )
+                        builder.addKdoc("\nNOTE: This field was not wrapped in optional as currently custom scalars do not work with optional wrappers.")
+                    }
+                }
+            }
+            .build()
         inputObjectTypeSpecBuilder.addProperty(inputPropertySpec)
 
-        val inputParameterSpec = ParameterSpec.builder(inputPropertySpec.name, inputPropertySpec.type)
-        if (kotlinFieldType.isNullable) {
-            inputParameterSpec.defaultValue(nullableDefaultValueCodeBlock(context))
-        }
-        constructorBuilder.addParameter(inputParameterSpec.build())
+        constructorBuilder.addParameter(
+            ParameterSpec.builder(inputPropertySpec.name, inputPropertySpec.type)
+                .also { builder ->
+                    if (kotlinFieldTypeName.isNullable) {
+                        builder.defaultValue(nullableDefaultValueCodeBlock(context, isCustomScalar))
+                    }
+                }
+                .build()
+        )
     }
     inputObjectTypeSpecBuilder.primaryConstructor(constructorBuilder.build())
 
     return inputObjectTypeSpecBuilder.build()
 }
 
-internal fun TypeName.wrapOptionalInputType(context: GraphQLClientGeneratorContext): TypeName = if (this.isNullable && context.useOptionalInputWrapper) {
+internal fun shouldWrapInOptional(type: TypeName, context: GraphQLClientGeneratorContext) = type.isNullable && context.useOptionalInputWrapper
+
+internal fun TypeName.wrapOptionalInputType(context: GraphQLClientGeneratorContext): TypeName =
     if (context.serializer == GraphQLSerializer.JACKSON) {
         ClassName("com.expediagroup.graphql.client.jackson.types", "OptionalInput")
             .parameterizedBy(this.copy(nullable = false))
     } else {
         ClassName("com.expediagroup.graphql.client.serialization.types", "OptionalInput")
             .parameterizedBy(this.copy(nullable = false))
     }
-} else {
-    this
-}
 
-internal fun nullableDefaultValueCodeBlock(context: GraphQLClientGeneratorContext): CodeBlock = if (context.useOptionalInputWrapper) {
+internal fun nullableDefaultValueCodeBlock(context: GraphQLClientGeneratorContext, isCustomScalar: Boolean): CodeBlock = if (context.useOptionalInputWrapper && !isCustomScalar) {
     if (context.serializer == GraphQLSerializer.JACKSON) {
         CodeBlock.of("%M", MemberName("com.expediagroup.graphql.client.jackson.types", "OptionalInput.Undefined"))
     } else {

plugins/client/graphql-kotlin-client-generator/src/main/kotlin/com/expediagroup/graphql/plugin/client/generator/types/generatePropertySpecs.kt

@@ -27,6 +27,7 @@ import com.squareup.kotlinpoet.AnnotationSpec
 import com.squareup.kotlinpoet.KModifier
 import com.squareup.kotlinpoet.ParameterizedTypeName
 import com.squareup.kotlinpoet.PropertySpec
+import com.squareup.kotlinpoet.TypeName
 import graphql.Directives.DeprecatedDirective
 import graphql.language.Field
 import graphql.language.FieldDefinition
@@ -57,47 +58,16 @@ internal fun generatePropertySpecs(
             throw MissingArgumentException(context.operationName, objectName, selectedField.name, missingRequiredArguments)
         }
 
-        val nullable = fieldDefinition.type !is NonNullType
         val kotlinFieldType = generateTypeName(context, fieldDefinition.type, selectedField.selectionSet)
         val fieldName = selectedField.alias ?: fieldDefinition.name
 
-        val propertySpecBuilder = PropertySpec.builder(fieldName, kotlinFieldType.copy(nullable = nullable))
+        val propertySpecBuilder = PropertySpec.builder(fieldName, kotlinFieldType)
         if (!abstract) {
             propertySpecBuilder.initializer(fieldName)
-
-            val (unwrappedFieldType, isList) = if (kotlinFieldType is ParameterizedTypeName) {
-                kotlinFieldType.typeArguments.first() to true
-            } else {
-                kotlinFieldType to false
-            }
-
-            if (context.isCustomScalar(unwrappedFieldType)) {
-                val converterInfo = context.scalarClassToConverterTypeSpecs[unwrappedFieldType]
-                when {
-                    converterInfo is ScalarConverterInfo.JacksonConvertersInfo -> {
-                        val annotationMember = if (isList) {
-                            "contentConverter"
-                        } else {
-                            "converter"
-                        }
-                        propertySpecBuilder.addAnnotation(
-                            AnnotationSpec.builder(JsonSerialize::class)
-                                .addMember("$annotationMember = %T::class", converterInfo.serializerClassName)
-                                .build()
-                        )
-                        propertySpecBuilder.addAnnotation(
-                            AnnotationSpec.builder(JsonDeserialize::class)
-                                .addMember("$annotationMember = %T::class", converterInfo.deserializerClassName)
-                                .build()
-                        )
-                    }
-                    converterInfo is ScalarConverterInfo.KotlinxSerializerInfo && !isList -> {
-                        propertySpecBuilder.addAnnotation(
-                            AnnotationSpec.builder(Serializable::class)
-                                .addMember("with = %T::class", converterInfo.serializerClassName)
-                                .build()
-                        )
-                    }
+            val (rawType, isList) = unwrapRawType(kotlinFieldType)
+            if (context.isCustomScalar(rawType)) {
+                generateCustomScalarPropertyAnnotations(context, rawType, isList).forEach { scalarAnnotation ->
+                    propertySpecBuilder.addAnnotation(scalarAnnotation)
                 }
             }
         } else {
@@ -122,3 +92,51 @@ internal fun generatePropertySpecs(
         }
         propertySpecBuilder.build()
     }
+
+internal fun unwrapRawType(type: TypeName): Pair<TypeName, Boolean> {
+    val rawType = type.unwrapNullableType()
+    return if (rawType is ParameterizedTypeName) {
+        rawType.typeArguments.first() to true
+    } else {
+        rawType to false
+    }
+}
+
+private fun TypeName.unwrapNullableType(): TypeName = if (this.isNullable) {
+    this.copy(nullable = false)
+} else {
+    this
+}
+
+internal fun generateCustomScalarPropertyAnnotations(context: GraphQLClientGeneratorContext, rawType: TypeName, isList: Boolean): List<AnnotationSpec> {
+    val result = mutableListOf<AnnotationSpec>()
+    val converterInfo = context.scalarClassToConverterTypeSpecs[rawType]
+    when {
+        converterInfo is ScalarConverterInfo.JacksonConvertersInfo -> {
+            val annotationMember = if (isList) {
+                "contentConverter"
+            } else {
+                "converter"
+            }
+            result.add(
+                AnnotationSpec.builder(JsonSerialize::class)
+                    .addMember("$annotationMember = %T::class", converterInfo.serializerClassName)
+                    .build()
+            )
+            result.add(
+                AnnotationSpec.builder(JsonDeserialize::class)
+                    .addMember("$annotationMember = %T::class", converterInfo.deserializerClassName)
+                    .build()
+            )
+        }
+        converterInfo is ScalarConverterInfo.KotlinxSerializerInfo && !isList -> {
+            result.add(
+                AnnotationSpec.builder(Serializable::class)
+                    .addMember("with = %T::class", converterInfo.serializerClassName)
+                    .build()
+            )
+        }
+    }
+
+    return result
+}

plugins/client/graphql-kotlin-client-generator/src/main/kotlin/com/expediagroup/graphql/plugin/client/generator/types/generateTypeName.kt

@@ -24,7 +24,7 @@ import com.expediagroup.graphql.plugin.client.generator.extensions.findFragmentD
 import com.squareup.kotlinpoet.AnnotationSpec
 import com.squareup.kotlinpoet.BOOLEAN
 import com.squareup.kotlinpoet.ClassName
-import com.squareup.kotlinpoet.FLOAT
+import com.squareup.kotlinpoet.DOUBLE
 import com.squareup.kotlinpoet.INT
 import com.squareup.kotlinpoet.LIST
 import com.squareup.kotlinpoet.ParameterizedTypeName.Companion.parameterizedBy
@@ -59,7 +59,7 @@ internal fun generateTypeName(context: GraphQLClientGeneratorContext, graphQLTyp
         is NamedNode<*> -> when (graphQLType.name) {
             Scalars.GraphQLString.name -> STRING
             Scalars.GraphQLInt.name -> INT
-            Scalars.GraphQLFloat.name -> FLOAT
+            Scalars.GraphQLFloat.name -> DOUBLE
             Scalars.GraphQLBoolean.name -> BOOLEAN
             else -> generateCustomClassName(context, graphQLType, selectionSet)
         }

plugins/client/graphql-kotlin-client-generator/src/main/kotlin/com/expediagroup/graphql/plugin/client/generator/types/generateVariableTypeSpec.kt

@@ -19,6 +19,7 @@ package com.expediagroup.graphql.plugin.client.generator.types
 import com.expediagroup.graphql.client.Generated
 import com.expediagroup.graphql.plugin.client.generator.GraphQLClientGeneratorContext
 import com.expediagroup.graphql.plugin.client.generator.GraphQLSerializer
+import com.expediagroup.graphql.plugin.client.generator.LOGGER
 import com.squareup.kotlinpoet.FunSpec
 import com.squareup.kotlinpoet.KModifier
 import com.squareup.kotlinpoet.ParameterSpec
@@ -31,7 +32,8 @@ import kotlinx.serialization.Serializable
  * Generate [TypeSpec] data class wrapper for variables used within the target query.
  */
 internal fun generateVariableTypeSpec(context: GraphQLClientGeneratorContext, variableDefinitions: List<VariableDefinition>): TypeSpec? {
-    val variableTypeSpec = TypeSpec.classBuilder("Variables")
+    val variablesTypeName = "Variables"
+    val variableTypeSpec = TypeSpec.classBuilder(variablesTypeName)
         .addModifiers(KModifier.DATA)
         .addAnnotation(Generated::class)
     if (context.serializer == GraphQLSerializer.KOTLINX) {
@@ -41,16 +43,42 @@ internal fun generateVariableTypeSpec(context: GraphQLClientGeneratorContext, va
     val constructorSpec = FunSpec.constructorBuilder()
     variableDefinitions.forEach { variableDef ->
         val kotlinTypeName = generateTypeName(context, variableDef.type)
-        val variableTypeName = kotlinTypeName.wrapOptionalInputType(context)
 
-        val parameterBuilder = ParameterSpec.builder(variableDef.name, variableTypeName)
-        if (kotlinTypeName.isNullable) {
-            parameterBuilder.defaultValue(nullableDefaultValueCodeBlock(context))
+        val (rawType, isList) = unwrapRawType(kotlinTypeName)
+        val isCustomScalar = context.isCustomScalar(rawType)
+        val shouldWrapInOptional = shouldWrapInOptional(kotlinTypeName, context)
+
+        val variableTypeName = if (!isCustomScalar && shouldWrapInOptional) {
+            kotlinTypeName.wrapOptionalInputType(context)
+        } else {
+            kotlinTypeName
         }
-        constructorSpec.addParameter(parameterBuilder.build())
-        variableTypeSpec.addProperty(
-            PropertySpec.builder(variableDef.name, variableTypeName)
-                .initializer(variableDef.name)
+
+        val variable = PropertySpec.builder(variableDef.name, variableTypeName)
+            .initializer(variableDef.name)
+            .also { builder ->
+                if (isCustomScalar) {
+                    builder.addAnnotations(generateCustomScalarPropertyAnnotations(context, rawType, isList))
+
+                    if (shouldWrapInOptional) {
+                        LOGGER.warn(
+                            "Operation ${context.operationName} specifies optional custom scalar as input - ${variableDef.name} in Variables. " +
+                                "Currently custom scalars do not work with optional wrappers."
+                        )
+                        builder.addKdoc("NOTE: This field was not wrapped in optional as currently custom scalars do not work with optional wrappers.")
+                    }
+                }
+            }
+            .build()
+        variableTypeSpec.addProperty(variable)
+
+        constructorSpec.addParameter(
+            ParameterSpec.builder(variable.name, variable.type)
+                .also { builder ->
+                    if (kotlinTypeName.isNullable) {
+                        builder.defaultValue(nullableDefaultValueCodeBlock(context, isCustomScalar))
+                    }
+                }
                 .build()
         )
     }