From 6825843f59bd8dbb297dbe49a880cb8193ea2b1e Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Sun, 13 Apr 2025 19:34:58 +0900 Subject: [PATCH 1/4] Removed consideration of JsonProperty --- .../kotlin/KotlinAnnotationIntrospector.kt | 39 +++++-------------- 1 file changed, 10 insertions(+), 29 deletions(-) diff --git a/src/main/kotlin/tools/jackson/module/kotlin/KotlinAnnotationIntrospector.kt b/src/main/kotlin/tools/jackson/module/kotlin/KotlinAnnotationIntrospector.kt index 623b1920a..68aa99f5c 100644 --- a/src/main/kotlin/tools/jackson/module/kotlin/KotlinAnnotationIntrospector.kt +++ b/src/main/kotlin/tools/jackson/module/kotlin/KotlinAnnotationIntrospector.kt @@ -1,7 +1,5 @@ package tools.jackson.module.kotlin -import com.fasterxml.jackson.annotation.JsonProperty -import com.fasterxml.jackson.annotation.OptBoolean import tools.jackson.databind.DeserializationFeature import tools.jackson.databind.JacksonModule import tools.jackson.databind.cfg.MapperConfig @@ -14,7 +12,6 @@ import tools.jackson.databind.introspect.AnnotatedParameter import tools.jackson.databind.introspect.NopAnnotationIntrospector import tools.jackson.databind.jsontype.NamedType import tools.jackson.databind.util.Converter -import java.lang.reflect.AccessibleObject import java.lang.reflect.Field import java.lang.reflect.Method import kotlin.reflect.KFunction @@ -43,19 +40,8 @@ internal class KotlinAnnotationIntrospector( // TODO: implement nullIsSameAsDefault flag, which represents when TRUE that if something has a default value, it can be passed a null to default it // this likely impacts this class to be accurate about what COULD be considered required - // If a new isRequired is explicitly specified or the old required is true, those values take precedence. - // In other cases, override is done by KotlinModule. - private fun JsonProperty.forceRequiredByAnnotation(): Boolean? = when { - isRequired != OptBoolean.DEFAULT -> isRequired.asBoolean() - required -> true - else -> null - } - - private fun AccessibleObject.forceRequiredByAnnotation(): Boolean? = - getAnnotation(JsonProperty::class.java)?.forceRequiredByAnnotation() - override fun hasRequiredMarker( - cfg : MapperConfig<*>, + cfg: MapperConfig<*>, m: AnnotatedMember ): Boolean? = m.takeIf { it.member.declaringClass.isKotlinClass() }?.let { _ -> cache.javaMemberIsRequired(m) { @@ -105,7 +91,7 @@ internal class KotlinAnnotationIntrospector( * Subclasses can be detected automatically for sealed classes, since all possible subclasses are known * at compile-time to Kotlin. This makes [com.fasterxml.jackson.annotation.JsonSubTypes] redundant. */ - override fun findSubtypes(cfg : MapperConfig<*>, a: Annotated): MutableList? = a.rawType + override fun findSubtypes(cfg: MapperConfig<*>, a: Annotated): MutableList? = a.rawType .takeIf { it.isKotlinClass() } ?.let { rawType -> rawType.kotlin.sealedSubclasses @@ -116,8 +102,7 @@ internal class KotlinAnnotationIntrospector( private fun AnnotatedField.hasRequiredMarker(): Boolean? { val field = member as Field - return field.forceRequiredByAnnotation() - ?: field.kotlinProperty?.returnType?.isRequired() + return field.kotlinProperty?.returnType?.isRequired() } // Since Kotlin's property has the same Type for each field, getter, and setter, @@ -132,7 +117,7 @@ internal class KotlinAnnotationIntrospector( private fun AnnotatedMethod.getRequiredMarkerFromCorrespondingAccessor(): Boolean? { member.declaringClass.kotlin.declaredMemberProperties.forEach { kProperty -> if (kProperty.javaGetter == this.member || (kProperty as? KMutableProperty1)?.javaSetter == this.member) { - return member.forceRequiredByAnnotation() ?: kProperty.isRequiredByNullability() + return kProperty.isRequiredByNullability() } } return null @@ -140,7 +125,7 @@ internal class KotlinAnnotationIntrospector( // Is the member method a regular method of the data class or private fun Method.getRequiredMarkerFromAccessorLikeMethod(): Boolean? = cache.kotlinFromJava(this)?.let { func -> - forceRequiredByAnnotation() ?: when { + when { func.isGetterLike() -> func.returnType.isRequired() // If nullToEmpty could be supported for setters, // a branch similar to AnnotatedParameter.hasRequiredMarker should be added. @@ -152,15 +137,11 @@ internal class KotlinAnnotationIntrospector( private fun KFunction<*>.isGetterLike(): Boolean = parameters.size == 1 private fun KFunction<*>.isSetterLike(): Boolean = parameters.size == 2 && returnType == UNIT_TYPE - private fun AnnotatedParameter.hasRequiredMarker(): Boolean? = getAnnotation(JsonProperty::class.java) - ?.forceRequiredByAnnotation() - ?: run { - when { - nullToEmptyCollection && type.isCollectionLikeType -> false - nullToEmptyMap && type.isMapLikeType -> false - else -> cache.findKotlinParameter(this)?.isRequired() - } - } + private fun AnnotatedParameter.hasRequiredMarker(): Boolean? = when { + nullToEmptyCollection && type.isCollectionLikeType -> false + nullToEmptyMap && type.isMapLikeType -> false + else -> cache.findKotlinParameter(this)?.isRequired() + } private fun AnnotatedMethod.findValueClassReturnType() = cache.findValueClassReturnType(this) From 517f534d8383aeea664ca174e409ccce1995bddb Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Sat, 19 Apr 2025 14:18:09 +0900 Subject: [PATCH 2/4] Changed the priority of the requirement identification process to be lower than JacksonAnnotationIntrospector closes https://github.com/FasterXML/jackson-module-kotlin/issues/941 --- .../kotlin/KotlinAnnotationIntrospector.kt | 104 ------------------ .../jackson/module/kotlin/KotlinModule.kt | 19 ++-- .../KotlinNamesAnnotationIntrospector.kt | 99 +++++++++++++++++ 3 files changed, 109 insertions(+), 113 deletions(-) diff --git a/src/main/kotlin/tools/jackson/module/kotlin/KotlinAnnotationIntrospector.kt b/src/main/kotlin/tools/jackson/module/kotlin/KotlinAnnotationIntrospector.kt index 68aa99f5c..bf16cf739 100644 --- a/src/main/kotlin/tools/jackson/module/kotlin/KotlinAnnotationIntrospector.kt +++ b/src/main/kotlin/tools/jackson/module/kotlin/KotlinAnnotationIntrospector.kt @@ -1,63 +1,19 @@ package tools.jackson.module.kotlin -import tools.jackson.databind.DeserializationFeature -import tools.jackson.databind.JacksonModule import tools.jackson.databind.cfg.MapperConfig import tools.jackson.databind.introspect.Annotated import tools.jackson.databind.introspect.AnnotatedClass -import tools.jackson.databind.introspect.AnnotatedField -import tools.jackson.databind.introspect.AnnotatedMember import tools.jackson.databind.introspect.AnnotatedMethod -import tools.jackson.databind.introspect.AnnotatedParameter import tools.jackson.databind.introspect.NopAnnotationIntrospector import tools.jackson.databind.jsontype.NamedType import tools.jackson.databind.util.Converter -import java.lang.reflect.Field -import java.lang.reflect.Method -import kotlin.reflect.KFunction -import kotlin.reflect.KMutableProperty1 -import kotlin.reflect.KParameter -import kotlin.reflect.KProperty1 -import kotlin.reflect.KType -import kotlin.reflect.full.createType -import kotlin.reflect.full.declaredMemberProperties -import kotlin.reflect.full.valueParameters -import kotlin.reflect.jvm.javaGetter -import kotlin.reflect.jvm.javaSetter -import kotlin.reflect.jvm.javaType -import kotlin.reflect.jvm.kotlinProperty import kotlin.time.Duration internal class KotlinAnnotationIntrospector( - private val context: JacksonModule.SetupContext, private val cache: ReflectionCache, - private val nullToEmptyCollection: Boolean, - private val nullToEmptyMap: Boolean, - private val nullIsSameAsDefault: Boolean, private val useJavaDurationConversion: Boolean, ) : NopAnnotationIntrospector() { - // TODO: implement nullIsSameAsDefault flag, which represents when TRUE that if something has a default value, it can be passed a null to default it - // this likely impacts this class to be accurate about what COULD be considered required - - override fun hasRequiredMarker( - cfg: MapperConfig<*>, - m: AnnotatedMember - ): Boolean? = m.takeIf { it.member.declaringClass.isKotlinClass() }?.let { _ -> - cache.javaMemberIsRequired(m) { - try { - when (m) { - is AnnotatedField -> m.hasRequiredMarker() - is AnnotatedMethod -> m.hasRequiredMarker() - is AnnotatedParameter -> m.hasRequiredMarker() - else -> null - } - } catch (_: UnsupportedOperationException) { - null - } - } - } - override fun findSerializationConverter(config: MapperConfig<*>?, a: Annotated): Converter<*, *>? = when (a) { // Find a converter to handle the case where the getter returns an unboxed value from the value class. is AnnotatedMethod -> a.findValueClassReturnType()?.let { @@ -100,65 +56,5 @@ internal class KotlinAnnotationIntrospector( .ifEmpty { null } } - private fun AnnotatedField.hasRequiredMarker(): Boolean? { - val field = member as Field - return field.kotlinProperty?.returnType?.isRequired() - } - - // Since Kotlin's property has the same Type for each field, getter, and setter, - // nullability can be determined from the returnType of KProperty. - private fun KProperty1<*, *>.isRequiredByNullability() = returnType.isRequired() - - // This could be a setter or a getter of a class property or - // a setter-like/getter-like method. - private fun AnnotatedMethod.hasRequiredMarker(): Boolean? = this.getRequiredMarkerFromCorrespondingAccessor() - ?: this.member.getRequiredMarkerFromAccessorLikeMethod() - - private fun AnnotatedMethod.getRequiredMarkerFromCorrespondingAccessor(): Boolean? { - member.declaringClass.kotlin.declaredMemberProperties.forEach { kProperty -> - if (kProperty.javaGetter == this.member || (kProperty as? KMutableProperty1)?.javaSetter == this.member) { - return kProperty.isRequiredByNullability() - } - } - return null - } - - // Is the member method a regular method of the data class or - private fun Method.getRequiredMarkerFromAccessorLikeMethod(): Boolean? = cache.kotlinFromJava(this)?.let { func -> - when { - func.isGetterLike() -> func.returnType.isRequired() - // If nullToEmpty could be supported for setters, - // a branch similar to AnnotatedParameter.hasRequiredMarker should be added. - func.isSetterLike() -> func.valueParameters[0].isRequired() - else -> null - } - } - - private fun KFunction<*>.isGetterLike(): Boolean = parameters.size == 1 - private fun KFunction<*>.isSetterLike(): Boolean = parameters.size == 2 && returnType == UNIT_TYPE - - private fun AnnotatedParameter.hasRequiredMarker(): Boolean? = when { - nullToEmptyCollection && type.isCollectionLikeType -> false - nullToEmptyMap && type.isMapLikeType -> false - else -> cache.findKotlinParameter(this)?.isRequired() - } - private fun AnnotatedMethod.findValueClassReturnType() = cache.findValueClassReturnType(this) - - private fun KParameter.isRequired(): Boolean { - val paramType = type - val isPrimitive = when (val javaType = paramType.javaType) { - is Class<*> -> javaType.isPrimitive - else -> false - } - - return !paramType.isMarkedNullable && !isOptional && !isVararg && - !(isPrimitive && !context.isEnabled(DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES)) - } - - private fun KType.isRequired(): Boolean = !isMarkedNullable - - companion object { - val UNIT_TYPE: KType by lazy { Unit::class.createType() } - } } diff --git a/src/main/kotlin/tools/jackson/module/kotlin/KotlinModule.kt b/src/main/kotlin/tools/jackson/module/kotlin/KotlinModule.kt index 14cc2bd12..7444d0208 100644 --- a/src/main/kotlin/tools/jackson/module/kotlin/KotlinModule.kt +++ b/src/main/kotlin/tools/jackson/module/kotlin/KotlinModule.kt @@ -92,16 +92,17 @@ class KotlinModule private constructor( context.addDeserializerModifier(KotlinValueDeserializerModifier) } - context.insertAnnotationIntrospector(KotlinAnnotationIntrospector( - context, - cache, - nullToEmptyCollection, - nullToEmptyMap, - nullIsSameAsDefault, - useJavaDurationConversion - )) + context.insertAnnotationIntrospector(KotlinAnnotationIntrospector(cache, useJavaDurationConversion)) context.appendAnnotationIntrospector( - KotlinNamesAnnotationIntrospector(cache, newStrictNullChecks, kotlinPropertyNameAsImplicitName) + KotlinNamesAnnotationIntrospector( + context = context, + cache = cache, + nullToEmptyCollection = nullToEmptyCollection, + nullToEmptyMap = nullToEmptyMap, + nullIsSameAsDefault = nullIsSameAsDefault, + strictNullChecks = newStrictNullChecks, + kotlinPropertyNameAsImplicitName = kotlinPropertyNameAsImplicitName + ) ) context.addDeserializers(KotlinDeserializers(cache, useJavaDurationConversion)) diff --git a/src/main/kotlin/tools/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt b/src/main/kotlin/tools/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt index 767f22611..9d49c30cc 100644 --- a/src/main/kotlin/tools/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt +++ b/src/main/kotlin/tools/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt @@ -3,32 +3,127 @@ package tools.jackson.module.kotlin import com.fasterxml.jackson.annotation.JsonProperty import com.fasterxml.jackson.annotation.JsonSetter import com.fasterxml.jackson.annotation.Nulls +import tools.jackson.databind.DeserializationFeature +import tools.jackson.databind.JacksonModule import tools.jackson.databind.JavaType import tools.jackson.databind.cfg.MapperConfig import tools.jackson.databind.introspect.Annotated import tools.jackson.databind.introspect.AnnotatedClass +import tools.jackson.databind.introspect.AnnotatedField import tools.jackson.databind.introspect.AnnotatedMember import tools.jackson.databind.introspect.AnnotatedMethod import tools.jackson.databind.introspect.AnnotatedParameter import tools.jackson.databind.introspect.NopAnnotationIntrospector import tools.jackson.databind.introspect.PotentialCreator import java.lang.reflect.Constructor +import java.lang.reflect.Field +import java.lang.reflect.Method import java.util.Locale import kotlin.collections.getOrNull import kotlin.reflect.KClass import kotlin.reflect.KFunction +import kotlin.reflect.KMutableProperty1 import kotlin.reflect.KParameter +import kotlin.reflect.KProperty1 +import kotlin.reflect.KType +import kotlin.reflect.full.createType +import kotlin.reflect.full.declaredMemberProperties import kotlin.reflect.full.hasAnnotation import kotlin.reflect.full.memberProperties import kotlin.reflect.full.primaryConstructor +import kotlin.reflect.full.valueParameters import kotlin.reflect.jvm.javaGetter +import kotlin.reflect.jvm.javaSetter import kotlin.reflect.jvm.javaType +import kotlin.reflect.jvm.kotlinProperty internal class KotlinNamesAnnotationIntrospector( + private val context: JacksonModule.SetupContext, private val cache: ReflectionCache, + private val nullToEmptyCollection: Boolean, + private val nullToEmptyMap: Boolean, + private val nullIsSameAsDefault: Boolean, private val strictNullChecks: Boolean, private val kotlinPropertyNameAsImplicitName: Boolean ) : NopAnnotationIntrospector() { + private fun KType.isRequired(): Boolean = !isMarkedNullable + + // Since Kotlin's property has the same Type for each field, getter, and setter, + // nullability can be determined from the returnType of KProperty. + private fun KProperty1<*, *>.isRequiredByNullability() = returnType.isRequired() + + private fun KParameter.isRequired(): Boolean { + val paramType = type + val isPrimitive = when (val javaType = paramType.javaType) { + is Class<*> -> javaType.isPrimitive + else -> false + } + + return !paramType.isMarkedNullable && !isOptional && !isVararg && + !(isPrimitive && !context.isEnabled(DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES)) + } + + private fun AnnotatedField.hasRequiredMarker(): Boolean? { + val field = member as Field + return field.kotlinProperty?.returnType?.isRequired() + } + + private fun KFunction<*>.isGetterLike(): Boolean = parameters.size == 1 + private fun KFunction<*>.isSetterLike(): Boolean = parameters.size == 2 && returnType == UNIT_TYPE + + private fun AnnotatedMethod.getRequiredMarkerFromCorrespondingAccessor(): Boolean? { + member.declaringClass.kotlin.declaredMemberProperties.forEach { kProperty -> + if (kProperty.javaGetter == this.member || (kProperty as? KMutableProperty1)?.javaSetter == this.member) { + return kProperty.isRequiredByNullability() + } + } + return null + } + + // Is the member method a regular method of the data class or + private fun Method.getRequiredMarkerFromAccessorLikeMethod(): Boolean? = cache.kotlinFromJava(this)?.let { func -> + when { + func.isGetterLike() -> func.returnType.isRequired() + // If nullToEmpty could be supported for setters, + // a branch similar to AnnotatedParameter.hasRequiredMarker should be added. + func.isSetterLike() -> func.valueParameters[0].isRequired() + else -> null + } + } + + // This could be a setter or a getter of a class property or + // a setter-like/getter-like method. + private fun AnnotatedMethod.hasRequiredMarker(): Boolean? = this.getRequiredMarkerFromCorrespondingAccessor() + ?: this.member.getRequiredMarkerFromAccessorLikeMethod() + + // TODO: implement nullIsSameAsDefault flag, which represents when TRUE that if something has a default value, it can be passed a null to default it + // this likely impacts this class to be accurate about what COULD be considered required + private fun AnnotatedParameter.hasRequiredMarker(): Boolean? = when { + nullToEmptyCollection && type.isCollectionLikeType -> false + nullToEmptyMap && type.isMapLikeType -> false + else -> cache.findKotlinParameter(this)?.isRequired() + } + + override fun hasRequiredMarker( + cfg: MapperConfig<*>, + m: AnnotatedMember + ): Boolean? = m.takeIf { it.member.declaringClass.isKotlinClass() }?.let { _ -> + println(m) + + cache.javaMemberIsRequired(m) { + try { + when (m) { + is AnnotatedField -> m.hasRequiredMarker() + is AnnotatedMethod -> m.hasRequiredMarker() + is AnnotatedParameter -> m.hasRequiredMarker() + else -> null + } + } catch (_: UnsupportedOperationException) { + null + } + } + } + private fun getterNameFromJava(member: AnnotatedMethod): String? { val name = member.name @@ -123,6 +218,10 @@ internal class KotlinNamesAnnotationIntrospector( private fun findKotlinParameter(param: Annotated) = (param as? AnnotatedParameter) ?.let { cache.findKotlinParameter(it) } + + companion object { + val UNIT_TYPE: KType by lazy { Unit::class.createType() } + } } private fun KParameter.markedNonNullAt(index: Int) = type.arguments.getOrNull(index)?.type?.isMarkedNullable == false From e888217e7e013cb7fb4e65b798aca60c7a0fb21f Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Sat, 19 Apr 2025 14:23:45 +0900 Subject: [PATCH 3/4] Fix tests --- .../jackson/module/kotlin/test/PropertyRequirednessTests.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/kotlin/tools/jackson/module/kotlin/test/PropertyRequirednessTests.kt b/src/test/kotlin/tools/jackson/module/kotlin/test/PropertyRequirednessTests.kt index a9b7d4e66..37d2acab8 100644 --- a/src/test/kotlin/tools/jackson/module/kotlin/test/PropertyRequirednessTests.kt +++ b/src/test/kotlin/tools/jackson/module/kotlin/test/PropertyRequirednessTests.kt @@ -125,7 +125,7 @@ class TestPropertyRequiredness { "i".isOptionalForDeserializationOf(testClass, mapper) "x".isRequiredForDeserializationOf(testClass, mapper) - "x".isOptionalForSerializationOf(testClass, mapper) + "x".isRequiredForSerializationOf(testClass, mapper) "z".isRequiredForDeserializationOf(testClass, mapper) "z".isRequiredForSerializationOf(testClass, mapper) @@ -161,7 +161,7 @@ class TestPropertyRequiredness { "h".isOptionalForDeserializationOf(testClass, mapper) "x".isRequiredForDeserializationOf(testClass, mapper) - "x".isOptionalForSerializationOf(testClass, mapper) + "x".isRequiredForSerializationOf(testClass, mapper) "z".isRequiredForDeserializationOf(testClass, mapper) "z".isRequiredForSerializationOf(testClass, mapper) From c1a7528ba7275b0f7ec612f62d008807e15c7e71 Mon Sep 17 00:00:00 2001 From: wrongwrong Date: Sat, 19 Apr 2025 14:40:14 +0900 Subject: [PATCH 4/4] Update release notes wrt #952 --- release-notes/CREDITS | 4 ++++ release-notes/VERSION | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/release-notes/CREDITS b/release-notes/CREDITS index 182e1a3f7..36c9e4ed7 100644 --- a/release-notes/CREDITS +++ b/release-notes/CREDITS @@ -19,7 +19,11 @@ Contributors: # 3.0.0-rc4 (not yet released) +WrongWrong (@k163377) +* #952: Change the `hasRequiredMarker` implementation to the 3.x specification + # 3.0.0-rc3 (13-Apr-2025) + WrongWrong (@k163377) * #945: Replace JacksonXmlRootElement used in the test with JsonRootName diff --git a/release-notes/VERSION b/release-notes/VERSION index c8601b7d3..5f34a3377 100644 --- a/release-notes/VERSION +++ b/release-notes/VERSION @@ -24,6 +24,11 @@ Former maintainers: 3.0.0-rc4 (not yet released) +#952: The `isRequired` result from `kotlin-module` no longer overrides the result from + `JacksonAnnotationIntrospector` or other `AnnotationIntrospector`s. + Tests have confirmed that `@JsonProperty(required = true)` for nullable parameters has been changed to be + determined as `required`. + 3.0.0-rc3 (13-Apr-2025) #887: Change 3.0 to use `module-info.java` directly [JSTEP-11]