Skip to content

Commit 849e1dd

Browse files
k163377k163377
authored
Merge pull request #659 from k163377/fix-value-class-ser
Improve serialization support for value class
2 parents a278d43 + 80b0d38 commit 849e1dd

File tree

7 files changed

+197
-81
lines changed

7 files changed

+197
-81
lines changed

‎src/main/kotlin/com/fasterxml/jackson/module/kotlin/Converters.kt

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package com.fasterxml.jackson.module.kotlin
2+
3+
import com.fasterxml.jackson.databind.ser.std.StdDelegatingSerializer
4+
import com.fasterxml.jackson.databind.util.StdConverter
5+
import kotlin.reflect.KClass
6+
7+
// S is nullable because value corresponds to a nullable value class
8+
// @see KotlinNamesAnnotationIntrospector.findNullSerializer
9+
internal class ValueClassBoxConverter<S : Any?, D : Any>(
10+
unboxedClass: Class<S>,
11+
valueClass: KClass<D>
12+
) : StdConverter<S, D>() {
13+
private val boxMethod = valueClass.java.getDeclaredMethod("box-impl", unboxedClass).apply {
14+
if (!this.isAccessible) this.isAccessible = true
15+
}
16+
17+
@Suppress("UNCHECKED_CAST")
18+
override fun convert(value: S): D = boxMethod.invoke(null, value) as D
19+
20+
val delegatingSerializer: StdDelegatingSerializer by lazy { StdDelegatingSerializer(this) }
21+
}

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

Lines changed: 16 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@ package com.fasterxml.jackson.module.kotlin
33
import com.fasterxml.jackson.annotation.JsonCreator
44
import com.fasterxml.jackson.annotation.JsonProperty
55
import com.fasterxml.jackson.databind.DeserializationFeature
6+
import com.fasterxml.jackson.databind.JsonSerializer
67
import com.fasterxml.jackson.databind.Module
78
import com.fasterxml.jackson.databind.cfg.MapperConfig
89
import com.fasterxml.jackson.databind.introspect.*
910
import com.fasterxml.jackson.databind.jsontype.NamedType
1011
import com.fasterxml.jackson.databind.ser.std.StdSerializer
12+
import com.fasterxml.jackson.databind.util.Converter
1113
import java.lang.reflect.AccessibleObject
1214
import java.lang.reflect.Constructor
1315
import java.lang.reflect.Field
@@ -62,44 +64,23 @@ internal class KotlinAnnotationIntrospector(private val context: Module.SetupCon
6264
return super.findCreatorAnnotation(config, a)
6365
}
6466

