Merge pull request #527 from k163377/github-524/fix

Improvements to serialization of `value class`.

Author: dinomite

release-notes/CREDITS-2.x

@@ -24,6 +24,7 @@ wrongwrong (k163377@github)
 * #456: Refactor KNAI.findImplicitPropertyName()
 * #449: Refactor AnnotatedMethod.hasRequiredMarker()
 * #521: Fixed lookup of instantiators
+* #527: Improvements to serialization of `value class`.
 
 Dmitri Domanine (novtor@github)
 * Contributed fix for #490: Missing value of type JsonNode? is deserialized as NullNode instead of null

src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt

@@ -11,12 +11,14 @@ import java.lang.reflect.AccessibleObject
 import java.lang.reflect.Constructor
 import java.lang.reflect.Field
 import java.lang.reflect.Method
+import kotlin.reflect.KClass
 import kotlin.reflect.KFunction
 import kotlin.reflect.KMutableProperty1
 import kotlin.reflect.KProperty1
 import kotlin.reflect.KType
 import kotlin.reflect.full.createType
 import kotlin.reflect.full.declaredMemberProperties
+import kotlin.reflect.full.memberProperties
 import kotlin.reflect.jvm.*
 
 
@@ -59,6 +61,43 @@ internal class KotlinAnnotationIntrospector(private val context: Module.SetupCon
         return super.findCreatorAnnotation(config, a)
     }
 
+    // Find a serializer to handle the case where the getter returns an unboxed value from the value class.
+    override fun findSerializer(am: Annotated): ValueClassBoxSerializer<*>? = when (am) {
+        is AnnotatedMethod -> {
+            val getter = am.member.apply {
+                // If the return value of the getter is a value class,
+                // it will be serialized properly without doing anything.
+                if (this.returnType.isUnboxableValueClass()) return null
+            }
+
+            val kotlinProperty = getter
+                .declaringClass
+                .kotlin
+                .let {
+                    // KotlinReflectionInternalError is raised in GitHub167 test,
+                    // but it looks like an edge case, so it is ignored.
+                    try {
+                        it.memberProperties
+                    } catch (e: Error) {
+                        null
+                    }
+                }?.find { it.javaGetter == getter }
+
+            (kotlinProperty?.returnType?.classifier as? KClass<*>)
+                ?.takeIf { it.isValue }
+                ?.java
+                ?.let { outerClazz ->
+                    @Suppress("UNCHECKED_CAST")
+                    ValueClassBoxSerializer(outerClazz, getter.returnType)
+                }
+        }
+        // Ignore the case of AnnotatedField, because JvmField cannot be set in the field of value class.
+        else -> null
+    }
+
+    // Perform proper serialization even if the value wrapped by the value class is null.
+    override fun findNullSerializer(am: Annotated) = findSerializer(am)
+
     /**
      * Subclasses can be detected automatically for sealed classes, since all possible subclasses are known
      * at compile-time to Kotlin. This makes [com.fasterxml.jackson.annotation.JsonSubTypes] redundant.

src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinSerializers.kt

@@ -9,6 +9,7 @@ import com.fasterxml.jackson.databind.SerializerProvider
 import com.fasterxml.jackson.databind.ser.Serializers
 import com.fasterxml.jackson.databind.ser.std.StdSerializer
 import java.math.BigInteger
+import kotlin.reflect.KClass
 
 object SequenceSerializer : StdSerializer<Sequence<*>>(Sequence::class.java) {
     override fun serialize(value: Sequence<*>, gen: JsonGenerator, provider: SerializerProvider) {
@@ -71,3 +72,20 @@ internal class KotlinSerializers : Serializers.Base() {
         else -> null
     }
 }
+
+// This serializer is used to properly serialize the value class.
+// The getter generated for the value class is special,
+// so this class will not work properly when added to the Serializers
+// (it is configured from KotlinAnnotationIntrospector.findSerializer).
+internal class ValueClassBoxSerializer<T : Any>(
+    private val outerClazz: Class<out Any>, innerClazz: Class<T>
+) : StdSerializer<T>(innerClazz) {
+    private val boxMethod = outerClazz.getMethod("box-impl", innerClazz)
+
+    override fun serialize(value: T?, gen: JsonGenerator, provider: SerializerProvider) {
+        // Values retrieved from getter are considered validated and constructor-impl is not executed.
+        val boxed = boxMethod.invoke(null, value)
+
+        provider.findValueSerializer(outerClazz).serialize(boxed, gen, provider)
+    }
+}

src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/GitHub524.kt

@@ -0,0 +1,75 @@
+package com.fasterxml.jackson.module.kotlin.test.github
+
+import com.fasterxml.jackson.core.JsonGenerator
+import com.fasterxml.jackson.databind.SerializerProvider
+import com.fasterxml.jackson.databind.annotation.JsonSerialize
+import com.fasterxml.jackson.databind.module.SimpleModule
+import com.fasterxml.jackson.databind.ser.std.StdSerializer
+import com.fasterxml.jackson.module.kotlin.jacksonMapperBuilder
+import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
+import org.junit.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertNotEquals
+
+// Most of the current behavior has been tested on GitHub464, so only serializer-related behavior is tested here.
+class GitHub524 {
+    @JvmInline
+    value class HasSerializer(val value: Int?)
+    class Serializer : StdSerializer<HasSerializer>(HasSerializer::class.java) {
+        override fun serialize(value: HasSerializer, gen: JsonGenerator, provider: SerializerProvider) {
+            gen.writeString(value.toString())
+        }
+    }
+
+    @JvmInline
+    value class NoSerializer(val value: Int?)
+
+    data class Poko(
+        // ULong has a custom serializer defined in Serializers.
+        val foo: ULong = ULong.MAX_VALUE,
+        // If a custom serializer is set, the ValueClassUnboxSerializer will be overridden.
+        val bar: HasSerializer = HasSerializer(1),
+        val baz: HasSerializer = HasSerializer(null),
+        val qux: HasSerializer? = null,
+        // If there is no serializer, it will be unboxed as the existing.
+        val quux: NoSerializer = NoSerializer(2)
+    )
+
+    @Test
+    fun test() {
+        val sm = SimpleModule()
+            .addSerializer(Serializer())
+        val writer = jacksonMapperBuilder().addModule(sm).build().writerWithDefaultPrettyPrinter()
+
+        // 18446744073709551615 is ULong.MAX_VALUE.
+        assertEquals(
+            """
+                {
+                  "foo" : 18446744073709551615,
+                  "bar" : "HasSerializer(value=1)",
+                  "baz" : "HasSerializer(value=null)",
+                  "qux" : null,
+                  "quux" : 2
+                }
+            """.trimIndent(),
+            writer.writeValueAsString(Poko())
+        )
+    }
+
+    class SerializeByAnnotation(@get:JsonSerialize(using = Serializer::class) val foo: HasSerializer = HasSerializer(1))
+
+    @Test
+    fun failing() {
+        val writer = jacksonObjectMapper().writerWithDefaultPrettyPrinter()
+
+        // JsonSerialize is not working now.
+        assertNotEquals(
+            """
+                {
+                  "foo" : "HasSerializer(value=1)"
+                }
+            """.trimIndent(),
+            writer.writeValueAsString(SerializeByAnnotation())
+        )
+    }
+}