Change the hasRequiredMarker implementation to the 3.x specification

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
 

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]

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<NamedType>? = a.rawType
+    override fun findSubtypes(cfg: MapperConfig<*>, a: Annotated): MutableList<NamedType>? = 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() }
-    }
 }

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))

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

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)