From 8d8086012e6b7ae10921adb24b10f2293eca151b Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Sat, 18 Dec 2021 16:39:39 +0900 Subject: [PATCH 1/7] add ValueClassBoxSerializer --- .../jackson/module/kotlin/KotlinSerializers.kt | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinSerializers.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinSerializers.kt index 2ac960bc7..e0cee7887 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinSerializers.kt +++ b/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::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( + outerClazz: KClass, innerClazz: Class<*> +) : StdSerializer(outerClazz.java) { + private val boxMethod = _handledType.getMethod("box-impl", innerClazz) + + override fun serialize(value: Any, 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(_handledType).serialize(boxed, gen, provider) + } +} From db939c72e7ecee4df3bd683d47b812f313c02916 Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Sat, 18 Dec 2021 17:02:55 +0900 Subject: [PATCH 2/7] add findeSerializer --- .../kotlin/KotlinAnnotationIntrospector.kt | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt index 918c91d8b..7fd3e4880 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt +++ b/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,39 @@ 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 } + ?.let { outerClazz -> + @Suppress("UNCHECKED_CAST") + ValueClassBoxSerializer(outerClazz as KClass, getter.returnType) + } + } + // Ignore the case of AnnotatedField, because JvmField cannot be set in the field of value class. + else -> null + } + /** * 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. From 19c5c0292720932bc71d55a3c5b2aa5ea2bec6ef Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Sat, 18 Dec 2021 17:45:48 +0900 Subject: [PATCH 3/7] added consideration for cases where the value wrapped by value class is null --- .../jackson/module/kotlin/KotlinAnnotationIntrospector.kt | 3 +++ .../com/fasterxml/jackson/module/kotlin/KotlinSerializers.kt | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt index 7fd3e4880..6e1528833 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt @@ -94,6 +94,9 @@ internal class KotlinAnnotationIntrospector(private val context: Module.SetupCon 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. diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinSerializers.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinSerializers.kt index e0cee7887..12b65f6e0 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinSerializers.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinSerializers.kt @@ -82,7 +82,7 @@ internal class ValueClassBoxSerializer( ) : StdSerializer(outerClazz.java) { private val boxMethod = _handledType.getMethod("box-impl", innerClazz) - override fun serialize(value: Any, gen: JsonGenerator, provider: SerializerProvider) { + override fun serialize(value: Any?, gen: JsonGenerator, provider: SerializerProvider) { // Values retrieved from getter are considered validated and constructor-impl is not executed. val boxed = boxMethod.invoke(null, value) From 2de548afc85041f978e94cecdd70cedc06a91931 Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Sat, 18 Dec 2021 17:57:06 +0900 Subject: [PATCH 4/7] add test --- .../module/kotlin/test/github/GitHub524.kt | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/GitHub524.kt diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/GitHub524.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/GitHub524.kt new file mode 100644 index 000000000..29d7c2a95 --- /dev/null +++ b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/GitHub524.kt @@ -0,0 +1,55 @@ +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.module.SimpleModule +import com.fasterxml.jackson.databind.ser.std.StdSerializer +import com.fasterxml.jackson.module.kotlin.jacksonMapperBuilder +import org.junit.Test +import kotlin.test.assertEquals + +// 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?) + object Serializer : StdSerializer(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()) + ) + } +} From e4fd10d050ed21ccc1f1ca402d5495141971c197 Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Sat, 18 Dec 2021 18:33:12 +0900 Subject: [PATCH 5/7] add failing test --- .../module/kotlin/test/github/GitHub524.kt | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/GitHub524.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/GitHub524.kt index 29d7c2a95..b1c7c80bb 100644 --- a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/GitHub524.kt +++ b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/GitHub524.kt @@ -2,17 +2,20 @@ 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?) - object Serializer : StdSerializer(HasSerializer::class.java) { + class Serializer : StdSerializer(HasSerializer::class.java) { override fun serialize(value: HasSerializer, gen: JsonGenerator, provider: SerializerProvider) { gen.writeString(value.toString()) } @@ -35,7 +38,7 @@ class GitHub524 { @Test fun test() { val sm = SimpleModule() - .addSerializer(Serializer) + .addSerializer(Serializer()) val writer = jacksonMapperBuilder().addModule(sm).build().writerWithDefaultPrettyPrinter() // 18446744073709551615 is ULong.MAX_VALUE. @@ -52,4 +55,21 @@ class GitHub524 { 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()) + ) + } } From eb38c62068ea1083cf066d951d8f2dd71891129d Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Sat, 18 Dec 2021 19:16:59 +0900 Subject: [PATCH 6/7] add CREDITS --- release-notes/CREDITS-2.x | 1 + 1 file changed, 1 insertion(+) diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index 309b593f5..c4cef5066 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -22,6 +22,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 From 1660b948737eea6351804d6b47cf2892be41e884 Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Fri, 31 Dec 2021 10:13:35 +0900 Subject: [PATCH 7/7] fix generics --- .../module/kotlin/KotlinAnnotationIntrospector.kt | 5 +++-- .../jackson/module/kotlin/KotlinSerializers.kt | 12 ++++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt index 6e1528833..bb99dd8ef 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt @@ -62,7 +62,7 @@ internal class KotlinAnnotationIntrospector(private val context: Module.SetupCon } // 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) { + 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, @@ -85,9 +85,10 @@ internal class KotlinAnnotationIntrospector(private val context: Module.SetupCon (kotlinProperty?.returnType?.classifier as? KClass<*>) ?.takeIf { it.isValue } + ?.java ?.let { outerClazz -> @Suppress("UNCHECKED_CAST") - ValueClassBoxSerializer(outerClazz as KClass, getter.returnType) + ValueClassBoxSerializer(outerClazz, getter.returnType) } } // Ignore the case of AnnotatedField, because JvmField cannot be set in the field of value class. diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinSerializers.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinSerializers.kt index 12b65f6e0..2c27cc7f3 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinSerializers.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinSerializers.kt @@ -77,15 +77,15 @@ internal class KotlinSerializers : Serializers.Base() { // 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( - outerClazz: KClass, innerClazz: Class<*> -) : StdSerializer(outerClazz.java) { - private val boxMethod = _handledType.getMethod("box-impl", innerClazz) +internal class ValueClassBoxSerializer( + private val outerClazz: Class, innerClazz: Class +) : StdSerializer(innerClazz) { + private val boxMethod = outerClazz.getMethod("box-impl", innerClazz) - override fun serialize(value: Any?, gen: JsonGenerator, provider: SerializerProvider) { + 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(_handledType).serialize(boxed, gen, provider) + provider.findValueSerializer(outerClazz).serialize(boxed, gen, provider) } }