65-
// Find a serializer to handle the case where the getter returns an unboxed value from the value class.
66-
override fun findSerializer(am: Annotated): StdSerializer<*>? = when (am) {
67-
is AnnotatedMethod -> {
68-
val getter = am.member.apply {
69-
// If the return value of the getter is a value class,
70-
// it will be serialized properly without doing anything.
71-
if (this.returnType.isUnboxableValueClass()) return null
72-
}
73-
74-
val kotlinProperty = getter
75-
.declaringClass
76-
.kotlin
77-
.let {
78-
// KotlinReflectionInternalError is raised in GitHub167 test,
79-
// but it looks like an edge case, so it is ignored.
80-
try {
81-
it.memberProperties
82-
} catch (e: Error) {
83-
null
84-
}
85-
}?.find { it.javaGetter == getter }
86-
87-
(kotlinProperty?.returnType?.classifier as? KClass<*>)
88-
?.takeIf { it.isValue }
89-
?.java
90-
?.let { outerClazz ->
91-
val innerClazz = getter.returnType
92-
93-
ValueClassStaticJsonValueSerializer.createdOrNull(outerClazz, innerClazz)
94-
?: @Suppress("UNCHECKED_CAST") ValueClassBoxSerializer(outerClazz, innerClazz)
95-
}
96-
}
97-
// Ignore the case of AnnotatedField, because JvmField cannot be set in the field of value class.
98-
else -> null
67+
// Find a converter to handle the case where the getter returns an unboxed value from the value class.
68+
override fun findSerializationConverter(a: Annotated): Converter<*, *>? = (a as? AnnotatedMethod)?.let { _ ->
69+
cache.findValueClassReturnType(a)?.let { cache.getValueClassBoxConverter(a.rawReturnType, it) }
9970
}
10071

72+
// Determine if the unbox result of value class is nullAable
73+
// @see findNullSerializer
74+
private fun KClass<*>.requireRebox(): Boolean =
75+
this.memberProperties.first { it.javaField != null }.returnType.isMarkedNullable
76+
10177
// Perform proper serialization even if the value wrapped by the value class is null.
102-
override fun findNullSerializer(am: Annotated) = findSerializer(am)
78+
// If value is a non-null object type, it must not be reboxing.
79+
override fun findNullSerializer(am: Annotated): JsonSerializer<*>? = (am as? AnnotatedMethod)?.let { _ ->
80+
cache.findValueClassReturnType(am)
81+
?.takeIf { it.requireRebox() }
82+
?.let { cache.getValueClassBoxConverter(am.rawReturnType, it).delegatingSerializer }
83+
}
10384

10485
/**
10586
* Subclasses can be detected automatically for sealed classes, since all possible subclasses are known

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

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -109,44 +109,3 @@ internal class KotlinSerializers : Serializers.Base() {
109109
}
110110
}
111111
}
112-
113-
// This serializer is used to properly serialize the value class.
114-
// The getter generated for the value class is special,
115-
// so this class will not work properly when added to the Serializers
116-
// (it is configured from KotlinAnnotationIntrospector.findSerializer).
117-
internal class ValueClassBoxSerializer<T : Any>(
118-
private val outerClazz: Class<out Any>, innerClazz: Class<T>
119-
) : StdSerializer<T>(innerClazz) {
120-
private val boxMethod = outerClazz.getMethod("box-impl", innerClazz)
121-
122-
override fun serialize(value: T?, gen: JsonGenerator, provider: SerializerProvider) {
123-
// Values retrieved from getter are considered validated and constructor-impl is not executed.
124-
val boxed = boxMethod.invoke(null, value)
125-
126-
provider.findValueSerializer(outerClazz).serialize(boxed, gen, provider)
127-
}
128-
}
129-
130-
internal class ValueClassStaticJsonValueSerializer<T> private constructor(
131-
innerClazz: Class<T>,
132-
private val staticJsonValueGetter: Method
133-
) : StdSerializer<T>(innerClazz) {
134-
override fun serialize(value: T?, gen: JsonGenerator, provider: SerializerProvider) {
135-
// As shown in the processing of the factory function, jsonValueGetter is always a static method.
136-
val jsonValue: Any? = staticJsonValueGetter.invoke(null, value)
137-
jsonValue
138-
?.let { provider.findValueSerializer(it::class.java).serialize(it, gen, provider) }
139-
?: provider.findNullValueSerializer(null).serialize(null, gen, provider)
140-
}
141-
142-
// Since JsonValue can be processed correctly if it is given to a non-static getter/field,
143-
// this class will only process if it is a `static` method.
144-
companion object {
145-
fun <T> createdOrNull(
146-
outerClazz: Class<out Any>,
147-
innerClazz: Class<T>
148-
): ValueClassStaticJsonValueSerializer<T>? = outerClazz
149-
.getStaticJsonValueGetter()
150-
?.let { ValueClassStaticJsonValueSerializer(innerClazz, it) }
151-
}
152-
}

‎src/main/kotlin/com/fasterxml/jackson/module/kotlin/ReflectionCache.kt

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,11 @@ import java.io.Serializable
99
import java.lang.reflect.Constructor
1010
import java.lang.reflect.Executable
1111
import java.lang.reflect.Method
12+
import java.util.*
13+
import kotlin.reflect.KClass
1214
import kotlin.reflect.KFunction
15+
import kotlin.reflect.full.memberProperties
16+
import kotlin.reflect.jvm.javaGetter
1317
import kotlin.reflect.jvm.kotlinFunction
1418

1519
internal class ReflectionCache(reflectionCacheSize: Int) : Serializable {
@@ -44,6 +48,15 @@ internal class ReflectionCache(reflectionCacheSize: Int) : Serializable {
4448
private val javaConstructorIsCreatorAnnotated = LRUMap<AnnotatedConstructor, Boolean>(reflectionCacheSize, reflectionCacheSize)
4549
private val javaMemberIsRequired = LRUMap<AnnotatedMember, BooleanTriState?>(reflectionCacheSize, reflectionCacheSize)
4650

51+
// Initial size is 0 because the value class is not always used
52+
private val valueClassReturnTypeCache: LRUMap<AnnotatedMethod, Optional<KClass<*>>> =
53+
LRUMap(0, reflectionCacheSize)
54+
55+
// TODO: Consider whether the cache size should be reduced more,
56+
// since the cache is used only twice locally at initialization per property.
57+
private val valueClassBoxConverterCache: LRUMap<KClass<*>, ValueClassBoxConverter<*, *>> =
58+
LRUMap(0, reflectionCacheSize)
59+
4760
fun kotlinFromJava(key: Constructor<Any>): KFunction<Any>? = javaConstructorToKotlin.get(key)
4861
?: key.kotlinFunction?.let { javaConstructorToKotlin.putIfAbsent(key, it) ?: it }
4962

@@ -87,4 +100,44 @@ internal class ReflectionCache(reflectionCacheSize: Int) : Serializable {
87100

88101
fun javaMemberIsRequired(key: AnnotatedMember, calc: (AnnotatedMember) -> Boolean?): Boolean? = javaMemberIsRequired.get(key)?.value
89102
?: calc(key).let { javaMemberIsRequired.putIfAbsent(key, BooleanTriState.fromBoolean(it))?.value ?: it }
103+
104+
private fun AnnotatedMethod.getValueClassReturnType(): KClass<*>? {
105+
val getter = this.member.apply {
106+
// If the return value of the getter is a value class,
107+
// it will be serialized properly without doing anything.
108+
// TODO: Verify the case where a value class encompasses another value class.
109+
if (this.returnType.isUnboxableValueClass()) return null
110+
}
111+
112+
// Extract the return type from the property or getter-like.
113+
val kotlinReturnType = getter.declaringClass.kotlin
114+
.let { kClass ->
115+
// KotlinReflectionInternalError is raised in GitHub167 test,
116+
// but it looks like an edge case, so it is ignored.
117+
val prop = runCatching { kClass.memberProperties }.getOrNull()?.find { it.javaGetter == getter }
118+
(prop?.returnType ?: runCatching { getter.kotlinFunction }.getOrNull()?.returnType)
119+
?.classifier as? KClass<*>
120+
} ?: return null
121+
122+
// Since there was no way to directly determine whether returnType is a value class or not,
123+
// Class is restored and processed.
124+
return kotlinReturnType.takeIf { it.isValue }
125+
}
126+
127+
fun findValueClassReturnType(getter: AnnotatedMethod): KClass<*>? {
128+
val optional = valueClassReturnTypeCache.get(getter)
129+
130+
return if (optional != null) {
131+
optional
132+
} else {
133+
val value = Optional.ofNullable(getter.getValueClassReturnType())
134+
(valueClassReturnTypeCache.putIfAbsent(getter, value) ?: value)
135+
}.orElse(null)
136+
}
137+
138+
fun getValueClassBoxConverter(unboxedClass: Class<*>, valueClass: KClass<*>): ValueClassBoxConverter<*, *> =
139+
valueClassBoxConverterCache.get(valueClass) ?: run {
140+
val value = ValueClassBoxConverter(unboxedClass, valueClass)
141+
(valueClassBoxConverterCache.putIfAbsent(valueClass, value) ?: value)
142+
}
90143
}

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
1010
import com.fasterxml.jackson.module.kotlin.testPrettyWriter
1111
import org.junit.Test
1212
import kotlin.test.assertEquals
13-
import kotlin.test.assertNotEquals
1413

1514
// Most of the current behavior has been tested on GitHub464, so only serializer-related behavior is tested here.
1615
class GitHub524 {
@@ -59,12 +58,12 @@ class GitHub524 {
5958

6059
class SerializeByAnnotation(@get:JsonSerialize(using = Serializer::class) val foo: HasSerializer = HasSerializer(1))
6160

61+
// fixed on #659
6262
@Test
63-
fun failing() {
64-
val writer = jacksonObjectMapper().writerWithDefaultPrettyPrinter()
63+
fun getterAnnotated() {
64+
val writer = jacksonObjectMapper().testPrettyWriter()
6565

66-
// JsonSerialize is not working now.
67-
assertNotEquals(
66+
assertEquals(
6867
"""
6968
{
7069
"foo" : "HasSerializer(value=1)"

‎src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/GitHub618.kt

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package com.fasterxml.jackson.module.kotlin.test.github
2+
3+
import com.fasterxml.jackson.core.JsonGenerator
4+
import com.fasterxml.jackson.databind.SerializerProvider
5+
import com.fasterxml.jackson.databind.annotation.JsonSerialize
6+
import com.fasterxml.jackson.databind.ser.std.StdSerializer
7+
import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
8+
import org.junit.Test
9+
import kotlin.test.assertEquals
10+
11+
class GitHub618 {
12+
@JsonSerialize(using = V.Serializer::class)
13+
@JvmInline
14+
value class V(val value: String) {
15+
class Serializer : StdSerializer<V>(V::class.java) {
16+
override fun serialize(p0: V, p1: JsonGenerator, p2: SerializerProvider) {
17+
p1.writeString(p0.toString())
18+
}
19+
}
20+
}
21+
22+
data class D(val v: V?)
23+
24+
@Test
25+
fun test() {
26+
val mapper = jacksonObjectMapper()
27+
// expected: {"v":null}, but NullPointerException thrown
28+
assertEquals("""{"v":null}""", mapper.writeValueAsString(D(null)))
29+
}
30+
}

‎src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/GitHub625.kt

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
package com.fasterxml.jackson.module.kotlin.test.github
2+
3+
import com.fasterxml.jackson.annotation.JsonInclude
4+
import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
5+
import com.fasterxml.jackson.module.kotlin.testPrettyWriter
6+
import org.junit.Test
7+
import kotlin.test.assertEquals
8+
import kotlin.test.assertNotEquals
9+
10+
class GitHub625 {
11+
@JvmInline
12+
value class Primitive(val v: Int)
13+
14+
@JvmInline
15+
value class NonNullObject(val v: String)
16+
17+
@JvmInline
18+
value class NullableObject(val v: String?)
19+
20+
@JsonInclude(value = JsonInclude.Include.NON_NULL, content = JsonInclude.Include.NON_NULL)
21+
data class Dto(
22+
val primitive: Primitive? = null,
23+
val nonNullObject: NonNullObject? = null,
24+
val nullableObject: NullableObject? = null
25+
) {
26+
fun getPrimitiveGetter(): Primitive? = null
27+
fun getNonNullObjectGetter(): NonNullObject? = null
28+
fun getNullableObjectGetter(): NullableObject? = null
29+
}
30+
31+
@Test
32+
fun test() {
33+
val mapper = jacksonObjectMapper()
34+
val dto = Dto()
35+
assertEquals("{}", mapper.writeValueAsString(dto))
36+
}
37+
38+
@JsonInclude(value = JsonInclude.Include.NON_EMPTY, content = JsonInclude.Include.NON_NULL)
39+
data class FailingDto(
40+
val nullableObject1: NullableObject = NullableObject(null),
41+
val nullableObject2: NullableObject? = NullableObject(null),
42+
val map: Map<Any, Any?> = mapOf("nullableObject" to NullableObject(null),)
43+
) {
44+
fun getNullableObjectGetter1(): NullableObject = NullableObject(null)
45+
fun getNullableObjectGetter2(): NullableObject? = NullableObject(null)
46+
fun getMapGetter(): Map<Any, Any?> = mapOf("nullableObject" to NullableObject(null))
47+
}
48+
49+
@Test
50+
fun failing() {
51+
val writer = jacksonObjectMapper().testPrettyWriter()
52+
val json = writer.writeValueAsString(FailingDto())
53+
54+
assertNotEquals("{ }", json)
55+
assertEquals(
56+
"""
57+
{
58+
"nullableObject1" : null,
59+
"nullableObject2" : null,
60+
"map" : {
61+
"nullableObject" : null
62+
},
63+
"mapGetter" : {
64+
"nullableObject" : null
65+
},
66+
"nullableObjectGetter2" : null,
67+
"nullableObjectGetter1" : null
68+
}
69+
""".trimIndent(),
70+
json
71+
)
72+
}
73+
}

0 commit comments

Comments
 (0)