-
-
Notifications
You must be signed in to change notification settings - Fork 7.1k
Kotlinx polymorphism with custom discriminator support #21531
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- added new config with kotlinx, discriminator (/w custom name) and kotlinx_serialization - remove discriminator properties from the generator in both base and derived classes - set discriminatorValue in additionalProperties of derived classes - add JsonClassDiscriminator the derived classes in the template - set SerialName to discriminatorValue in the template - change base classes to sealed class instead of interface - make variables in base classes abstract
.../openapi-generator/src/main/java/org/openapitools/codegen/languages/KotlinClientCodegen.java
Show resolved
Hide resolved
Great work, this is exactly what we are missing! Hopefully this can be merged somewhat quickly 👍 |
thanks for the pr let's give it a try |
animal: | ||
title: An animal | ||
type: object | ||
properties: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this spec incomplete? According to https://swagger.io/docs/specification/v3_0/data-models/inheritance-and-polymorphism/#discriminator, the discriminator property should be part of the listed properties.
I know this PR is already merged, but I'm hoping to get some clarification since we're looking forward to using this, but our spec does contain the discriminator property in the list of properties so I'm worried it won't work for us.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@F43nd1r I don't know about the spec being incomplete. I'm no expert there.
I've modified the test resource, added the discriminator to the list of properties. The generator outputed the same exact code, so you should be able to use it with your spec.
@Override | ||
public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> objs) { | ||
objs = super.postProcessAllModels(objs); | ||
if (getSerializationLibrary() == SERIALIZATION_LIBRARY_TYPE.kotlinx_serialization) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@some00 thank you so much!!
the only thing, this condition is missing kotlin-multiplatform, || getLibrary().equals(MULTIPLATFORM)
could you please fix that?..:)
@wing328 should this functionality somehow be related to SchemaSupportFeature.Union? |
I've created multiplatform port of this PR: #21772 |
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.0
minor release - breaking changes with fallbacks),8.0.x
(breaking changes without fallbacks)@dr4ke616 @karismann @Zomzog @andrewemery @4brunu @yutaka0m @stefankoppier @e5l