Merge branch 'master' into tatu/3.0/887-use-module-info

Author: cowtowncoder

README.md

@@ -1,5 +1,6 @@
 [![Maven Central](https://maven-badges.herokuapp.com/maven-central/com.fasterxml.jackson.module/jackson-module-kotlin/badge.svg)](https://mvnrepository.com/artifact/com.fasterxml.jackson.module/jackson-module-kotlin)
 [![Change log](https://img.shields.io/badge/change%20log-%E2%96%A4-yellow.svg)](./release-notes/VERSION-2.x)
+[![Tidelift](https://tidelift.com/badges/package/maven/com.fasterxml.jackson.module:jackson-module-kotlin)](https://tidelift.com/subscription/pkg/maven-com-fasterxml-jackson-module-jackson-module-kotlin?utm_source=maven-com-fasterxml-jackson-module-jackson-module-kotlin&utm_medium=referral&utm_campaign=readme)
 [![Kotlin Slack](https://img.shields.io/badge/chat-kotlin%20slack-orange.svg)](https://slack.kotlinlang.org/)
 
 # Overview

release-notes/CREDITS-2.x

@@ -18,6 +18,7 @@ Contributors:
 # 2.19.0 (not yet released)
 
 WrongWrong (@k163377)
+* #910: Add default KeyDeserializer for value class
 * #885: Performance improvement of strictNullChecks
 * #884: Changed the base class of MissingKotlinParameterException to InvalidNullException
 * #878: Fix for #876
@@ -28,6 +29,13 @@ WrongWrong (@k163377)
 * #839: Remove useKotlinPropertyNameForGetter and unify with kotlinPropertyNameAsImplicitName
 * #835: Remove old SingletonSupport class and unified with KotlinFeature.SingletonSupport
 
+# 2.18.3 (not yet released)
+
+WrongWrong (@k163377)
+* #908: Additional fixes related to #904.
+* #904: Fixed an error when serializing a `value class` that wraps a `Map`
+* #900: Fixed an issue where some tests were not running
+
 # 2.18.0 (26-Sep-2024)
 
 WrongWrong (@k163377)

release-notes/VERSION-2.x

@@ -18,6 +18,8 @@ Co-maintainers:
 
 2.19.0 (not yet released)
 
+#910: A default `KeySerializer` for `value class` has been added.
+ This eliminates the need to have a custom `KeySerializer` for each `value class` when using it as a key in a `Map`, if only simple boxing is needed.
 #889: Kotlin has been upgraded to 1.9.25.
 #885: A new `StrictNullChecks` option(KotlinFeature.NewStrictNullChecks) has been added which greatly improves throughput.
  Benchmarks show a consistent throughput drop of less than 2% when enabled (prior to the improvement, the worst throughput drop was more than 30%).
@@ -34,6 +36,12 @@ Co-maintainers:
 #839: Remove useKotlinPropertyNameForGetter and unify with kotlinPropertyNameAsImplicitName.
 #835: Remove old SingletonSupport class and unified with KotlinFeature.SingletonSupport.
 
+2.18.3 (not yet released)
+
+#904: Fixed a problem where context was not being propagated properly when serializing an unboxed value of `value class`
+  or a value retrieved with `JsonValue`.
+  This fixes a problem where an error would occur when serializing a `value class` that wraps a `Map`(#873).
+
 2.18.2 (27-Nov-2024)
 2.18.1 (28-Oct-2024)
 

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

@@ -5,6 +5,11 @@ import tools.jackson.core.exc.InputCoercionException
 import tools.jackson.databind.*
 import tools.jackson.databind.deser.jdk.JDKKeyDeserializer
 import tools.jackson.databind.deser.jdk.JDKKeyDeserializers
+import tools.jackson.databind.exc.InvalidDefinitionException
+import java.lang.reflect.Method
+import kotlin.reflect.KClass
+import kotlin.reflect.full.primaryConstructor
+import kotlin.reflect.jvm.javaMethod
 
 // The reason why key is treated as nullable is to match the tentative behavior of JDKKeyDeserializer.
 // If JDKKeyDeserializer is modified, need to modify this too.
@@ -57,16 +62,68 @@ internal object ULongKeyDeserializer : JDKKeyDeserializer(TYPE_LONG, ULong::clas
     }
 }
 
-internal object KotlinKeyDeserializers : JDKKeyDeserializers() {
+// The implementation is designed to be compatible with various creators, just in case.
+internal class ValueClassKeyDeserializer<S, D : Any>(
+    private val creator: Method,
+    private val converter: ValueClassBoxConverter<S, D>
+) : KeyDeserializer() {
+    private val unboxedClass: Class<*> = creator.parameterTypes[0]
+
+    init {
+        creator.apply { if (!this.isAccessible) this.isAccessible = true }
+    }
+
+    // Based on databind error
+    // https://github.com/FasterXML/jackson-databind/blob/341f8d360a5f10b5e609d6ee0ea023bf597ce98a/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java#L624
+    private fun errorMessage(boxedType: JavaType): String =
+        "Could not find (Map) Key deserializer for types wrapped in $boxedType"
+
+    override fun deserializeKey(key: String?, ctxt: DeserializationContext): Any {
+        val unboxedJavaType = ctxt.constructType(unboxedClass)
+
+        return try {
+            // findKeyDeserializer does not return null, and an exception will be thrown if not found.
+            val value = ctxt.findKeyDeserializer(unboxedJavaType, null).deserializeKey(key, ctxt)
+            @Suppress("UNCHECKED_CAST")
+            converter.convert(creator.invoke(null, value) as S)
+        } catch (e: InvalidDefinitionException) {
+            throw DatabindException.from(ctxt.parser, errorMessage(ctxt.constructType(converter.boxedClass.java)), e)
+        }
+    }
+
+    companion object {
+        fun createOrNull(
+            boxedClass: KClass<*>,
+            cache: ReflectionCache
+        ): ValueClassKeyDeserializer<*, *>? {
+            // primaryConstructor.javaMethod for the value class returns constructor-impl
+            // Only primary constructor is allowed as creator, regardless of visibility.
+            // This is because it is based on the WrapsNullableValueClassBoxDeserializer.
+            // Also, as far as I could research, there was no such functionality as JsonKeyCreator,
+            // so it was not taken into account.
+            val creator = boxedClass.primaryConstructor?.javaMethod ?: return null
+            val converter = cache.getValueClassBoxConverter(creator.returnType, boxedClass)
+
+            return ValueClassKeyDeserializer(creator, converter)
+        }
+    }
+}
+
+internal class KotlinKeyDeserializers(private val cache: ReflectionCache) : JDKKeyDeserializers() {
     override fun findKeyDeserializer(
         type: JavaType,
         config: DeserializationConfig?,
         beanDesc: BeanDescription?,
-    ): KeyDeserializer? = when (type.rawClass) {
-        UByte::class.java -> UByteKeyDeserializer
-        UShort::class.java -> UShortKeyDeserializer
-        UInt::class.java -> UIntKeyDeserializer
-        ULong::class.java -> ULongKeyDeserializer
-        else -> null
+    ): KeyDeserializer? {
+        val rawClass = type.rawClass
+
+        return when {
+            rawClass == UByte::class.java -> UByteKeyDeserializer
+            rawClass == UShort::class.java -> UShortKeyDeserializer
+            rawClass == UInt::class.java -> UIntKeyDeserializer
+            rawClass == ULong::class.java -> ULongKeyDeserializer
+            rawClass.isUnboxableValueClass() -> ValueClassKeyDeserializer.createOrNull(rawClass.kotlin, cache)
+            else -> null
+        }
     }
 }

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

@@ -19,8 +19,6 @@ fun Class<*>.isKotlinClass(): Boolean = this.isAnnotationPresent(Metadata::class
  *  using the default value provided in Kotlin.
  * @property singletonSupport        Default: false.  Mode for singleton handling.
  *  See [KotlinFeature.SingletonSupport]
- * @property enabledSingletonSupport Default: false.  A temporary property that is maintained until the return value of `singletonSupport` is changed.
- *  It will be removed in 2.21.
  * @property strictNullChecks        Default: false.  Whether to check deserialized collections.  With this disabled,
  *  the default, collections which are typed to disallow null members
  *  (e.g. List<String>) may contain null values after deserialization.  Enabling it
@@ -107,7 +105,7 @@ class KotlinModule private constructor(
         )
 
         context.addDeserializers(KotlinDeserializers(cache, useJavaDurationConversion))
-        context.addKeyDeserializers(KotlinKeyDeserializers)
+        context.addKeyDeserializers(KotlinKeyDeserializers(cache))
         context.addSerializers(KotlinSerializers())
         context.addKeySerializers(KotlinKeySerializers())
 

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

@@ -50,13 +50,7 @@ private fun Class<*>.getStaticJsonValueGetter(): Method? = this.declaredMethods.
 object ValueClassUnboxSerializer : StdSerializer<Any>(Any::class.java) {
     override fun serialize(value: Any, gen: JsonGenerator, ctxt: SerializationContext) {
         val unboxed = value::class.java.getMethod("unbox-impl").invoke(value)
-
-        if (unboxed == null) {
-            ctxt.findNullValueSerializer(null).serialize(null, gen, ctxt)
-            return
-        }
-
-        ctxt.findValueSerializer(unboxed::class.java).serialize(unboxed, gen, ctxt)
+        ctxt.writeValue(gen, unboxed)
     }
 }
 
@@ -70,9 +64,7 @@ internal sealed class ValueClassSerializer<T : Any>(t: Class<T>) : StdSerializer
             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 { ctxt.findValueSerializer(it::class.java).serialize(it, gen, ctxt) }
-                ?: ctxt.findNullValueSerializer(null).serialize(null, gen, ctxt)
+            ctxt.writeValue(gen, jsonValue)
         }
     }
 

src/test/kotlin/tools/jackson/module/kotlin/kogeraIntegration/deser/StrictNullChecksTest.kt

@@ -1,4 +1,4 @@
-package com.fasterxml.jackson.module.kotlin.kogeraIntegration.deser
+package tools.jackson.module.kotlin.kogeraIntegration.deser
 
 import com.fasterxml.jackson.annotation.JsonSetter
 import com.fasterxml.jackson.annotation.Nulls

src/test/kotlin/tools/jackson/module/kotlin/kogeraIntegration/deser/valueClass/ValueClasses.kt

@@ -4,12 +4,17 @@ import tools.jackson.core.JsonParser
 import tools.jackson.databind.DeserializationContext
 import tools.jackson.databind.deser.std.StdDeserializer
 import tools.jackson.module.kotlin.WrapsNullableValueClassDeserializer
+import tools.jackson.databind.KeyDeserializer as JacksonKeyDeserializer
 
 @JvmInline
 value class Primitive(val v: Int) {
     class Deserializer : StdDeserializer<Primitive>(Primitive::class.java) {
         override fun deserialize(p: JsonParser, ctxt: DeserializationContext): Primitive = Primitive(p.intValue + 100)
     }
+
+    class KeyDeserializer : JacksonKeyDeserializer() {
+        override fun deserializeKey(key: String, ctxt: DeserializationContext) = Primitive(key.toInt() + 100)
+    }
 }
 
 @JvmInline
@@ -18,6 +23,10 @@ value class NonNullObject(val v: String) {
         override fun deserialize(p: JsonParser, ctxt: DeserializationContext): NonNullObject =
             NonNullObject(p.valueAsString + "-deser")
     }
+
+    class KeyDeserializer : JacksonKeyDeserializer() {
+        override fun deserializeKey(key: String, ctxt: DeserializationContext) = NonNullObject("$key-deser")
+    }
 }
 
 @JvmInline
@@ -28,4 +37,8 @@ value class NullableObject(val v: String?) {
 
         override fun getBoxedNullValue(): NullableObject = NullableObject("null-value-deser")
     }
+
+    class KeyDeserializer : JacksonKeyDeserializer() {
+        override fun deserializeKey(key: String, ctxt: DeserializationContext) = NullableObject("$key-deser")
+    }
 }

src/test/kotlin/tools/jackson/module/kotlin/kogeraIntegration/deser/valueClass/mapKey/WithoutCustomDeserializeMethodTest.kt

@@ -0,0 +1,118 @@
+package tools.jackson.module.kotlin.kogeraIntegration.deser.valueClass.mapKey
+
+import tools.jackson.module.kotlin.defaultMapper
+import tools.jackson.module.kotlin.readValue
+import tools.jackson.module.kotlin.kogeraIntegration.deser.valueClass.NonNullObject
+import tools.jackson.module.kotlin.kogeraIntegration.deser.valueClass.NullableObject
+import tools.jackson.module.kotlin.kogeraIntegration.deser.valueClass.Primitive
+import org.junit.jupiter.api.Assertions.assertEquals
+import org.junit.jupiter.api.Assertions.assertTrue
+import org.junit.jupiter.api.Nested
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.assertThrows
+import tools.jackson.databind.DatabindException
+import tools.jackson.databind.DeserializationContext
+import tools.jackson.databind.exc.InvalidDefinitionException
+import tools.jackson.databind.module.SimpleModule
+import tools.jackson.module.kotlin.jacksonMapperBuilder
+import java.lang.reflect.InvocationTargetException
+import tools.jackson.databind.KeyDeserializer as JacksonKeyDeserializer
+
+class WithoutCustomDeserializeMethodTest {
+    companion object {
+        val throwable = IllegalArgumentException("test")
+    }
+
+    @Nested
+    inner class DirectDeserialize {
+        @Test
+        fun primitive() {
+            val result = defaultMapper.readValue<Map<Primitive, String?>>("""{"1":null}""")
+            assertEquals(mapOf(Primitive(1) to null), result)
+        }
+
+        @Test
+        fun nonNullObject() {
+            val result = defaultMapper.readValue<Map<NonNullObject, String?>>("""{"foo":null}""")
+            assertEquals(mapOf(NonNullObject("foo") to null), result)
+        }
+
+        @Test
+        fun nullableObject() {
+            val result = defaultMapper.readValue<Map<NullableObject, String?>>("""{"bar":null}""")
+            assertEquals(mapOf(NullableObject("bar") to null), result)
+        }
+    }
+
+    data class Dst(
+        val p: Map<Primitive, String?>,
+        val nn: Map<NonNullObject, String?>,
+        val n: Map<NullableObject, String?>
+    )
+
+    @Test
+    fun wrapped() {
+        val src = """
+            {
+              "p":{"1":null},
+              "nn":{"foo":null},
+              "n":{"bar":null}
+            }
+        """.trimIndent()
+        val result = defaultMapper.readValue<Dst>(src)
+        val expected = Dst(
+            mapOf(Primitive(1) to null),
+            mapOf(NonNullObject("foo") to null),
+            mapOf(NullableObject("bar") to null)
+        )
+
+        assertEquals(expected, result)
+    }
+
+    @JvmInline
+    value class HasCheckConstructor(val value: Int) {
+        init {
+            if (value < 0) throw throwable
+        }
+    }
+
+    @Test
+    fun callConstructorCheckTest() {
+        val e = assertThrows<InvocationTargetException> {
+            defaultMapper.readValue<Map<HasCheckConstructor, String?>>("""{"-1":null}""")
+        }
+        assertTrue(e.cause === throwable)
+    }
+
+    data class Wrapped(val first: String, val second: String) {
+        class KeyDeserializer : JacksonKeyDeserializer() {
+            override fun deserializeKey(key: String, ctxt: DeserializationContext) =
+                key.split("-").let { Wrapped(it[0], it[1]) }
+        }
+    }
+
+    @JvmInline
+    value class Wrapper(val w: Wrapped)
+
+    @Test
+    fun wrappedCustomObject() {
+        // If a type that cannot be deserialized is specified, the default is an error.
+        val thrown = assertThrows<DatabindException> {
+            defaultMapper.readValue<Map<Wrapper, String?>>("""{"foo-bar":null}""")
+        }
+        assertTrue(thrown.cause is InvalidDefinitionException)
+
+        val mapper = jacksonMapperBuilder()
+            .addModule(
+                object : SimpleModule() {
+                    init { addKeyDeserializer(Wrapped::class.java, Wrapped.KeyDeserializer()) }
+                }
+            )
+            .build()
+
+        val result = mapper.readValue<Map<Wrapper, String?>>("""{"foo-bar":null}""")
+        val expected = mapOf(Wrapper(Wrapped("foo", "bar")) to null)
+
+        assertEquals(expected, result)
+    }
+}