[BUG][Kotlin] JsonSetter(nulls = Nulls.FAIL) generated even with openApiNullable = false

[mfunda-cen59049]

Bug Report Checklist

• Have you provided a full/minimal spec to reproduce the issue?
• Have you validated the input using an OpenAPI validator?
• Have you tested with the latest master to confirm the issue still exists?
• Have you searched for related issues/PRs?
• What's the actual output vs expected output?
• [Optional] Sponsorship to speed up the bug fix or feature request (example)

Description

Even after setting <openApiNullable>false</openApiNullable> in the Kotlin generate-sources config options, the generated model classes contain annotation @field:JsonSetter(nulls = Nulls.FAIL) for non-required fields.

openapi-generator version

7.23.0

OpenAPI declaration file content or url

type: object
properties:
  id:
    type: string

Generation Details

                    <execution>
                        <id>KOTLIN</id>
                        <phase>generate-sources</phase>
                        <goals>
                            <goal>generate</goal>
                        </goals>
                        <configuration>
                            <inputSpec>${openapi.output}</inputSpec>
                            ...
                            <generatorName>kotlin-spring</generatorName>
                            <generateSupportingFiles>false</generateSupportingFiles>
                            <configOptions>
                                <useBeanValidation>false</useBeanValidation>
                                <useJackson3>true</useJackson3>
                                <useSpringBoot4>true</useSpringBoot4>
                                <enumPropertyNaming>original</enumPropertyNaming>
                                <useTags>true</useTags>
                                <openApiNullable>false</openApiNullable>
                            </configOptions>
                        </configuration>
                    </execution>

Steps to reproduce

1. Setup a project with Kotlin OpenAPI generator with the provided configuration.
2. Create a schema with non-required field not annotated as nullable: true
3. Generate the Kotlin classes and observe that the generated class contains @field:JsonSetter(nulls = Nulls.FAIL) annotation.

Related issues/PRs

#23804

Suggest a fix

With the openApiNullable = false setting, the JsonSetter(nulls = Nulls.FAIL) annotation should not be included in the generated classes the same way it works for Java.

[Picazsoo]

Wait,

I believe the generated @JsonSetter(nulls = Nulls.FAIL) is intentional for optional non-nullable fields.

required: false only allows the property to be omitted; it does not allow the property to be present with JSON null. Since nullable defaults to false, {"id": null} should fail while {} should deserialize successfully.

openApiNullable controls use of JsonNullable<T> for optional nullable properties, primarily to distinguish between absent and explicitly null values, for example in PATCH operations. It does not mean that optional non-nullable properties should accept explicit JSON null.

If you want an optional property to accept explicit JSON null, it should be marked as nullable in the spec:

type: object
properties:
  id:
    type: string
    nullable: true

I am not convinced this behavior is incorrect. Jackson should (and IMO does) deserialize successfully for a property annotated with @JsonSetter(nulls = Nulls.FAIL) when the property is omitted entirely.

I will not pretend this did not break a couple of tests in our projects as well. But that is because the previous generator output was more lenient than the OpenAPI contract.

Feel free to push back if you feel my understanding is incorrect.

[Picazsoo]

There may be a separate compatibility / option-semantics question here: if openApiNullable=false is expected to disable all behavior introduced by the openApiNullable feature, then this may be surprising. But semantically, rejecting explicit null for optional non-nullable fields seems correct.

[mfunda-cen59049]

From what I understood from the PR I've linked, this feature should be opt-in through the config option.

I totally agree with you that the new approach is semantically more appropriate. However, the issue is that this minor version upgrade in the OpenAPI generator effectively introduced a breaking change to our application APIs without any notice.

Moreover, when generating Java classes, the openApiNullable=false option prevents the @JsonSetter(nulls = Nulls.FAIL) annotations from being generated. This is not the case for Kotlin with the same configuration.

[Picazsoo]

I have thought about it, written something about how the behavior is correct as-is. Then I have thought about it some more and I kind of see your point. I guess gating the @JsonSetter(nulls = Nulls.FAIL) behind openApiNullable=true makes sense, since by enabling of that feature, you signal as the deserializing side of the api, that you deeply care about being able to distinguish between the three possible different states (and probably also explicitly forbid deserializing values when some of the states are not allowed by the spec).

I would still recommend as a proper fix to go and adjust the problematic schema attributes in your APIs to be nullable: true, but I will go and prepare a "fix" for this in the coming days.

Sorry for any issues caused - it was implemented with the best intentions. Luckily you can probably temporarily work around this issue by using a customized version of the affected mustache templates. That should carry you over until 7.24.0

