Not Nullable field (but have default value) interpreted as required=true

[leonenkoVN]

jackson-module-kotlin/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt

Line 187 in a49aa3a

private fun KType.isRequired(): Boolean = !isMarkedNullable

Describe the bug:
Field without "?" but have default value interpreted as required=true.

To Reproduce:
We have class in Kotlin:

data class User(
    @Schema(title = "Id", type = "int")
    val id: Int,

    @Schema(title = "Name", type = "string", defaultValue = "default_name")
    val name: String? = "default_name",

    @Schema(title = "Fullname", required = false, type = "string", defaultValue = "default_fullname")
    val fullName: String = "default_fullname"
)

However, the generated swagger documentation is as follows:

User:
   required:
     - fullName		<--- Why?
     - id
   type: object
   properties:
     name:
       title: Name
       type: string
       default: default_name
     fullName:
       title: Fullname
       type: string
       default: default_fullname
     id:
       title: Id
       type: integer
       format: int32

Why field "fullname" (without "?") interpreted as required=true? But it have default value?

Expected behavior
May be Jackson should find default value and interprete field as required = false

Versions:
Kotlin: 1.4.10
Jackson-databind: 2.11.2

[dinomite]

Hmm, I'm a bit confused by the interaction of Swagger and Jackson here—what is providing that @Schema annotation?

[cowtowncoder]

@dinomite must be Swagger, not defined by any fasterxml Jackson module

[dinomite]

Jackson properly handles non-nullable fields that have default values as being not required (deserializing without providing those fields functions properly), so I'm not sure what we need to do here. I think some investigation into how Swagger is interacting with Jackson would be required before we can act.

[JoeryH]

Hi @cowtowncoder and @dinomite ,

I'd like to re-open this issue as it's real. This is not about serialization, that is indeed working as intended. It is about the "requiredMarker", which is calculated in this repo, here: https://github.com/FasterXML/jackson-module-kotlin/blob/master/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt#L29.

The TODO there is somewhat related, but not exactly the same.

As a proof-of-concept I replaced line 106-109 with this locally:

        if (paramGetter != null) {
            val constructor = declaringClass.kotlin.constructors.firstOrNull()
            if (constructor != null) {
                val parameter = constructor.parameters.find {
                    it.name == paramGetter.name
                }
                if (parameter != null) {
                    return !parameter.isOptional
                }
            }
            val byAnnotation = paramGetter.javaGetter?.isRequiredByAnnotation()
            return requiredAnnotationOrNullability(byAnnotation, paramGetter.returnType.isRequired())
        }

That code is bad, not sure if it can be implemented in a nice way, but it does address this exact scenario.

[cowtowncoder]

@JoeryH we typically do not re-open closed issues (history gets complicated, original description may not be accurate as is the case here), but feel free to file a new replacement issue; you can link to this issue if it helps explain the background.
Just make sure to describe the problem: it is bit unclear to me what the problem is.

[JoeryH]

Okay thanks for the quick response! I'll open a new issue and explain more thoroughly.

[JoeryH]

So I looked at this more and I saw this test:

    private data class TestDataClass(
            val g: TestParamClass = TestParamClass(),
    )

        "g".isOptionalForDeserializationOf(testClass, mapper)
        "g".isRequiredForSerializationOf(testClass, mapper)

So there is a distinction between deserialization and serialization.
The library I use for json schema generation (mbknor-jackson-jsonSchema) is using JsonObjectFormatVisitor (source)

But JsonObjectFormatVisitor is defined as such:

public interface JsonObjectFormatVisitor extends JsonFormatVisitorWithSerializerProvider

so it must use the serialization requiredness, instead of the deserialization requiredness, which is what us Kotlin users want, so that it takes into account default values in the constructor of a data class.

I suppose Swagger is doing something similar.

So I guess there is no issue in jackson-module-kotlin. (But probably the design of jackson doesn't account for Kotlin default values?)

Do you have a tip for this situation? Is there a way for us Kotlin users to influence this?

[dinomite]

Hmm, I'm not sure what the implications of your prototyped change would be. A good start would be to make a branch & PR with that change. We can talk more in that PR.

[cowtowncoder]

@JoeryH yes, correct, introspection traversal is based on serialization side of things, and serialization/deserialization views can and often do differ in some aspects. Some things are combined (so annotations from getters/setters are taken into account; similarly constructor annotations can affect serialization), but value defaulting may well be something that is not yet accessible -- especially since Java does not have this concept, but Kotlin does.

At the same time, existing visitor-based traversal design is rather outdated (it predates Jackson 2.0) and it would be good to somehow create a better version. That is not a small undertaking, and would result in changes to many core abstractions.
There is also another possible way to introspect POJO/POKO structures (via BeanDescription) that can be useful for schema generation that Swagger does, instead of visitor api.

[JoeryH]

I want to share my (hacky) workaround, for people in the same boat as me, which is that mbknor-jackson-jsonSchema does not pick up on Kotlin default values.

private object KotlinRequiredAnnotationIntrospector : JacksonAnnotationIntrospector() {

    private val logger = KotlinLogging.logger {}

    override fun hasRequiredMarker(m: AnnotatedMember): Boolean {
        val paramGetter = m.member.declaringClass.kotlin.declaredMemberProperties.find {
            it.getter.javaMethod == m.member
        }
        if (paramGetter != null) {
            val constructor = m.declaringClass.kotlin.constructors.maxByOrNull { it.parameters.size }
            if (constructor != null) {
                val parameter = constructor.parameters.find {
                    it.name == paramGetter.name
                }
                if (parameter != null) {
                    val hasDefault = parameter.isOptional
                    val markedNullable = parameter.type.isMarkedNullable
                    val canBeLeftOut = hasDefault || markedNullable
                    return !canBeLeftOut
                }
            }
        }
        logger.info { "Could not find constructor argument for ${m.declaringClass.simpleName}.${m.name}. Defaulting to not required." }
        return false
    }
}

internal fun jsonSchemaObjectMapper() = jacksonObjectMapper().apply {
    registerModule(
        object : Module() {
            override fun version(): Version = Version.unknownVersion()

            override fun getModuleName(): String = "RequiredAnnotationModule"

            override fun setupModule(context: SetupContext) {
                context.insertAnnotationIntrospector(KotlinRequiredAnnotationIntrospector)
            }
        }
    )
}

fun main() {
    println(JsonSchemaGenerator(jsonSchemaObjectMapper()).generateJsonSchema(SomeTest::class.java))
}

This just looks at the data class constructor and makes the right decision based on that. Note that this doesn't help for primitive types (Int/Boolean) since that is hardcoded in mbknor-jackson-jsonSchema to be always required.

[krisgerhard]

@JoeryH I tried using your workaround with mbknor-jackson-jsonSchema but it only fixed it for lists with default values. Int, Boolean return value fields with default value are still marked as required. Did you make any additional changes?

[JoeryH]

@JoeryH I tried using your workaround with mbknor-jackson-jsonSchema but it only fixed it for lists with default values. Int, Boolean return value fields with default value are still marked as required. Did you make any additional changes?

No, I was not able to make it work for primitives. (see my previous message) So for those I just add a @JsonSchemeDefault always

[krisgerhard]

Selective reading masterclass 😁 Sorry.