Merge pull request #931 from graphql-java-kickstart/bugfix/926-kotlin-empty-scalars

Keep the values of Kotlin custom scalars without visible properties in arguments

Author: oryan-block

src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt

@@ -4,6 +4,7 @@ import tools.jackson.core.JacksonException
 import tools.jackson.core.JsonGenerator
 import tools.jackson.core.JsonParser
 import tools.jackson.core.JsonToken
+import tools.jackson.core.util.JsonGeneratorDelegate
 import tools.jackson.databind.DeserializationContext
 import tools.jackson.databind.DeserializationFeature
 import tools.jackson.databind.JacksonSerializable
@@ -14,6 +15,7 @@ import tools.jackson.databind.SerializationFeature
 import tools.jackson.databind.deser.DeserializationProblemHandler
 import tools.jackson.databind.deser.ValueInstantiator
 import tools.jackson.databind.jsontype.TypeSerializer
+import tools.jackson.databind.ser.bean.BeanSerializerBase
 import tools.jackson.databind.util.TokenBuffer
 import java.lang.reflect.Type
 
@@ -73,16 +75,17 @@ internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) {
     }
 
     private fun convertsToOwnClass(value: Any): Boolean = try {
-        mapper.readValue(buffer(value), value.javaClass)
+        mapper.readValue(bufferFailingOnEmptyBeans(value), value.javaClass)
         true
     } catch (e: JacksonException) {
         false
     }
 
-    private fun read(value: Any?): Any? = reader.readValue(buffer(value))
+    private fun read(value: Any?): Any? = reader.readValue(bufferFailingOnEmptyBeans(value))
 
-    private fun buffer(value: Any?): TokenBuffer =
-        writer.writeValueIntoBuffer(value).forceUseOfBigDecimal(useBigDecimalForFloats)
+    private fun bufferFailingOnEmptyBeans(value: Any?): TokenBuffer = buffer(FailOnEmptyBeans(value))
+
+    private fun buffer(value: Any?): TokenBuffer = writer.writeValueIntoBuffer(value).forceUseOfBigDecimal(useBigDecimalForFloats)
 
     private val embeddedValueHandler = object : DeserializationProblemHandler() {
         // called for an interface or abstract class, e.g. Part, since no deserializer of it returns embedded values
@@ -93,13 +96,21 @@ internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) {
             p: JsonParser,
             msg: String?
         ): Any? {
-            val value = if (p.hasToken(JsonToken.VALUE_EMBEDDED_OBJECT)) p.embeddedObject else null
+            val value = p.embeddedValue()
             return if (instClass.isInstance(value)) value else NOT_HANDLED
         }
 
         // called for an embedded value read as a bean of another class, which the value is converted to on its own
         override fun handleWeirdNativeValue(context: DeserializationContext, targetType: JavaType, valueToConvert: Any, p: JsonParser): Any? =
-            mapper.readValue(buffer(valueToConvert), targetType)
+            convertOnItsOwn(valueToConvert, targetType)
+
+        // called for an embedded value read as a Map or JsonNode, e.g. a scalar without properties, which converts to {}
+        override fun handleUnexpectedToken(context: DeserializationContext, targetType: JavaType, t: JsonToken?, p: JsonParser, failureMsg: String?): Any? =
+            p.embeddedValue()?.let { convertOnItsOwn(it, targetType) } ?: NOT_HANDLED
+
+        private fun JsonParser.embeddedValue(): Any? = if (hasToken(JsonToken.VALUE_EMBEDDED_OBJECT)) embeddedObject else null
+
+        private fun convertOnItsOwn(value: Any, targetType: JavaType): Any? = mapper.readValue(buffer(value), targetType)
     }
 
     // FAIL_ON_EMPTY_BEANS whatever the mapper says: a scalar class with no visible properties would otherwise be read back as