[Picazsoo]

Besides using custom templates, you can probably also cook up some small gradle task to clean up the generated files:

But I would go with the modified templates for sure if possible.
https://github.com/Picazsoo/openapi-generator/blob/1cbd6196e146cc64d1d5ee83f839e4c19c15fdcf/modules/openapi-generator/src/main/resources/kotlin-spring/dataClassOptVar.mustache
Just delete this section:

I really like the way this project is structured. That often a "bug" such as this can be worked around temporarily without having to forgo any additional features you might want to use in a freshly released version. It is really very flexible.

And just for fun - originally I really wanted to post this classic XKCD as well. That was before you changed my mind, but I still cannot not post it.

[Picazsoo]

And now that I think about it even more, one can consider the entire @JsonSetter(nulls = Nulls.FAIL) as useless strict behavior that brings no practical value other than being correct and setting expectations for the other side.

Postel robustness principle would suggest to instead never use @JsonSetter(nulls = Nulls.FAIL) and just act as if the field was simply omitted.

So I am really not sure how to handle this now. Maybe it makes no sense even for openApiNullable=true.

Any ideas @wing328 ? What is the general approach of open api generator? To generate DTOs closely matching the spec or to be able to deserialize even problematic payloads (optional non-nullable attribute being sent as null in json)

[Picazsoo]

After thinking about this a bit more, I do see concrete cases where omitting @JsonSetter(nulls = Nulls.FAIL) for non-required, non-nullable properties can cause problems.

In particular, this affects non-required properties that define a default value.

If explicit JSON null is accepted for such a property, the contract becomes weaker than the OpenAPI schema suggests: a client can send null and effectively override the schema-defined default.

For a property that is:

• required: false
• nullable: false
• and has a default

the intended behavior is usually:

• property omitted → use/preserve the default
• property present with a non-null value → use that value
• property present with null → reject as invalid

So in that case, @JsonSetter(nulls = Nulls.FAIL) protects default from being bypassed by explicit null values in the payload.

[mfunda-cen59049]

Thank you for looking into this problem. For the time being, we'll stay on 7.22.0 until the fix for openApiNullable=false is implemented. We'll also look for a better solution on our side.

The XKDC is on point though 😄

Appreciate your support!

[Picazsoo]

for optional, non-nullable properties:

• when openApiNullable=false -> I would go with @JsonSetter(nulls = Nulls.SKIP) to protect the default and in general make the deserialization more lenient.
• when openApiNullable=true -> I would go with @JsonSetter(nulls = Nulls.FAIL) to protect the default and in general make the deserialization more strict to clearly signal the difference for e.g. PATCH semantics.

What do you think @mfunda-cen59049 ? Does that make sense?

[mfunda-cen59049]

That makes sense to me. I think the Nulls.SKIP default is reasonable, at least for our use case.

One thing to note is that this change would make model generation inconsistent between Java and Kotlin classes. In Java, I've observed that openApiNullable=false does not generate the annotation at all, so the default behavior is used instead, which I believe is Nulls.SET.

That's not really an issue in our codebase, so I don't mind it. However, I suppose some projects using the generator could run into this inconsistency, especially when mixing Java and Kotlin source code within a single project, which is pretty typical.

[Picazsoo]

I think this @JsonSetter(nulls = Nulls.SKIP) fix should be done in java-spring codegen as well if it is confirmed that sending explicit JSON null for non-nullable optional attribute overrides expected default. The current behavior is dangerous.

And AFAIK historically the behavior of the two codegens was quite diverged. It is only in the last year that they are becoming closer in parity hence probably making devs think of them as two sides of the same coin. But I guess that is a good problem to have.

[Picazsoo]

I have discussed this with the main maintainer and his view is that if we (I and you @mfunda-cen59049 ) are in agreement, then I should proceed with the implementation.

So I will go with for optional, non-nullable properties:

• when openApiNullable=false -> I would go with @JsonSetter(nulls = Nulls.SKIP) to protect the default and in general make the deserialization more lenient.
• when openApiNullable=true -> I would go with @JsonSetter(nulls = Nulls.FAIL) to protect the default and in general make the deserialization more strict to clearly signal the difference for e.g. PATCH semantics.

I will adjust java-spring to behave the same way for the @JsonSetter(nulls = Nulls.SKIP). I will not tighten the behavior for openApiNullable=true in java-spring for now.

I will tag you in the PR for review as well, is that ok @mfunda-cen59049 ?

[mfunda-cen59049]

Sounds good to me, thank you!