FasterXML / jackson-module-kotlin

Module that adds support for serialization/deserialization of Kotlin (http://kotlinlang.org) classes and data classes.
Apache License 2.0
1.11k stars 175 forks source link

Custom deserializer of inlined value class (with delegate) #792

Closed Bixilon closed 2 months ago

Bixilon commented 3 months ago

Search before asking

Describe the bug

So, I am inlining a class that just contains an int (this improves performance and memory allocation in my application).

This inlined class (color) has a custom type deserializer (I am saving it as a hex encoded string with a hashtag in front of it.

The example is crashing with:

Cannot deserialize value of type `int` from String "#654": not a valid `int` value
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1, column: 12] (through reference chain: DirectData["direct"])
com.fasterxml.jackson.databind.exc.InvalidFormatException: Cannot deserialize value of type `int` from String "#654": not a valid `int` value
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1, column: 12] (through reference chain: DirectData["direct"])
    at com.fasterxml.jackson.databind.exc.InvalidFormatException.from(InvalidFormatException.java:67)
    at com.fasterxml.jackson.databind.DeserializationContext.weirdStringException(DeserializationContext.java:1958)
    at com.fasterxml.jackson.databind.DeserializationContext.handleWeirdStringValue(DeserializationContext.java:1245)
    at com.fasterxml.jackson.databind.deser.std.StdDeserializer._parseIntPrimitive(StdDeserializer.java:775)
    at com.fasterxml.jackson.databind.deser.std.StdDeserializer._parseIntPrimitive(StdDeserializer.java:753)
    at com.fasterxml.jackson.databind.deser.std.NumberDeserializers$IntegerDeserializer.deserialize(NumberDeserializers.java:529)
    at com.fasterxml.jackson.databind.deser.std.NumberDeserializers$IntegerDeserializer.deserialize(NumberDeserializers.java:506)
    at com.fasterxml.jackson.databind.deser.impl.FieldProperty.deserializeAndSet(FieldProperty.java:138)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.vanillaDeserialize(BeanDeserializer.java:310)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:177)
    at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:342)
    at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4905)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3848)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3831)

The problem is, that the deserializer is not regionized by jackson. Decompiled, the whole construct looks like this:

public final class DelegateData {
   private int direct = Color.constructor-impl(123);

   public final int getDirect_JaeQ8us/* $FF was: getDirect-JaeQ8us*/() {
      return this.direct;
   }

   public final void setDirect_5o48kM4/* $FF was: setDirect-5o48kM4*/(int var1) {
      this.direct = var1;
   }
}

The getter/setter is acutally called getDelegate_JaeQ8us and setDelegate_5o48kM4. The custom deserializer is not even called, only when I add @get:JsonDeserialize(using = Deserializer::class) @set:JsonDeserialize(using = Deserializer::class)

That was just the simple case, in real I got a delegate behind it (by).

To Reproduce

generic code:

import com.fasterxml.jackson.core.JsonParser
import com.fasterxml.jackson.databind.DeserializationContext
import com.fasterxml.jackson.databind.deser.std.StdDeserializer
import com.fasterxml.jackson.databind.json.JsonMapper
import com.fasterxml.jackson.databind.module.SimpleModule
import com.fasterxml.jackson.module.kotlin.KotlinModule
import com.fasterxml.jackson.module.kotlin.readValue
import org.testng.Assert.assertEquals
import org.testng.annotations.Test
import kotlin.reflect.KProperty

@JvmInline
value class Color(val color: Int)

object Deserializer : StdDeserializer<Color>(Color::class.java) {

    override fun deserialize(parser: JsonParser, context: DeserializationContext?): Color {
        val string = parser.valueAsString
        if (!string.startsWith("#")) throw IllegalArgumentException()
        return Color(string.removePrefix("#").toInt())
    }
}

direct field:

class DirectData {
    var direct = Color(123)
}
    @Test
    fun test() {
        val mapper = JsonMapper.builder().build().registerModule(KotlinModule.Builder().build()).registerModule(SimpleModule().apply { addDeserializer(Color::class.java, Deserializer) })

        val data = """{"direct": "#654"}"""
        val read: DirectData = mapper.readValue(data)
        assertEquals(read.direct.color, 654)
    }

delegate field:

class Delegate(var value: Color) {
    operator fun getValue(test: Any, property: KProperty<*>) = this.value

    operator fun setValue(test: Any, property: KProperty<*>, color: Color) {
        this.value = color
    }
}

class DelegateData {
    var delegate by Delegate(Color(123))
}
    @Test
    fun delegate() {
        val mapper = JsonMapper.builder().build().registerModule(KotlinModule.Builder().build()).registerModule(SimpleModule().apply { addDeserializer(Color::class.java, Deserializer) })

        val data = """{"delegate": "#654"}"""
        val read: DelegateData = mapper.readValue(data)
        assertEquals(read.delegate.color, 654)
    }

Expected behavior

The tests should pass

Versions

Kotlin: Jackson-module-kotlin: 2.17.0 Jackson-databind: 2.17.0 kotlin: 2.0 (maybe that makes a difference)

Additional context

No response

k163377 commented 2 months ago

I do not plan to implement setter support for value class at this time. The reasons are as follows

So, I am inlining a class that just contains an int (this improves performance and memory allocation in my application).

I would suggest benchmarking first. For libraries using kotlin-reflect, performance will degrade if you use the value class. Compared to using data class, for example, the throughput of deserialization is roughly 70% (see the benchmark results in the link). https://github.com/FasterXML/jackson-module-kotlin/blob/2.18/docs/value-class-support.md#note-on-the-use-of-value-class

Bixilon commented 2 months ago

Requires a very complex implementation.

Yes, agreed. I thought that is something already.

I would suggest benchmarking first.

My application does not use jackson heavily, it is more something for reading and writing settings (and a couple of other things). The main improvement comes from memory allocation, the color class is not mutable, and I already started replacing colors with int in a lot of places. The memory allocation goes down by 30-50 MB/s, it makes the gc almost sleep. In my case the jackson sacrifize would be totally worth it (even if it is slower by 10 times) to make the code quality go up and not use ints in the code. But I guess this is only true in my case and a lot of people would not want to take this.

Thanks for the answer, I need to find a different way (maybe use a color wrapper class or so for config files).

k163377 commented 2 months ago

The main improvement comes from memory allocation

Hmmm, if that is the case, it might be preferable not to use jackson-module-kotlin. I haven't done a comparison, but as far as I know, this module uses more memory because it uses kotlin-reflect.

If you are looking for performance, since Jackson supports record classes, you might consider defining DTO as JvmRecord and not using kotlinModule. Alternatively, kotlinx-serialization does not use reflection, which may also reduce runtime memory consumption (I believe value class is also supported).