Merge pull request #47 from ProjectMapK/improve-value-class-ser-support

Improve `value class` serialize support

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

@@ -1,8 +1,10 @@
 package com.fasterxml.jackson.module.kotlin
 
+import kotlinx.metadata.Flag
 import kotlinx.metadata.KmClass
 import kotlinx.metadata.KmConstructor
 import kotlinx.metadata.KmProperty
+import kotlinx.metadata.KmType
 import kotlinx.metadata.KmValueParameter
 import kotlinx.metadata.jvm.JvmFieldSignature
 import kotlinx.metadata.jvm.JvmMethodSignature
@@ -82,3 +84,5 @@ internal fun KmClass.findPropertyByGetter(getter: Method): KmProperty? {
     val signature = getter.toSignature()
     return properties.find { it.getterSignature == signature }
 }
+
+internal fun KmType.isNullable(): Boolean = Flag.Type.IS_NULLABLE(this.flags)

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

@@ -10,12 +10,8 @@ import com.fasterxml.jackson.databind.introspect.AnnotatedMethod
 import com.fasterxml.jackson.databind.introspect.AnnotatedParameter
 import com.fasterxml.jackson.databind.introspect.NopAnnotationIntrospector
 import com.fasterxml.jackson.databind.jsontype.NamedType
-import com.fasterxml.jackson.databind.ser.std.StdSerializer
-import com.fasterxml.jackson.module.kotlin.ser.serializers.ValueClassBoxSerializer
-import com.fasterxml.jackson.module.kotlin.ser.serializers.ValueClassStaticJsonValueSerializer
 import kotlinx.metadata.Flag
 import kotlinx.metadata.KmClass
-import kotlinx.metadata.KmClassifier
 import kotlinx.metadata.KmProperty
 import kotlinx.metadata.jvm.fieldSignature
 import kotlinx.metadata.jvm.getterSignature
@@ -53,38 +49,6 @@ internal class KotlinAnnotationIntrospector(
         null
     }
 
-    // Find a serializer to handle the case where the getter returns an unboxed value from the value class.
-    override fun findSerializer(am: Annotated): StdSerializer<*>? = 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 = cache.getKmClass(getter.declaringClass)?.findPropertyByGetter(getter)
-
-            (kotlinProperty?.returnType?.classifier as? KmClassifier.Class)?.let { classifier ->
-                // Since there was no way to directly determine whether returnType is a value class or not,
-                // Class is restored and processed.
-                // If the cost of this process is significant, consider caching it.
-                runCatching { classifier.name.reconstructClass() }
-                    .getOrNull()
-                    ?.takeIf { it.isUnboxableValueClass() }
-                    ?.let { outerClazz ->
-                        val innerClazz = getter.returnType
-
-                        ValueClassStaticJsonValueSerializer.createdOrNull(outerClazz, innerClazz)
-                            ?: ValueClassBoxSerializer(outerClazz, innerClazz)
-                    }
-            }
-        }
-        // 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.
@@ -100,7 +64,7 @@ internal class KotlinAnnotationIntrospector(
         val fieldSignature = member.toSignature()
         val byNullability = kmClass.properties
             .find { it.fieldSignature == fieldSignature }
-            ?.let { !Flag.Type.IS_NULLABLE(it.returnType.flags) }
+            ?.let { !it.returnType.isNullable() }
 
         return requiredAnnotationOrNullability(byAnnotation, byNullability)
     }
@@ -116,7 +80,7 @@ internal class KotlinAnnotationIntrospector(
         else -> byAnnotation
     }
 
