[kotlin-server] Add polymorphism, oneOf and allOf support #22610
Conversation
| |featureHSTS|Avoid sending content if client already has same content, by checking ETag or LastModified properties.| |true| | ||
| |featureMetrics|Enables metrics feature.| |true| | ||
| |featureResources|Generates routes in a typed way, for both: constructing URLs and reading the parameters.| |true| | ||
| |fixJacksonJsonTypeInfoInheritance|When true (default), ensures Jackson polymorphism works correctly by: (1) always setting visible=true on @JsonTypeInfo, and (2) adding the discriminator property to child models with appropriate default values. When false, visible is only set to true if all children already define the discriminator property.| |true| |
There was a problem hiding this comment.
This is where things get tricky. Let's take this schema as an example:
Pet schema
# https://spec.openapis.org/oas/v3.2.0.html#models-with-polymorphism-support-and-a-discriminator-object
# 4.24.8.8 Models with Polymorphism Support and a Discriminator Object
# The following example extends the example of the previous section by adding a Discriminator Object to the Pet schema. Note that the Discriminator Object is only a hint to the consumer of the API and does not change the validation outcome of the schema.
openapi: 3.1.0
info:
title: Basic polymorphism example with discriminator
version: "1.0"
components:
schemas:
Pet:
type: object
discriminator:
propertyName: petType
mapping:
cat: '#/components/schemas/Cat'
dog: '#/components/schemas/Dog'
properties:
name:
type: string
required:
- name
- petType
oneOf:
- $ref: '#/components/schemas/Cat'
- $ref: '#/components/schemas/Dog'
Cat:
description: A pet cat
type: object
properties:
petType:
const: 'cat'
huntingSkill:
type: string
description: The measured skill for hunting
enum:
- clueless
- lazy
- adventurous
- aggressive
required:
- huntingSkill
Dog:
description: A pet dog
type: object
properties:
petType:
const: 'dog'
packSize:
type: integer
format: int32
description: the size of the pack the dog is from
default: 0
minimum: 0
required:
- petType
- packSizeWhat the spec says about discriminator and petType
The discriminator is only a hint and does not change validation
In section 4.24.8.8, the spec explicitly says:
“the Discriminator Object is only a hint … and does not change the validation outcome of the schema.”
This means discriminator by itself doesn’t automatically “require” petType or enforce constraints.
Does the discriminator property have to be present in the payload?
Yes, if the parent schema requires it
In our Pet schema:
required:
- name
- petTypeSo the schema rules say: a valid Pet instance must contain petType, even if the discriminator is "just a hint." The required constraint is what makes it mandatory.
Also, in the discriminator examples section the spec states:
"The expectation now is that a property with name petType MUST be present in the response payload …"
So: yes, petType must be present in the payload, because the parent schema requires it and the discriminator usage assumes it exists.
openapi-generator behavior - with and without the Jackson fix applied
Without the fix, the generated code looks like this:
Without the fix
/**
* Basic polymorphism example with discriminator
* No description provided (generated by Openapi Generator https://github.com/openapitools/openapi-generator)
*
* The version of the OpenAPI document: 1.0
*
*
* NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech).
* https://openapi-generator.tech
* Do not edit the class manually.
*/
package org.openapitools.server.models
import org.openapitools.server.models.Cat
import org.openapitools.server.models.Dog
/**
*
*/
@com.fasterxml.jackson.annotation.JsonTypeInfo(use = com.fasterxml.jackson.annotation.JsonTypeInfo.Id.NAME, include = com.fasterxml.jackson.annotation.JsonTypeInfo.As.PROPERTY, property = "petType", visible = false)
@com.fasterxml.jackson.annotation.JsonSubTypes(
com.fasterxml.jackson.annotation.JsonSubTypes.Type(value = Cat::class, name = "cat"),
com.fasterxml.jackson.annotation.JsonSubTypes.Type(value = Dog::class, name = "dog")
)
sealed class Pet/**
* Basic polymorphism example with discriminator
* No description provided (generated by Openapi Generator https://github.com/openapitools/openapi-generator)
*
* The version of the OpenAPI document: 1.0
*
*
* NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech).
* https://openapi-generator.tech
* Do not edit the class manually.
*/
package org.openapitools.server.models
/**
* A pet cat
* @param huntingSkill The measured skill for hunting
* @param petType
*/
data class Cat(
/* The measured skill for hunting */
@field:com.fasterxml.jackson.annotation.JsonProperty("huntingSkill")
val huntingSkill: Cat.HuntingSkill,
@field:com.fasterxml.jackson.annotation.JsonProperty("petType")
val petType: kotlin.Any? = null
) : Pet()
{
/**
* The measured skill for hunting
* Values: CLUELESS,LAZY,ADVENTUROUS,AGGRESSIVE
*/
enum class HuntingSkill(val value: kotlin.String){
CLUELESS("clueless"),
LAZY("lazy"),
ADVENTUROUS("adventurous"),
AGGRESSIVE("aggressive");
}
}/**
* Basic polymorphism example with discriminator
* No description provided (generated by Openapi Generator https://github.com/openapitools/openapi-generator)
*
* The version of the OpenAPI document: 1.0
*
*
* NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech).
* https://openapi-generator.tech
* Do not edit the class manually.
*/
package org.openapitools.server.models
/**
* A pet dog
* @param petType
* @param packSize the size of the pack the dog is from
*/
data class Dog(
@field:com.fasterxml.jackson.annotation.JsonProperty("petType")
val petType: kotlin.Any?,
/* the size of the pack the dog is from */
@field:com.fasterxml.jackson.annotation.JsonProperty("packSize")
val packSize: kotlin.Int = 0
) : Pet()As you can see:
🧐 petType is not a property of the parent/sealed Pet class. While this works, Jackson does require petType in the JSON payload for being able to deserialize, so we might as well add it as a property to the sealed class for convenience. Note that we need visible = true in the JsonTypeInfo annotation for Jackson to populate this value.
🧐 the petType property being present in the children classes (Dog and Cat) now fully depends on whether they're defined in the YAML spec. In this case they are, but it's optional in Cat and required in Dog. And it's nullable in both cases. While the code generator correctly generated code according to spec here, its practical value is highly questionable, because Jackson requires petType to always be provided in order to be able to properly deserialize.
With the fix applied, it looks like this:
With the fix applied
/**
* Basic polymorphism example with discriminator
* No description provided (generated by Openapi Generator https://github.com/openapitools/openapi-generator)
*
* The version of the OpenAPI document: 1.0
*
*
* NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech).
* https://openapi-generator.tech
* Do not edit the class manually.
*/
package org.openapitools.server.models
import org.openapitools.server.models.Cat
import org.openapitools.server.models.Dog
/**
*
* @param petType
*/
@com.fasterxml.jackson.annotation.JsonTypeInfo(use = com.fasterxml.jackson.annotation.JsonTypeInfo.Id.NAME, include = com.fasterxml.jackson.annotation.JsonTypeInfo.As.PROPERTY, property = "petType", visible = true)
@com.fasterxml.jackson.annotation.JsonSubTypes(
com.fasterxml.jackson.annotation.JsonSubTypes.Type(value = Cat::class, name = "cat"),
com.fasterxml.jackson.annotation.JsonSubTypes.Type(value = Dog::class, name = "dog")
)
sealed class Pet(
@field:com.fasterxml.jackson.annotation.JsonProperty("petType")
open val petType: kotlin.String
)/**
* Basic polymorphism example with discriminator
* No description provided (generated by Openapi Generator https://github.com/openapitools/openapi-generator)
*
* The version of the OpenAPI document: 1.0
*
*
* NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech).
* https://openapi-generator.tech
* Do not edit the class manually.
*/
package org.openapitools.server.models
/**
* A pet cat
* @param huntingSkill The measured skill for hunting
* @param petType
*/
data class Cat(
/* The measured skill for hunting */
@field:com.fasterxml.jackson.annotation.JsonProperty("huntingSkill")
val huntingSkill: Cat.HuntingSkill,
@field:com.fasterxml.jackson.annotation.JsonProperty("petType")
override val petType: kotlin.String = "cat",
) : Pet(petType = petType)
{
/**
* The measured skill for hunting
* Values: CLUELESS,LAZY,ADVENTUROUS,AGGRESSIVE
*/
enum class HuntingSkill(val value: kotlin.String){
CLUELESS("clueless"),
LAZY("lazy"),
ADVENTUROUS("adventurous"),
AGGRESSIVE("aggressive");
}
}/**
* Basic polymorphism example with discriminator
* No description provided (generated by Openapi Generator https://github.com/openapitools/openapi-generator)
*
* The version of the OpenAPI document: 1.0
*
*
* NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech).
* https://openapi-generator.tech
* Do not edit the class manually.
*/
package org.openapitools.server.models
/**
* A pet dog
* @param petType
* @param packSize the size of the pack the dog is from
*/
data class Dog(
@field:com.fasterxml.jackson.annotation.JsonProperty("petType")
override val petType: kotlin.String = "dog",
/* the size of the pack the dog is from */
@field:com.fasterxml.jackson.annotation.JsonProperty("packSize")
val packSize: kotlin.Int = 0
) : Pet(petType = petType)As you can see:
✅ petType is now a required, non-nullable property on the parent/sealed Pet class. Since Jackson already requires it to be set for JsonTypeInfo to be able to deserialize things properly, and visible = true, we can be assured that the property will always be set by Jackson.
✅ The petType property is now present as a required, non-nullable property in the children classes (Dog and Cat). As mentioned above, Jackson already requires this in order to be able to deserialize properly.
This might not be desired behavior in all cases
Let's say I want to re-use the Cat class elsewhere without using the discriminator, it might not be expected that petType is now a required, non-nullable field on the Kotlin data class, because the spec says that it's not required for the Cat model. It's probably not the best example, but I hope you get my train of thought.
For this reason, I wanted to make this behavior configurable. But to be honest, it feels like a bit of an edge case, and we might as well not make it configurable (always applying this fix for Jackson). Curious what y'all think about this.
| @Getter | ||
| @Setter | ||
| private boolean fixJacksonJsonTypeInfoInheritance = true; |
There was a problem hiding this comment.
See my other comment above about this new config option and its behavior.
| // Enable proper oneOf/anyOf discriminator handling for polymorphism | ||
| legacyDiscriminatorBehavior = false; |
There was a problem hiding this comment.
Since oneOf and Polymorphism weren't supported at all in the kotlin-server generator before, I consider this to be a non-breaking change, even though the default is set to true in DefaultCodegen. Let me know if you think otherwise.
| // For libraries that use Jackson, set up parent-child relationships for discriminator children | ||
| // This enables proper polymorphism support with @JsonTypeInfo and @JsonSubTypes annotations | ||
| if (usesJacksonSerialization()) { |
There was a problem hiding this comment.
The kotlin-server generator supports 5 libraries at the time or writing:
| Library | Serialization library |
|---|---|
| ktor (Ktor v3) | kotlinx.serialization |
| ktor2 | kotlinx.serialization |
| jaxrs-spec | Jackson |
| javalin5 | Jackson |
| javalin6 | Jackson |
While ktor defaults to kotlinx.serialization, it also supports Jackson and GSON (not implemented in the openapi-generator AFAIK).
And for Javalin, it defaults to Jackson, but there's instructions to use GSON instead as well.
The kotlin generator has a config option called serializationLibrary, which we might actually want to introduce in the kotlin-server generator as well for consistency (ideally re-using code if possible).
Then, useJacksonSerialization would change from:
/**
* Returns true if the current library uses Jackson for JSON serialization.
* This is used to determine if Jackson-specific features like polymorphism annotations should be enabled.
*/
private boolean usesJacksonSerialization() {
return Constants.JAVALIN5.equals(library) ||
Constants.JAVALIN6.equals(library) ||
Constants.JAXRS_SPEC.equals(library);
}... to something that checks the value of the serializationLibrary config option instead. That would also open the door for supporting more serialization libraries in the future if folks want. WDYT?
| // When false, we only set visible=true if the parent has properties (allOf pattern) | ||
| boolean visibleTrue; | ||
|
|
||
| if (fixJacksonJsonTypeInfoInheritance) { |
There was a problem hiding this comment.
This is the actual implementation of the Jackson-specific fix as mentioned above.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 67 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="modules/openapi-generator/src/main/resources/kotlin-server/data_class_interface_var.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/kotlin-server/data_class_interface_var.mustache:4">
P1: Double nullability marker issue: when a property is both `isNullable=true` and `required=false`, this generates `Type??` which is invalid Kotlin syntax. The `{{^required}}?{{/required}}` should be nested inside a `{{^isNullable}}...{{/isNullable}}` block to prevent adding a second `?` when the type is already nullable.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/kotlin-server/libraries/javalin6/data_class_interface_var.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/kotlin-server/libraries/javalin6/data_class_interface_var.mustache:5">
P1: Potential double nullable marker (`??`) when property is both nullable and not required. If `isNullable=true` and `required=false`, this template generates invalid Kotlin syntax like `Type??`. Consider using a single conditional that checks either condition, e.g., `{{#isNullable}}?{{/isNullable}}{{^isNullable}}{{^required}}?{{/required}}{{/isNullable}}`.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
modules/openapi-generator/src/main/resources/kotlin-server/data_class_interface_var.mustache
Outdated
Show resolved
Hide resolved
...erator/src/main/resources/kotlin-server/libraries/javalin6/data_class_interface_var.mustache
Outdated
Show resolved
Hide resolved
1 participant
Partially fixes #18167 - the original issue was reported for the
kotlinclient generator while this PR is for thekotlin-servergeneratorBefore
❌ Jackson annotations for
JsonTypeInfoandJsonSubTypesare missing❌
Petis a regular data class instead of a sealed class❌
Pethas all properties in its constructor❌
DogandCatdon't inheritPetKotlin sample for Pet, Dog and Cat
After
✅ Jackson annotations for
JsonTypeInfoandJsonSubTypesare present✅
Petis a sealed class✅
Petdoesn't have any properties in its constructor; that's left up to the children✅
DogandCatinheritPetKotlin sample for Pet, Dog and Cat
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.
master(upcoming7.x.0minor release - breaking changes with fallbacks),8.0.x(breaking changes without fallbacks)"fixes #123"present in the PR description)Tagging the Kotlin technical committee members: @karismann (2019/03) @Zomzog (2019/04) @andrewemery (2019/10) @4brunu (2019/11) @yutaka0m (2020/03) @stefankoppier (2022/06) @e5l (2024/10)
Summary by cubic
Adds polymorphism support (oneOf and allOf) to the kotlin-server generator. Models now deserialize correctly with Jackson using sealed parents and proper child inheritance.
New Features
Migration
Written for commit 6df9076. Summary will update on new commits.