Reuse the wrapped type's deserializer in value class deserializers (#1250)

src/main/kotlin/tools/jackson/module/kotlin/KotlinDeserializers.kt

@@ -1,6 +1,7 @@
 package tools.jackson.module.kotlin
 
 import tools.jackson.core.JsonParser
+import tools.jackson.core.JsonToken.VALUE_NULL
 import tools.jackson.core.JsonToken.VALUE_NUMBER_INT
 import tools.jackson.core.exc.InputCoercionException
 import tools.jackson.databind.BeanDescription
@@ -93,12 +94,41 @@ object ULongDeserializer : StdDeserializer<ULong>(ULong::class.java) {
         )
 }
 
+/**
+ * Reads the value wrapped by a value class with the deserializer of [inputType].
+ *
+ * The deserializer is looked up once and then reused.
+ * `p.readValueAs(inputType)` looks it up for every value instead, and that lookup is not cached
+ * for deserializers that are not cachable (e.g. `BigDecimal`, `UUID` or `java.time` types),
+ * so they were constructed again for every value (#1250).
+ */
+internal class WrappedValueReader(private val inputType: Class<*>) {
+    // Looked up on first use, like `readValueAs()` did, so that the lookup (and any error it reports)
+    // still happens only when a value is actually read.
+    @Volatile
+    private var deserializer: ValueDeserializer<Any>? = null
+
+    private fun findDeserializer(ctxt: DeserializationContext): ValueDeserializer<Any> {
+        val type = ctxt.constructType(inputType)
+        return ctxt.findRootValueDeserializer(type)
+            ?: ctxt.reportBadDefinition(type, "Could not find `ValueDeserializer` for type ${ClassUtil.getTypeDescription(type)}")
+    }
+
+    // Same handling as DeserializationContext.readValue, which `p.readValueAs()` delegates to.
+    fun read(p: JsonParser, ctxt: DeserializationContext): Any? {
+        val deser = deserializer ?: findDeserializer(ctxt).also { deserializer = it }
+        if (!p.hasCurrentToken()) p.nextToken()
+        return if (p.hasToken(VALUE_NULL)) deser.getNullValue(ctxt) else deser.deserialize(p, ctxt)
+    }
+}
+
 // If the creator does not perform type conversion, implement a unique deserializer for each for fast invocation.
 internal sealed class NoConversionCreatorBoxDeserializer<S, D : Any>(
     creator: Method,
     converter: ValueClassBoxConverter<S, D>,
+    inputType: Class<*>,
 ) : WrapsNullableValueClassDeserializer<D>(converter.boxedClass) {
-    protected abstract val inputType: Class<*>
+    private val reader = WrappedValueReader(inputType)
     protected val handle: MethodHandle = MethodHandles
         .filterReturnValue(unreflectWithAccessibilityModification(creator), converter.boxHandle)
 
@@ -118,45 +148,37 @@ internal sealed class NoConversionCreatorBoxDeserializer<S, D : Any>(
 
     final override fun deserialize(p: JsonParser, ctxt: DeserializationContext): D {
         @Suppress("UNCHECKED_CAST")
-        return invokeExact(p.readValueAs(inputType) as S)
+        return invokeExact(reader.read(p, ctxt) as S)
     }
 
     internal class WrapsInt<D : Any>(
         creator: Method,
         converter: IntValueClassBoxConverter<D>,
-    ) : NoConversionCreatorBoxDeserializer<Int, D>(creator, converter) {
-        override val inputType get() = Int::class.java
-
+    ) : NoConversionCreatorBoxDeserializer<Int, D>(creator, converter, Int::class.java) {
         @Suppress("UNCHECKED_CAST")
         override fun invokeExact(value: Int): D = handle.invokeExact(value) as D
     }
 
     internal class WrapsLong<D : Any>(
         creator: Method,
         converter: LongValueClassBoxConverter<D>,
-    ) : NoConversionCreatorBoxDeserializer<Long, D>(creator, converter) {
-        override val inputType get() = Long::class.java
-
+    ) : NoConversionCreatorBoxDeserializer<Long, D>(creator, converter, Long::class.java) {
         @Suppress("UNCHECKED_CAST")
         override fun invokeExact(value: Long): D = handle.invokeExact(value) as D
     }
 
     internal class WrapsString<D : Any>(
         creator: Method,
         converter: StringValueClassBoxConverter<D>,
-    ) : NoConversionCreatorBoxDeserializer<String?, D>(creator, converter) {
-        override val inputType get() = String::class.java
-
+    ) : NoConversionCreatorBoxDeserializer<String?, D>(creator, converter, String::class.java) {
         @Suppress("UNCHECKED_CAST")
         override fun invokeExact(value: String?): D = handle.invokeExact(value) as D
     }
 
     internal class WrapsJavaUuid<D : Any>(
         creator: Method,
         converter: JavaUuidValueClassBoxConverter<D>,
-    ) : NoConversionCreatorBoxDeserializer<UUID?, D>(creator, converter) {
-        override val inputType get() = UUID::class.java
-
+    ) : NoConversionCreatorBoxDeserializer<UUID?, D>(creator, converter, UUID::class.java) {
         @Suppress("UNCHECKED_CAST")
         override fun invokeExact(value: UUID?): D = handle.invokeExact(value) as D
     }
@@ -199,10 +221,9 @@ internal class HasConversionCreatorWrapsSpecifiedBoxDeserializer<S, D : Any>(
     @Suppress("UNCHECKED_CAST")
     private fun instantiate(input: Any?): D = handle.invokeExact(input) as D
 
-    override fun deserialize(p: JsonParser, ctxt: DeserializationContext): D {
-        val input = p.readValueAs(inputType)
-        return instantiate(input)
-    }
+    private val reader = WrappedValueReader(inputType)
+
+    override fun deserialize(p: JsonParser, ctxt: DeserializationContext): D = instantiate(reader.read(p, ctxt))
 }
 
 internal class WrapsAnyValueClassBoxDeserializer<S, D : Any>(
@@ -229,10 +250,9 @@ internal class WrapsAnyValueClassBoxDeserializer<S, D : Any>(
     @Suppress("UNCHECKED_CAST")
     private fun instantiate(input: Any?): D = handle.invokeExact(input) as D
 
-    override fun deserialize(p: JsonParser, ctxt: DeserializationContext): D {
-        val input = p.readValueAs(inputType)
-        return instantiate(input)
-    }
+    private val reader = WrappedValueReader(inputType)
+
+    override fun deserialize(p: JsonParser, ctxt: DeserializationContext): D = instantiate(reader.read(p, ctxt))
 }
 
 private fun invalidCreatorMessage(m: Method): String =

src/test/kotlin/tools/jackson/module/kotlin/test/github/GitHub1250.kt

@@ -0,0 +1,140 @@
+package tools.jackson.module.kotlin.test.github
+
+import com.fasterxml.jackson.annotation.JsonCreator
+import tools.jackson.databind.BeanDescription
+import tools.jackson.databind.DeserializationConfig
+import tools.jackson.databind.ValueDeserializer
+import tools.jackson.databind.deser.ValueDeserializerModifier
+import tools.jackson.databind.module.SimpleModule
+import tools.jackson.module.kotlin.defaultMapper
+import tools.jackson.module.kotlin.jacksonMapperBuilder
+import tools.jackson.module.kotlin.readValue
+import java.math.BigDecimal
+import java.time.LocalDate
+import java.util.UUID
+import kotlin.test.Test
+import kotlin.test.assertEquals
+
+class GitHub1250 {
+    // WrapsAnyValueClassBoxDeserializer
+    @JvmInline
+    value class Amount(val value: BigDecimal)
+
+    @JvmInline
+    value class Day(val value: LocalDate)
+
+    @JvmInline
+    value class NullableAmount(val value: BigDecimal?)
+
+    // NoConversionCreatorBoxDeserializer.WrapsJavaUuid
+    @JvmInline
+    value class Id(val value: UUID)
+
+    // HasConversionCreatorWrapsSpecifiedBoxDeserializer
+    @JvmInline
+    value class Converted(val value: Int) {
+        companion object {
+            @JvmStatic
+            @JsonCreator
+            fun of(value: BigDecimal): Converted = Converted(value.toInt())
+        }
+    }
+
+    // Wrapped type refers back to the value class
+    @JvmInline
+    value class Node(val value: Tree)
+
+    data class Tree(val children: List<Node>)
+
+    data class Dto(
+        val amount: Amount,
+        val day: Day,
+        val nullableAmount: NullableAmount,
+        val nullableProperty: Amount?,
+        val id: Id,
+        val converted: Converted,
+        val amounts: List<Amount>,
+        val days: Map<String, Day>,
+    )
+
+    private fun json(i: Int) = """
+        {
+          "amount": $i.50,
+          "day": "2024-01-${(i % 28 + 1).toString().padStart(2, '0')}",
+          "nullableAmount": null,
+          "nullableProperty": null,
+          "id": "00000000-0000-0000-0000-${i.toString().padStart(12, '0')}",
+          "converted": $i.9,
+          "amounts": [1.0, $i],
+          "days": { "a": "2024-02-29" }
+        }
+    """.trimIndent()
+
+    private fun expected(i: Int) = Dto(
+        Amount(BigDecimal("$i.50")),
+        Day(LocalDate.of(2024, 1, i % 28 + 1)),
+        NullableAmount(null),
+        null,
+        Id(UUID.fromString("00000000-0000-0000-0000-${i.toString().padStart(12, '0')}")),
+        Converted(i),
+        listOf(Amount(BigDecimal("1.0")), Amount(BigDecimal(i))),
+        mapOf("a" to Day(LocalDate.of(2024, 2, 29))),
+    )
+
+    private val input = (0 until 10).joinToString(",", "[", "]") { json(it) }
+
+    // Counts how often a deserializer is constructed (= DeserializerCache misses), per type
+    class ConstructionCounter : ValueDeserializerModifier() {
+        val counts = mutableMapOf<String, Int>()
+
+        @Synchronized
+        override fun modifyDeserializer(
+            config: DeserializationConfig,
+            beanDescRef: BeanDescription.Supplier,
+            deserializer: ValueDeserializer<*>,
+        ): ValueDeserializer<*> {
+            counts.merge(beanDescRef.type.rawClass.simpleName, 1, Int::plus)
+            return deserializer
+        }
+
+        @Synchronized
+        fun snapshot(): Map<String, Int> = counts.toMap()
+    }
+
+    @Test
+    fun wrappedDeserializersAreNotConstructedPerValue() {
+        val counter = ConstructionCounter()
+        val mapper = jacksonMapperBuilder()
+            .addModule(SimpleModule().setDeserializerModifier(counter))
+            .build()
+
+        assertEquals((0 until 10).map { expected(it) }, mapper.readValue<List<Dto>>(input))
+        val afterFirstRead = counter.snapshot()
+        assertEquals((0 until 10).map { expected(it) }, mapper.readValue<List<Dto>>(input))
+
+        assertEquals(afterFirstRead, counter.snapshot())
+    }
+
+    @Test
+    fun values() {
+        assertEquals(expected(3), defaultMapper.readValue<Dto>(json(3)))
+        assertEquals(Amount(BigDecimal("1.50")), defaultMapper.readValue<Amount>("1.50"))
+        assertEquals(Day(LocalDate.of(2024, 1, 2)), defaultMapper.readValue<Day>("\"2024-01-02\""))
+        assertEquals(NullableAmount(BigDecimal.TEN), defaultMapper.readValue<NullableAmount>("10"))
+        assertEquals(Converted(1), defaultMapper.readValue<Converted>("1.5"))
+        assertEquals(
+            listOf(Id(UUID(0, 1)), Id(UUID(0, 2))),
+            defaultMapper.readValue<List<Id>>("""["00000000-0000-0000-0000-000000000001","00000000-0000-0000-0000-000000000002"]""")
+        )
+        assertEquals(listOf(null, Amount(BigDecimal.ONE)), defaultMapper.readValue<List<Amount?>>("[null, 1]"))
+    }
+
+    @Test
+    fun recursiveWrappedType() {
+        val leaf = Node(Tree(emptyList()))
+        assertEquals(
+            Node(Tree(listOf(leaf, Node(Tree(listOf(leaf)))))),
+            defaultMapper.readValue<Node>("""{"children":[{"children":[]},{"children":[{"children":[]}]}]}""")
+        )
+    }
+}