-    private fun KmProperty.isRequiredByNullability(): Boolean = !Flag.Type.IS_NULLABLE(this.returnType.flags)
+    private fun KmProperty.isRequiredByNullability(): Boolean = !this.returnType.isNullable()
 
     private fun AnnotatedMethod.getRequiredMarkerFromCorrespondingAccessor(kmClass: KmClass): Boolean? {
         val memberSignature = member.toSignature()
@@ -152,7 +116,7 @@ internal class KotlinAnnotationIntrospector(
         }?.let { (paramDef, paramType) ->
             val isPrimitive = paramType.isPrimitive
             val isOptional = Flag.ValueParameter.DECLARES_DEFAULT_VALUE(paramDef.flags)
-            val isMarkedNullable = Flag.Type.IS_NULLABLE(paramDef.type.flags)
+            val isMarkedNullable = paramDef.type.isNullable()
 
             !isMarkedNullable && !isOptional &&
                 !(isPrimitive && !context.isEnabled(DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES))

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

@@ -2,6 +2,7 @@ package com.fasterxml.jackson.module.kotlin
 
 import com.fasterxml.jackson.annotation.JsonCreator
 import com.fasterxml.jackson.annotation.JsonProperty
+import com.fasterxml.jackson.databind.JsonSerializer
 import com.fasterxml.jackson.databind.cfg.MapperConfig
 import com.fasterxml.jackson.databind.introspect.Annotated
 import com.fasterxml.jackson.databind.introspect.AnnotatedConstructor
@@ -10,11 +11,13 @@ import com.fasterxml.jackson.databind.introspect.AnnotatedMember
 import com.fasterxml.jackson.databind.introspect.AnnotatedMethod
 import com.fasterxml.jackson.databind.introspect.AnnotatedParameter
 import com.fasterxml.jackson.databind.introspect.NopAnnotationIntrospector
+import com.fasterxml.jackson.databind.ser.std.StdDelegatingSerializer
 import com.fasterxml.jackson.databind.util.Converter
 import com.fasterxml.jackson.module.kotlin.deser.CollectionValueStrictNullChecksConverter
 import com.fasterxml.jackson.module.kotlin.deser.MapValueStrictNullChecksConverter
 import com.fasterxml.jackson.module.kotlin.deser.ValueClassUnboxConverter
 import com.fasterxml.jackson.module.kotlin.deser.value_instantiator.creator.ValueParameter
+import com.fasterxml.jackson.module.kotlin.ser.ValueClassBoxConverter
 import kotlinx.metadata.Flag
 import kotlinx.metadata.KmClass
 import kotlinx.metadata.KmClassifier
@@ -125,6 +128,38 @@ internal class KotlinNamesAnnotationIntrospector(
             }
         }
     }
