[KOTLIN-SPRING;JAVA-SPRING] bugfix: update JsonInclude annotations for optional+nullable fields to handle serialization correctly when openApiNullable = true - #24185
Conversation
…andle serialization correctly
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Picazsoo
changed the title
Picazsoo
changed the title
|
We just ran into this fix when renovate updated openapi generator from 7.23.0 to 7.24.0 where it is included: our openapi spec has an optional field with nullable: true, and beginning with 7.24.0, the https://github.com/field:JsonInclude(JsonInclude.Include.NON_NULL) annotation is added. (We're using kotlin-spring here). Consequently, the serialized json doesn't include the field anymore when it is set to null. Is this intended? (add. info: we're not using openApiNullable here) |
|
I also think we should be careful with these annotations in generated code because they make it difficult to override them. |
Hello @ugrepel , I would need to see a sample spec. How do you specify the optRefSiblingNullable:
nullable: true
$ref: '#/components/schemas/Sub'instead of: optRefSiblingNullable:
nullable: true
allOf:
- $ref: '#/components/schemas/Sub'or you are using |
You raise a fair point about override precedence, but I'd argue the annotation is the right call here - specifically for the optional non-nullable case. For an optional + non-nullable field, the Kotlin Why the per-field annotation beats "just configure the
For completeness, the nullable + optional case (without The kernel of truth in your concern: field-level So the annotation isn't an arbitrary opinion baked into the code. It is the schema's own nullability contract expressed at the only granularity that can represent it. I am looking forward to what your thoughts are on the above. |
uhhh... yes, we're using OAS 3.1.0 here. If nullable is ignored it won't matter, but we've put the nullable directly in the properties definition of the field in question, no $ref in use here either. Can't post our source here, but in general it looks like If OAS 3.1.x ignores nullable - what's the suggested way to get a null value into a JSON field now? |
|
Thanks for the prompt reply. In OAS 3.1 the way to specify nullability is to specify it in the components:
schemas:
MySchema:
type: object
required:
- id
properties:
id:
type: integer
format: int32
myOptionalField:
type:
- string
- "null"
description: my optional fieldDoes that resolve the issue? I checked and the current validation message when using |
|
For the misleading validation message I opened a PR here #24389 |
Yes, it does, thanks for the info! Following OAS spec changes just isn't for the faint of heart... |
Hard agree. I have learned a lot since I started contributing to this project. And there is still a lot more I probably do not grasp. |
|
The problem with the spec is that you can interpret it in different ways, and many people have opinions about it. You can see this in the various tickets too. I keep trying to say: take a step back and look at your use case independently of the spec, and what you can actually express with it. So far there haven't been good examples or use cases, even though people can somehow express things in the spec that make it seem "correct." Nullable should always be considered in relation to required/mandatory. Let me give a few examples. Say you have a person where the name is mandatory. What semantic difference does it make (independent of the spec) whether the field is absent, blank, or null in the JSON? From my point of view, it makes no semantic difference — the field simply isn't there, and accordingly it's an error. The same applies to mandatory collections, i.e., you can have multiple names, but at least one is mandatory. What semantic difference does it make in JSON whether your collection is absent, null, or an empty array? From my point of view, it makes no semantic difference. Applied to the spec, the case of required and nullable therefore makes little sense. Now let's assume you have an optional name. Same question: what semantic difference does it make (independent of the spec) whether the field is absent, blank, or null in the JSON? From my point of view, none — every time it should result in an Optional.empty Now there's an exception case where the word NULL really does have a different semantic meaning than absent and blank, and that's the case of JSON Merge Patch, because there it means "reset." That's where you run into the semantic problem of needing a tri-state:
From my point of view, you could have avoided this by giving the "null" state a meaning through meaningful names instead. And this is where the "evil" begins. Now you have to distinguish this case in the spec(optional vs jsonnullable), and nullable was used for that (maybe not a good choice and didn't look into the new spec with >3.1). Since you can now represent 4 states with required and nullable, everyone interprets it differently, and everyone argues about the technical solutions and what's possible with them. But I believe that to truly solve this problem once and for all, you need to get clear on the semantics as mentioned above, and on what problem you actually want to solve. Then you document the states and their meaning once, and it's done. That's why I think so far the spec can represent the semantics quite well and implement REST best practices quite well:
I've also seen cases where people want to use Maybe you have other examples for why you really need this. So far I've mostly seen examples that sound like workarounds. By the way, I don't think the Spring defaults are a good argument, because you have to adjust most of them anyway. |
|
I'm actually fine with the spec and what it can express. As the JsonPatch reference shows there is a somewhat subtle difference between null and absence, and, other than e.g. Java or Kotlin, JSON is able to represent it directly. We actually didn't have either - nullable: true or the OAS 3.1.0 variant of type: -string - "null" and fall into the trap with the automated update that fixed the generator bug (and showed our bug in the spec). Asking our current favorite AI presented us with the nullable: true solution which didn't work, and that brought me here. Digging deeper with the AI might have found the right solution for OAS 3.1.0 as well, but in this case, Picazsoo was faster. Also, the OAS 3.1.0 change to specifying "null" as one of the types seems reasonable, just a bit unexpected. |
|
@MelleD Thanks, this is a really useful framing and I agree with a lot of it. Let me go through it piece by piece.
Fully agree with the method. Reasoning from semantics is the right lens, and I'll use exactly that below.
This is where I'd push back. Your examples are ones where
Agreed that merge-patch is the canonical tri-state case. Your "use meaningful names instead" alternative works in some designs, but it isn't always available: you often don't own the wire format (external/legacy API, RFC 7386 endpoints), and JSON already represents absent-vs-null directly, so inventing sentinel field names is itself the workaround. The required+nullable case above is really just the explicit, always-sent sibling of the same distinct-null semantics you grant here.
This is a reasonable set of defaults, but I don't think it's a complete semantic model. It only covers three of the four combinations — required + nullable is simply absent, which quietly assumes the "required+nullable makes little sense" claim I pushed back on above. And "JsonNullable only for patch" is narrower than the semantics warrant: the absent-vs-null distinction it captures isn't unique to merge-patch. But set that aside, because my point is orthogonal to which rows we bless: given that these combinations already get generated today, where must the null-inclusion decision live? Even across your own rows, the required wire behavior differs per field (optional non-nullable must omit
The strongest one isn't exotic: three fields in the same class. One optional non-nullable, one optional nullable, and one required nullable. Granting your semantic model, how would a single global ObjectMapper inclusion setting serialize all three correctly? Walk through the options (
The reason no global setting can win is structural: under
Agreed, and I want to be explicit: my argument does not rest on Spring's default. Even assuming you tune the mapper freely, there is still no single global inclusion value that is simultaneously correct for these fields in the same class, because at the JVM level they're indistinguishable ( ( On the practical path forward: the annotations are inherently per-field, since the correct inclusion differs field-to-field, and I'd keep that as the default (annotations on). Maybe we can add a new |
|
One more angle that I think cuts to the heart of it: it's worth being precise about what these annotations actually take away from anyone, because I don't think it's anything legitimate. For an optional non-nullable field, Kotlin It's also worth looking at what the generator actually emits, because it makes the "this is too strict / hard to override" worry less sharp than it sounds. For an optional non-nullable field the template doesn't only add @field:JsonInclude(JsonInclude.Include.NON_NULL)
@field:JsonSetter(nulls = Nulls.SKIP)That's Postel's law applied per field, and only on the
So the annotations aren't a rigid contract that rejects real traffic. The generator is strict about the invalid thing it produces and tolerant about the invalid thing it receives, which is exactly the robustness principle for this field shape. That's why the "hard to override" concern really reduces to "I can't make the generator emit output that contradicts its own schema", which isn't a use case being lost, it's a bug being prevented. If someone genuinely wants And that's also why I don't think this changes the status quo for any valid payload: it only removes an invalid one. Concretely, a field the schema says can't be |
|
@Picazsoo - I fully agree: you fixed a bug causing invalid output, which removed the hiding of a bug on our side, so I had to fix our bug too by adding the proper type: [..., "null] specification. |
But that's not a good example for. If you don't have a middle name, it's simply “Optional.” I mean, that's the very definition of an optional case, and I would never treat it as an error.
See, and already we’re drifting off into technical discussions ;). Let’s take a step back—why should there even be three different fields like that from a semantic standpoint? What’s the use case? Sure, you can model and program anything. People also use In my current APIs, I use the semantic principles mentioned above with the JsonMapper’s |
I agree somewhat with that as advice for someone designing an API. Your principles (avoid But this mostly affects the generator in client mode (e.g. spring-cloud or declarative http interface), and a client is by definition a consumer of an API it doesn't own. The person generating a client almost never controls the spec; it belongs to a third party, a legacy system, or another team. They can't "reconsider the use case," because someone else already decided it and shipped it. Their only job is to talk to that server correctly. So "why would you ever model three fields like that?" isn't a question the generator's user gets to answer. If the upstream spec declares those fields, the client has to serialize each one the way that server expects, or the integration breaks. Whether the design is wise isn't ours to adjudicate. That's the difference in our positions:
The middle-name case resolves the same way. In an API you own, collapsing absent/null/blank into This is also why the per-field annotation isn't us baking in an opinion; it's the opposite. It's what lets the client reproduce the spec author's intent at the only granularity where the distinction survives ( So I'd still land it as: per-field annotations on by default (faithful to the contract), plus the opt-out On the concrete path forward: this ties into #24401, where a stricter global setting (e.g.
The So even though we don't fully converge on the modeling philosophy, the practical outcome should satisfy both of us: the configurable option restores the ability to defer entirely to the global The one caveat I'd flag for the record: opting into |
|
If anyone has an example of such a legacy API, that's fine, but as I've already said, no real-world example has come up yet. But I think to handle as client is much easier.
Yeah, sure, but if you're “just” the client, then you can do exactly that—instead of “absent,” you just enter null. That should be possible, right directly in the object mapper. As I said, I don't want to judge whether the use case here is right or wrong in general just yet; I need to do an update first, and then I'll be able to see. |
|
Here's the concrete, non-patch, real-world use case you asked for: a search/filter endpoint. It needs a genuine tri-state on ordinary fields, and it's a case where the client doesn't own the spec. ScenarioA read-only endpoint
Absent, OpenAPI schema (OAS 3.1)components:
schemas:
TaskFilter:
type: object
description: >
All properties are OPTIONAL (none required), so an absent property means
"do not filter by this attribute". Properties whose type permits "null"
additionally allow an explicit null to mean "match records whose value IS NULL".
properties:
# optional + nullable -> three states: absent / null / value
assigneeId:
type: [integer, "null"]
format: int64
description: >
Absent = no assignee filter.
null = match tasks with NO assignee.
number = match tasks assigned to that user id.
# optional + nullable -> three states as well
status:
type: [string, "null"]
enum: [OPEN, IN_PROGRESS, DONE, null]
description: >
Absent = no status filter.
null = match tasks with NO status set.
value = match tasks with that status.
# optional + NON-nullable -> only two meaningful states: absent / value
# null here is NOT a query value; it just means "not provided".
textContains:
type: string
description: Absent = no text filter. A value = substring match.
# optional + NON-nullable paging control
limit:
type: integer
format: int32
minimum: 1
description: Absent = server default page size. A value overrides it.Key facts:
Generated Kotlin model (kotlin-spring,
|
| Kotlin value | Serialized JSON | Query meaning |
|---|---|---|
JsonNullable.undefined() |
(omitted) | no assignee filter |
JsonNullable.of(null) |
"assigneeId": null |
find unassigned |
JsonNullable.of(42L) |
"assigneeId": 42 |
assigned to 42 |
NON_ABSENT is what drops undefined() while still emitting the meaningful null. Without it, undefined() leaks as "assigneeId": null, silently turning "no filter" into "find unassigned" — a different query.
Why a single global ObjectMapper setting can't do this
Serialize one TaskFilter that sets assigneeId = null (find unassigned) but leaves limit unset. Walk the global options:
Include.ALWAYS→ emits"assigneeId": null✅ but also emits"limit": null❌ (spurious/invalid criterion on a non-nullable field).Include.NON_NULL→ omits"limit": null✅ but also omits"assigneeId": null❌ (silently drops the "find unassigned" query, turning it into "no filter").- Any other single value → applies uniformly; still can't emit null for one field while omitting it for the other.
The two fields demand opposite null-handling in the same object, and under openApiNullable=false both are just T? = null on the JVM — indistinguishable to a mapper-level policy. That's the structural reason the choice has to live per field, not in the global mapper.
And the client here doesn't own the spec — it's just trying to query the endpoint correctly. It can't "reconsider the use case"; it has to reproduce absent-vs-null-vs-value exactly as the server defines them. "Just set the mapper to emit nulls" makes assigneeId: null work but simultaneously corrupts limit, so it isn't a real substitute for the per-field annotation.
But I still feel like we are talking past each other. I tagged you in another issue. I would prefer to continue the conversation there if you don't mind. If you have time, I would be glad if you could give some feedback on the proposed solution.
|
One clarification on the ObjectMapper argument: its scope is bigger than "a few awkward fields in one class." The inclusion setting isn't per-class, it's per-serializer-instance, and a standard Spring setup has one shared mapper for the whole application. So any global The conflict therefore needs no contrived model. It appears the moment the project has, anywhere:
No single global value serves both: This gets sharper for a client/gateway/BFF talking to multiple upstream APIs, each with its own spec and DTOs, all serialized by the same mapper. Different vendors own those specs, so they disagree on null semantics: Upstream A (merge-patch/filter) needs "Use a second ObjectMapper" doesn't rescue this either: Jackson applies one mapper per serialization, so two fields in the same payload can't get different inclusion policies, and wiring per-upstream mappers into generated clients is plumbing nobody maintains. That's why the decision has to be per-field: the schema declares nullability per property, so only a per-property mechanism can honor it. A single application-wide mapper setting is a category mismatch, and it can't reconcile conflicting contracts from different upstreams in one process. |
|
But in your case, it's something that can already be done with the plain ObjectMapper and without annotation, when I understand you correct. |
…e serilisation became more strict see eg. OpenAPITools/openapi-generator#24185 and we get some fields annotated with @JsonInclude(JsonInclude.Include.NON_NULL)
…e serilisation became more strict see eg. OpenAPITools/openapi-generator#24185 and we get some fields annotated with @JsonInclude(JsonInclude.Include.NON_NULL)
4 participants
Fix:
@JsonInclude(NON_ABSENT)forJsonNullablefields in kotlin-spring and java-spring generatorsIssue
When
openApiNullable=true, optional+nullable fields are generated asJsonNullable<T>but without any@JsonIncludeannotation. This meansJsonNullable.undefined()(the "absent" state) could be serialized into the JSON output, defeating the purpose of the three-state wrapper.Fix
Added
@field:JsonInclude(JsonInclude.Include.NON_ABSENT)to any field wherex-is-jackson-optional-nullableis true, indataClassOptVar.mustache. This ensures:JsonNullable.undefined()JsonNullable.of(null)nullJsonNullable.of("value")"value"This is a nasty bug that should be fixed - because someone using the generated clients for patch operations might be unknowingly deleting resources even when setting JsonNullable as undefined.
PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
Summary by cubic
Fixes serialization of optional+nullable fields for
kotlin-springandjava-springwhenopenApiNullable=trueby adding@JsonInclude(JsonInclude.Include.NON_ABSENT)toorg.openapitools.jackson.nullable.JsonNullablefields. Undefined values are omitted; nulls and concrete values are included.kotlin-spring: emit @field:JsonInclude(JsonInclude.Include.NON_ABSENT) indataClassOptVar.mustachewhenx-is-jackson-optional-nullableis true.java-spring: emit @JsonInclude(JsonInclude.Include.NON_ABSENT) inJavaSpring/pojo.mustache; ensureJsonIncludeimport inSpringCodegen.postProcessModelPropertyfor optional+nullable fields whenopenApiNullable=true.openApiNullabletrue/false; added Kotlin runtime tests to verify undefined/null/value serialization.JsonNullablefields.Written for commit 000a348. Summary will update on new commits.