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] diff --git a/src/main/kotlin/tools/jackson/module/kotlin/KotlinAnnotationIntrospector.kt b/src/main/kotlin/tools/jackson/module/kotlin/KotlinAnnotationIntrospector.kt index 623b1920a..bf16cf739 100644 --- a/src/main/kotlin/tools/jackson/module/kotlin/KotlinAnnotationIntrospector.kt +++ b/src/main/kotlin/tools/jackson/module/kotlin/KotlinAnnotationIntrospector.kt @@ -1,77 +1,19 @@ 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 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.AccessibleObject -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 - - // 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<*>, - 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 { @@ -105,7 +47,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 @@ -114,70 +56,5 @@ internal class KotlinAnnotationIntrospector( .ifEmpty { null } } - private fun AnnotatedField.hasRequiredMarker(): Boolean? { - val field = member as Field - return field.forceRequiredByAnnotation() - ?: 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 member.forceRequiredByAnnotation() ?: 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 -> - forceRequiredByAnnotation() ?: 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? = getAnnotation(JsonProperty::class.java) - ?.forceRequiredByAnnotation() - ?: run { - 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 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)