+
+    private fun AnnotatedMethod.findValueClassBoxConverter(
+        takePredicate: (Class<*>) -> Boolean
+    ): ValueClassBoxConverter<*, *>? {
+        val getter = this.member.apply {
+            // If the return value of the getter is a value class,
+            // it will be serialized properly without doing anything.
+            // TODO: Verify the case where a value class encompasses another value class.
+            if (this.returnType.isUnboxableValueClass()) return null
+        }
+        val kotlinProperty = cache.getKmClass(getter.declaringClass)?.findPropertyByGetter(getter)
+
+        return (kotlinProperty?.returnType?.classifier as? KmClassifier.Class)?.let { classifier ->
+            // Since there was no way to directly determine whether returnType is a value class or not,
+            // Class is restored and processed.
+            // If the cost of this process is significant, consider caching it.
+            runCatching { classifier.name.reconstructClass() }
+                .getOrNull()
+                ?.takeIf { takePredicate(it) }
+                ?.let { ValueClassBoxConverter(getter.returnType, it) }
+        }
+    }
+
+    // Find a converter to handle the case where the getter returns an unboxed value from the value class.
+    override fun findSerializationConverter(a: Annotated): Converter<*, *>? = (a as? AnnotatedMethod)
+        ?.let { _ -> a.findValueClassBoxConverter { it.isUnboxableValueClass() } }
+
+    // Perform proper serialization even if the value wrapped by the value class is null.
+    // If value is a non-null object type, it must not be reboxing.
+    override fun findNullSerializer(am: Annotated): JsonSerializer<*>? = (am as? AnnotatedMethod)?.let { _ ->
+        am.findValueClassBoxConverter { it.requireRebox() }?.let { StdDelegatingSerializer(it) }
+    }
 }
 
 private fun ValueParameter.createValueClassUnboxConverterOrNull(rawType: Class<*>): ValueClassUnboxConverter<*>? {
@@ -192,3 +227,8 @@ private fun hasCreator(clazz: Class<*>, kmClass: KmClass): Boolean {
     val propertyNames = kmClass.properties.map { it.name }.toSet()
     return hasCreatorConstructor(clazz, kmClass, propertyNames) || hasCreatorFunction(clazz, kmClass)
 }
+
+// Determine if the `unbox` result of `value class` is `nullable
+// @see KotlinNamesAnnotationIntrospector.findNullSerializer
+private fun Class<*>.requireRebox(): Boolean = isUnboxableValueClass() &&
+    toKmClass()!!.properties.first { it.fieldSignature != null }.returnType.isNullable()

src/main/kotlin/com/fasterxml/jackson/module/kotlin/deser/value_instantiator/creator/ValueParameter.kt

@@ -1,5 +1,6 @@
 package com.fasterxml.jackson.module.kotlin.deser.value_instantiator.creator
 
+import com.fasterxml.jackson.module.kotlin.isNullable
 import kotlinx.metadata.Flag
 import kotlinx.metadata.KmClassifier
 import kotlinx.metadata.KmType
@@ -17,7 +18,7 @@ internal class ValueParameter(private val param: KmValueParameter) {
         }
 
         class ArgumentImpl(type: KmType) : Argument {
-            override val isNullable: Boolean = Flag.Type.IS_NULLABLE(type.flags)
+            override val isNullable: Boolean = type.isNullable()
 
             // TODO: Formatting because it is a minimal display about the error content
             override val name: String = type.classifier.toString()
@@ -28,7 +29,7 @@ internal class ValueParameter(private val param: KmValueParameter) {
     val type: KmType = param.type
     val isOptional: Boolean = Flag.ValueParameter.DECLARES_DEFAULT_VALUE(param.flags)
     val isPrimitive: Boolean = Flag.IS_PRIVATE(param.type.flags)
-    val isNullable: Boolean = Flag.Type.IS_NULLABLE(param.type.flags)
+    val isNullable: Boolean = type.isNullable()
     val isGenericType: Boolean = param.type.classifier is KmClassifier.TypeParameter
 
     val arguments: List<Argument> by lazy {

src/main/kotlin/com/fasterxml/jackson/module/kotlin/ser/ValueClassBoxConverter.kt

@@ -0,0 +1,17 @@
+package com.fasterxml.jackson.module.kotlin.ser
+
+import com.fasterxml.jackson.databind.util.StdConverter
+
+// S is nullable because value corresponds to a nullable value class
+// @see KotlinNamesAnnotationIntrospector.findNullSerializer
+internal class ValueClassBoxConverter<S : Any?, D : Any>(
+    unboxedClass: Class<S>,
+    valueClass: Class<D>
+) : StdConverter<S, D>() {
+    private val boxMethod = valueClass.getDeclaredMethod("box-impl", unboxedClass).apply {
+        if (!this.isAccessible) this.isAccessible = true
+    }
+
+    @Suppress("UNCHECKED_CAST")
+    override fun convert(value: S): D = boxMethod.invoke(null, value) as D
+}

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

@@ -50,29 +50,28 @@ internal object ULongSerializer : StdSerializer<ULong>(ULong::class.java) {
 private fun Class<*>.getStaticJsonValueGetter(): Method? = this.declaredMethods
     .find { method -> Modifier.isStatic(method.modifiers) && method.annotations.any { it is JsonValue } }
 
-internal sealed class ValueClassSerializer<T : Any>(t: Class<T>) : StdSerializer<T>(t) {
-    class StaticJsonValue<T : Any>(
-        t: Class<T>,
-        private val staticJsonValueGetter: Method
-    ) : ValueClassSerializer<T>(t) {
-        private val unboxMethod: Method = t.getMethod("unbox-impl")
+internal class ValueClassStaticJsonValueSerializer<T>(
+    t: Class<T>,
+    private val staticJsonValueGetter: Method
+) : StdSerializer<T>(t) {
+    private val unboxMethod: Method = t.getMethod("unbox-impl")
 
-        override fun serialize(value: T, gen: JsonGenerator, provider: SerializerProvider) {
-            val unboxed = unboxMethod.invoke(value)
-            // As shown in the processing of the factory function, jsonValueGetter is always a static method.
-            val jsonValue: Any? = staticJsonValueGetter.invoke(null, unboxed)
-            jsonValue
-                ?.let { provider.findValueSerializer(it::class.java).serialize(it, gen, provider) }
-                ?: provider.findNullValueSerializer(null).serialize(null, gen, provider)
-        }
+    override fun serialize(value: T, gen: JsonGenerator, provider: SerializerProvider) {
+        val unboxed = unboxMethod.invoke(value)
+        // As shown in the processing of the factory function, jsonValueGetter is always a static method.
+        val jsonValue: Any? = staticJsonValueGetter.invoke(null, unboxed)
+        jsonValue
+            ?.let { provider.findValueSerializer(it::class.java).serialize(it, gen, provider) }
+            ?: provider.findNullValueSerializer(null).serialize(null, gen, provider)
     }
 
     companion object {
         // `t` must be UnboxableValueClass.
         // If create a function with a JsonValue in the value class,
         // it will be compiled as a static method (= cannot be processed properly by Jackson),
         // so use a ValueClassSerializer.StaticJsonValue to handle this.
-        fun createOrNull(t: Class<*>): StdSerializer<*>? = t.getStaticJsonValueGetter()?.let { StaticJsonValue(t, it) }
+        fun createOrNull(t: Class<*>): StdSerializer<*>? =
+            t.getStaticJsonValueGetter()?.let { ValueClassStaticJsonValueSerializer(t, it) }
     }
 }
 
@@ -91,50 +90,8 @@ internal class KotlinSerializers : Serializers.Base() {
             UInt::class.java.isAssignableFrom(rawClass) -> UIntSerializer
             ULong::class.java.isAssignableFrom(rawClass) -> ULongSerializer
             // The priority of Unboxing needs to be lowered so as not to break the serialization of Unsigned Integers.
-            rawClass.isUnboxableValueClass() -> ValueClassSerializer.createOrNull(rawClass)
+            rawClass.isUnboxableValueClass() -> ValueClassStaticJsonValueSerializer.createOrNull(rawClass)
             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)
-    }
-}
-
-internal class ValueClassStaticJsonValueSerializer<T> private constructor(
-    innerClazz: Class<T>,
-    private val staticJsonValueGetter: Method
-) : StdSerializer<T>(innerClazz) {
-    override fun serialize(value: T?, gen: JsonGenerator, provider: SerializerProvider) {
-        // As shown in the processing of the factory function, jsonValueGetter is always a static method.
-        val jsonValue: Any? = staticJsonValueGetter.invoke(null, value)
-        jsonValue
-            ?.let { provider.findValueSerializer(it::class.java).serialize(it, gen, provider) }
-            ?: provider.findNullValueSerializer(null).serialize(null, gen, provider)
-    }
-
-    // Since JsonValue can be processed correctly if it is given to a non-static getter/field,
-    // this class will only process if it is a `static` method.
-    companion object {
-        fun <T> createdOrNull(
-            outerClazz: Class<out Any>,
-            innerClazz: Class<T>
-        ): ValueClassStaticJsonValueSerializer<T>? = outerClazz
-            .getStaticJsonValueGetter()
-            ?.let { ValueClassStaticJsonValueSerializer(innerClazz, it) }
-    }
-}

src/test/kotlin/com/fasterxml/jackson/module/kotlin/_integration/ser/value_class/serializer/ValueClasses.kt

@@ -0,0 +1,32 @@
+package com.fasterxml.jackson.module.kotlin._integration.ser.value_class.serializer
+
+import com.fasterxml.jackson.core.JsonGenerator
+import com.fasterxml.jackson.databind.SerializerProvider
+import com.fasterxml.jackson.databind.ser.std.StdSerializer
+
+@JvmInline
+value class Primitive(val v: Int) {
+    class Serializer : StdSerializer<Primitive>(Primitive::class.java) {
+        override fun serialize(value: Primitive, gen: JsonGenerator, provider: SerializerProvider) {
+            gen.writeNumber(value.v)
+        }
+    }
+}
+
+@JvmInline
+value class NonNullObject(val v: String) {
+    class Serializer : StdSerializer<NonNullObject>(NonNullObject::class.java) {
+        override fun serialize(value: NonNullObject, gen: JsonGenerator, provider: SerializerProvider) {
+            gen.writeString(value.v)
+        }
+    }
+}
+
+@JvmInline
+value class NullableObject(val v: String?) {
+    class Serializer : StdSerializer<NullableObject>(NullableObject::class.java) {
+        override fun serialize(value: NullableObject, gen: JsonGenerator, provider: SerializerProvider) {
+            value.v?.let { gen.writeString(it) } ?: gen.writeNull()
+        }
+    }
+}