From 093842dda965aa2ee1cb47aace0ec98528716572 Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Sat, 15 Jul 2023 19:24:20 +0900 Subject: [PATCH 1/5] Add UseKotlinPropertyNameForGetter option --- .../jackson/module/kotlin/KotlinFeature.kt | 18 +++++++++++++++++- .../jackson/module/kotlin/KotlinModule.kt | 15 ++++++++++++--- .../KotlinNamesAnnotationIntrospector.kt | 7 ++++++- 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinFeature.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinFeature.kt index 27af0f23..c2afef7d 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinFeature.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinFeature.kt @@ -42,7 +42,23 @@ enum class KotlinFeature(private val enabledByDefault: Boolean) { * may contain null values after deserialization. * Enabling it protects against this but has significant performance impact. */ - StrictNullChecks(enabledByDefault = false); + StrictNullChecks(enabledByDefault = false), + + /** + * By enabling this feature, the property name on Kotlin will be used as the getter name. + * + * By default, the name based on the getter name on the JVM is used as the accessor name. + * This name may be different from the parameter/field name, in which case serialization results + * may be incorrect or annotations may malfunction. + * See [jackson-module-kotlin#630] for details. + * + * By enabling this feature, such malfunctions will not occur. + * + * On the other hand, enabling this option increases the amount of reflection processing, + * which may result in performance degradation for both serialization and deserialization. + * In addition, the adjustment of behavior using get:JvmName is disabled. + */ + UseKotlinPropertyNameForGetter(enabledByDefault = false); internal val bitSet: BitSet = (1 shl ordinal).toBitSet() diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinModule.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinModule.kt index 25d8d3f8..80c8e743 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinModule.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinModule.kt @@ -7,6 +7,7 @@ import com.fasterxml.jackson.module.kotlin.KotlinFeature.NullIsSameAsDefault import com.fasterxml.jackson.module.kotlin.KotlinFeature.NullToEmptyCollection import com.fasterxml.jackson.module.kotlin.KotlinFeature.NullToEmptyMap import com.fasterxml.jackson.module.kotlin.KotlinFeature.StrictNullChecks +import com.fasterxml.jackson.module.kotlin.KotlinFeature.UseKotlinPropertyNameForGetter import com.fasterxml.jackson.module.kotlin.SingletonSupport.CANONICALIZE import com.fasterxml.jackson.module.kotlin.SingletonSupport.DISABLED import java.util.* @@ -53,7 +54,8 @@ class KotlinModule @Deprecated( val nullToEmptyMap: Boolean = false, val nullIsSameAsDefault: Boolean = false, val singletonSupport: SingletonSupport = DISABLED, - val strictNullChecks: Boolean = false + val strictNullChecks: Boolean = false, + val useKotlinPropertyNameForGetter: Boolean = false ) : SimpleModule(KotlinModule::class.java.name, PackageVersion.VERSION) { init { if (!KotlinVersion.CURRENT.isAtLeast(1, 5)) { @@ -102,7 +104,8 @@ class KotlinModule @Deprecated( builder.isEnabled(KotlinFeature.SingletonSupport) -> CANONICALIZE else -> DISABLED }, - builder.isEnabled(StrictNullChecks) + builder.isEnabled(StrictNullChecks), + builder.isEnabled(UseKotlinPropertyNameForGetter) ) companion object { @@ -130,7 +133,13 @@ class KotlinModule @Deprecated( } context.insertAnnotationIntrospector(KotlinAnnotationIntrospector(context, cache, nullToEmptyCollection, nullToEmptyMap, nullIsSameAsDefault)) - context.appendAnnotationIntrospector(KotlinNamesAnnotationIntrospector(this, cache, ignoredClassesForImplyingJsonCreator)) + context.appendAnnotationIntrospector( + KotlinNamesAnnotationIntrospector( + this, + cache, + ignoredClassesForImplyingJsonCreator, + useKotlinPropertyNameForGetter) + ) context.addDeserializers(KotlinDeserializers()) context.addKeyDeserializers(KotlinKeyDeserializers) diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt index 42198924..4746549d 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt @@ -27,7 +27,12 @@ import kotlin.reflect.jvm.internal.KotlinReflectionInternalError import kotlin.reflect.jvm.javaType import kotlin.reflect.jvm.kotlinFunction -internal class KotlinNamesAnnotationIntrospector(val module: KotlinModule, val cache: ReflectionCache, val ignoredClassesForImplyingJsonCreator: Set>) : NopAnnotationIntrospector() { +internal class KotlinNamesAnnotationIntrospector( + val module: KotlinModule, + val cache: ReflectionCache, + val ignoredClassesForImplyingJsonCreator: Set>, + val useKotlinPropertyNameForGetter: Boolean +) : NopAnnotationIntrospector() { // since 2.4 override fun findImplicitPropertyName(member: AnnotatedMember): String? { if (!member.declaringClass.isKotlinClass()) return null From 3dd54074b33509d24beffe1ce1e8e23dc25963df Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Sat, 15 Jul 2023 18:15:34 +0900 Subject: [PATCH 2/5] Separate conventional processing into function --- .../KotlinNamesAnnotationIntrospector.kt | 42 ++++++++++--------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt index 4746549d..ffd4995d 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt @@ -33,31 +33,35 @@ internal class KotlinNamesAnnotationIntrospector( val ignoredClassesForImplyingJsonCreator: Set>, val useKotlinPropertyNameForGetter: Boolean ) : NopAnnotationIntrospector() { + private fun getterNameFromJava(member: AnnotatedMember): String? { + val name = member.name + + // The reason for truncating after `-` is to truncate the random suffix + // given after the value class accessor name. + return when { + name.startsWith("get") -> name.takeIf { it.contains("-") }?.let { _ -> + name.substringAfter("get") + .replaceFirstChar { it.lowercase(Locale.getDefault()) } + .substringBefore('-') + } + // since 2.15: support Kotlin's way of handling "isXxx" backed properties where + // logical property name needs to remain "isXxx" and not become "xxx" as with Java Beans + // (see https://kotlinlang.org/docs/reference/java-to-kotlin-interop.html and + // https://github.com/FasterXML/jackson-databind/issues/2527 and + // https://github.com/FasterXML/jackson-module-kotlin/issues/340 + // for details) + name.startsWith("is") -> if (name.contains("-")) name.substringAfter("-") else name + else -> null + } + } + // since 2.4 override fun findImplicitPropertyName(member: AnnotatedMember): String? { if (!member.declaringClass.isKotlinClass()) return null - val name = member.name - return when (member) { is AnnotatedMethod -> if (member.parameterCount == 0) { - // The reason for truncating after `-` is to truncate the random suffix - // given after the value class accessor name. - when { - name.startsWith("get") -> name.takeIf { it.contains("-") }?.let { _ -> - name.substringAfter("get") - .replaceFirstChar { it.lowercase(Locale.getDefault()) } - .substringBefore('-') - } - // since 2.15: support Kotlin's way of handling "isXxx" backed properties where - // logical property name needs to remain "isXxx" and not become "xxx" as with Java Beans - // (see https://kotlinlang.org/docs/reference/java-to-kotlin-interop.html and - // https://github.com/FasterXML/jackson-databind/issues/2527 and - // https://github.com/FasterXML/jackson-module-kotlin/issues/340 - // for details) - name.startsWith("is") -> if (name.contains("-")) name.substringAfter("-") else name - else -> null - } + if (useKotlinPropertyNameForGetter) TODO() else getterNameFromJava(member) } else null is AnnotatedParameter -> findKotlinParameterName(member) else -> null From cbbcfbea9a73e145be355e5c4d49f16b67431d9a Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Sat, 15 Jul 2023 18:47:55 +0900 Subject: [PATCH 3/5] Implement getting from Kotlin property name --- .../kotlin/KotlinNamesAnnotationIntrospector.kt | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt index ffd4995d..858ceb7e 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt @@ -24,6 +24,7 @@ import kotlin.reflect.full.hasAnnotation import kotlin.reflect.full.memberProperties import kotlin.reflect.full.primaryConstructor import kotlin.reflect.jvm.internal.KotlinReflectionInternalError +import kotlin.reflect.jvm.javaGetter import kotlin.reflect.jvm.javaType import kotlin.reflect.jvm.kotlinFunction @@ -33,7 +34,7 @@ internal class KotlinNamesAnnotationIntrospector( val ignoredClassesForImplyingJsonCreator: Set>, val useKotlinPropertyNameForGetter: Boolean ) : NopAnnotationIntrospector() { - private fun getterNameFromJava(member: AnnotatedMember): String? { + private fun getterNameFromJava(member: AnnotatedMethod): String? { val name = member.name // The reason for truncating after `-` is to truncate the random suffix @@ -55,13 +56,25 @@ internal class KotlinNamesAnnotationIntrospector( } } + private fun getterNameFromKotlin(member: AnnotatedMethod): String? { + val getter = member.member + + return member.member.declaringClass.takeIf { it.isKotlinClass() }?.let { clazz -> + clazz.kotlin.memberProperties.find { it.javaGetter == getter } + ?.let { it.name } + } + } + // since 2.4 override fun findImplicitPropertyName(member: AnnotatedMember): String? { if (!member.declaringClass.isKotlinClass()) return null return when (member) { is AnnotatedMethod -> if (member.parameterCount == 0) { - if (useKotlinPropertyNameForGetter) TODO() else getterNameFromJava(member) + if (useKotlinPropertyNameForGetter) { + // Fall back to default if it is a getter-like function + getterNameFromKotlin(member) ?: getterNameFromJava(member) + } else getterNameFromJava(member) } else null is AnnotatedParameter -> findKotlinParameterName(member) else -> null From 5b65f56db75afab3d4589e5d10bf605381d002e2 Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Sat, 15 Jul 2023 19:31:40 +0900 Subject: [PATCH 4/5] Add tests for #630 --- .../module/kotlin/test/github/Github630.kt | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github630.kt diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github630.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github630.kt new file mode 100644 index 00000000..8c6581ae --- /dev/null +++ b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github630.kt @@ -0,0 +1,40 @@ +package com.fasterxml.jackson.module.kotlin.test.github + +import com.fasterxml.jackson.annotation.JsonProperty +import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.module.kotlin.KotlinFeature +import com.fasterxml.jackson.module.kotlin.KotlinModule +import org.junit.Test +import kotlin.test.assertEquals + +class Github630 { + private val mapper = ObjectMapper() + .registerModule(KotlinModule.Builder().enable(KotlinFeature.UseKotlinPropertyNameForGetter).build())!! + + data class Dto( + // from #570, #603 + val FOO: Int = 0, + val bAr: Int = 0, + @JsonProperty("b") + val BAZ: Int = 0, + @JsonProperty("q") + val qUx: Int = 0, + // from #71 + internal val quux: Int = 0, + // from #434 + val `corge-corge`: Int = 0, + // additional + @get:JvmName("aaa") + val grault: Int = 0 + ) + + @Test + fun test() { + val dto = Dto() + + assertEquals( + """{"FOO":0,"bAr":0,"b":0,"q":0,"quux":0,"corge-corge":0,"grault":0}""", + mapper.writeValueAsString(dto) + ) + } +} From aa7217f4ba0ab09896f5806d6ce9458c072c319f Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Sat, 22 Jul 2023 15:45:04 +0900 Subject: [PATCH 5/5] Fix for review --- .../com/fasterxml/jackson/module/kotlin/KotlinFeature.kt | 8 ++++---- .../com/fasterxml/jackson/module/kotlin/KotlinModule.kt | 4 ++-- .../jackson/module/kotlin/test/github/Github630.kt | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinFeature.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinFeature.kt index c2afef7d..b6143fcc 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinFeature.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinFeature.kt @@ -1,7 +1,6 @@ package com.fasterxml.jackson.module.kotlin import java.util.BitSet -import kotlin.math.pow /** * @see KotlinModule.Builder @@ -45,9 +44,9 @@ enum class KotlinFeature(private val enabledByDefault: Boolean) { StrictNullChecks(enabledByDefault = false), /** - * By enabling this feature, the property name on Kotlin will be used as the getter name. + * By enabling this feature, the property name on Kotlin is used as the implicit name for the getter. * - * By default, the name based on the getter name on the JVM is used as the accessor name. + * By default, the getter name is used during serialization. * This name may be different from the parameter/field name, in which case serialization results * may be incorrect or annotations may malfunction. * See [jackson-module-kotlin#630] for details. @@ -57,8 +56,9 @@ enum class KotlinFeature(private val enabledByDefault: Boolean) { * On the other hand, enabling this option increases the amount of reflection processing, * which may result in performance degradation for both serialization and deserialization. * In addition, the adjustment of behavior using get:JvmName is disabled. + * Note also that this feature does not apply to setters. */ - UseKotlinPropertyNameForGetter(enabledByDefault = false); + KotlinPropertyNameAsImplicitName(enabledByDefault = false); internal val bitSet: BitSet = (1 shl ordinal).toBitSet() diff --git a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinModule.kt b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinModule.kt index 80c8e743..5d75a1ad 100644 --- a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinModule.kt +++ b/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinModule.kt @@ -7,7 +7,7 @@ import com.fasterxml.jackson.module.kotlin.KotlinFeature.NullIsSameAsDefault import com.fasterxml.jackson.module.kotlin.KotlinFeature.NullToEmptyCollection import com.fasterxml.jackson.module.kotlin.KotlinFeature.NullToEmptyMap import com.fasterxml.jackson.module.kotlin.KotlinFeature.StrictNullChecks -import com.fasterxml.jackson.module.kotlin.KotlinFeature.UseKotlinPropertyNameForGetter +import com.fasterxml.jackson.module.kotlin.KotlinFeature.KotlinPropertyNameAsImplicitName import com.fasterxml.jackson.module.kotlin.SingletonSupport.CANONICALIZE import com.fasterxml.jackson.module.kotlin.SingletonSupport.DISABLED import java.util.* @@ -105,7 +105,7 @@ class KotlinModule @Deprecated( else -> DISABLED }, builder.isEnabled(StrictNullChecks), - builder.isEnabled(UseKotlinPropertyNameForGetter) + builder.isEnabled(KotlinPropertyNameAsImplicitName) ) companion object { diff --git a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github630.kt b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github630.kt index 8c6581ae..a5fb7b63 100644 --- a/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github630.kt +++ b/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/Github630.kt @@ -9,7 +9,7 @@ import kotlin.test.assertEquals class Github630 { private val mapper = ObjectMapper() - .registerModule(KotlinModule.Builder().enable(KotlinFeature.UseKotlinPropertyNameForGetter).build())!! + .registerModule(KotlinModule.Builder().enable(KotlinFeature.KotlinPropertyNameAsImplicitName).build())!! data class Dto( // from #570, #603