@@ -114,6 +125,32 @@ internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) {
 
     private val nullToJavaDefaultReader by lazy { reader.without(DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES) }
 
+    // FAIL_ON_EMPTY_BEANS skips a class with annotations, which Jackson writes as an empty object even if it has no properties,
+    // e.g. any Kotlin class, since they all have @Metadata
+    private class FailOnEmptyBeans(private val value: Any?) : JacksonSerializable {
+        override fun serialize(gen: JsonGenerator, serializers: SerializationContext) {
+            serializers.writeValue(EmptyBeanCheck(gen, serializers), value)
+        }
+
+        override fun serializeWithType(gen: JsonGenerator, serializers: SerializationContext, typeSer: TypeSerializer) {
+            serialize(gen, serializers)
+        }
+    }
+
+    private class EmptyBeanCheck(gen: JsonGenerator, private val serializers: SerializationContext) : JsonGeneratorDelegate(gen, false) {
+        override fun writeEndObject(): JsonGenerator {
+            val context = streamWriteContext()
+            val bean = context.currentValue()
+            if (context.entryCount == 0 && bean != null && hasNoProperties(bean.javaClass)) {
+                serializers.reportBadDefinition<Unit>(bean.javaClass, "No properties discovered to write ${bean.javaClass.name}")
+            }
+            return super.writeEndObject()
+        }
+
+        private fun hasNoProperties(beanClass: Class<*>) =
+            (serializers.findValueSerializer(beanClass) as? BeanSerializerBase)?.properties()?.hasNext() == false
+    }
+
     private class EmbeddedValue(private val value: Any) : JacksonSerializable {
         override fun serialize(gen: JsonGenerator, serializers: SerializationContext) {
             gen.writeEmbeddedObject(value)

src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt

@@ -188,6 +188,35 @@ class CustomScalarInputTest {
         assertEquals(data, mapOf("payment" to "kept"))
     }
 
+    @Test
+    fun `Kotlin custom scalars without visible properties are passed`() {
+        val data = assertNoGraphQlErrors(graphQL(inputSchema)) {
+            """
+            query {
+                token(token: "argument")
+                optionalToken(token: "optional")
+                tokenInput(input: { token: "input" })
+                tokenNode(input: { token: "node" })
+            }
+            """
+        }
+
+        assertEquals(data, mapOf("token" to "argument", "optionalToken" to "optional", "tokenInput" to "input", "tokenNode" to "node"))
+    }
+
+    @Test
+    fun `Kotlin custom scalars without visible properties still convert to other classes`() {
+        val data = assertNoGraphQlErrors(graphQL(inputSchema)) {
+            """
+            query {
+                otherTokens(input: { bean: "bean", map: "map", node: "node" })
+            }
+            """
+        }
+
+        assertEquals(data, mapOf("otherTokens" to "OtherToken(value=other) {} {}"))
+    }
+
     @Test
     fun `errors converting inputs with custom scalars name the actual problem`() {
         val result = graphQL(inputSchema).execute(
@@ -205,7 +234,7 @@ class CustomScalarInputTest {
     private fun graphQL(schema: String, options: SchemaParserOptions = SchemaParserOptions.defaultOptions()): GraphQL {
         val graphQLSchema = SchemaParser.newParser()
             .schemaString(schema)
-            .scalars(localDateScalar, opaqueScalar, secretScalar, boxScalar, moneyScalar, customScalarUUID, uploadScalar)
+            .scalars(localDateScalar, opaqueScalar, secretScalar, boxScalar, moneyScalar, tokenScalar, customScalarUUID, uploadScalar)
             .resolvers(resolver)
             .options(options)
             .build()
@@ -221,6 +250,7 @@ class CustomScalarInputTest {
         scalar Box
         scalar Money
         scalar Upload
+        scalar Token
 
         type Query {
             event(input: EventInput!): String!
@@ -232,6 +262,11 @@ class CustomScalarInputTest {
             priceNode(input: NodeInput!): String!
             payment(input: PaymentInput!): String!
             price(price: Money!): String!
+            token(token: Token!): String!
+            optionalToken(token: Token): String!
+            tokenInput(input: TokenInput!): String!
+            tokenNode(input: NodeInput!): String!
+            otherTokens(input: OtherTokensInput!): String!
         }
 
         input EventInput {
@@ -255,6 +290,17 @@ class CustomScalarInputTest {
         input NodeInput {
             secret: Secret
             price: Money
+            token: Token
+        }
+
+        input TokenInput {
+            token: Token
+        }
+
+        input OtherTokensInput {
+            bean: Token
+            map: Token
+            node: Token
         }
 
         input PaymentInput {
@@ -290,6 +336,11 @@ class CustomScalarInputTest {
         fun priceNode(input: ObjectNode): String = input.toString()
         fun payment(input: PaymentInput): String = listOfNotNull(input.price, input.reference, input.secret, input.box).joinToString(" ")
         fun price(price: MoneyDto): String = price.toString()
+        fun token(token: Token): String = token.toString()
+        fun optionalToken(token: Optional<Token>): String = token.map { it.toString() }.orElse("empty")
+        fun tokenInput(input: TokenInput): String = input.token.toString()
+        fun otherTokens(input: OtherTokensInput): String = "${input.bean} ${input.map} ${input.node}"
+        fun tokenNode(input: ObjectNode): String = input.get("token").let { if (it is POJONode) it.pojo.toString() else "$it" }
 
         fun map(input: Map<String, Any?>) = receive("map", input)
         fun mapList(inputs: List<Map<String, Any?>>) = receive("mapList", inputs)
@@ -336,6 +387,16 @@ class CustomScalarInputTest {
 
     data class Box(val content: Any)
 
+    class Token(private val value: String = "default") {
+        override fun toString() = value
+    }
+
+    data class TokenInput(val token: Token? = null)
+
+    data class OtherToken(val value: String = "other")
+
+    data class OtherTokensInput(val bean: OtherToken? = null, val map: Map<String, String>? = null, val node: ObjectNode? = null)
+
     class BoxContentDeserializer : StdDeserializer<String>(String::class.java) {
         override fun deserialize(p: JsonParser, context: DeserializationContext): String =
             p.readValueAsTree<JsonNode>().path("content").stringValue("<missing>")
@@ -366,6 +427,7 @@ class CustomScalarInputTest {
     private val secretScalar = scalar("Secret") { Secret(it) }
     private val boxScalar = scalar("Box") { Box(if (it.startsWith("secret:")) Secret(it.removePrefix("secret:")) else it) }
     private val moneyScalar = scalar("Money") { Money(it.toInt(), "CAD") }
+    private val tokenScalar = scalar("Token") { Token(it) }
 
     private fun <T : Any> scalar(name: String, parse: (String) -> T): GraphQLScalarType = GraphQLScalarType.newScalar()
         .name(name)