-
-
Notifications
You must be signed in to change notification settings - Fork 7.1k
[kotlin][moshi] add polymorphic support #9159
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
base: master
Are you sure you want to change the base?
[kotlin][moshi] add polymorphic support #9159
Conversation
👍 Thanks for opening this issue! The team will review the labels and make any necessary changes. |
Short note: This is also a first step to support #8059 together with Moshi. |
{{#moshi}} | ||
@Json(name = "{{{vendorExtensions.x-base-name-literal}}}") | ||
{{/moshi}} | ||
{{#moshi}}{{^discriminator}}@Json(name = "{{{vendorExtensions.x-base-name-literal}}}"){{/discriminator}}{{/moshi}} |
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.
{{#moshi}}
{{^discriminator}}@Json(name = "{{{vendorExtensions.x-base-name-literal}}}"){{/discriminator}}
{{/moshi}}
to avoid blank line in the output.
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.
Changed. Thanks for the hint 👍
@@ -51,6 +52,8 @@ import java.util.concurrent.atomic.AtomicInteger | |||
import java.util.concurrent.atomic.AtomicLong | |||
{{/kotlinx_serialization}} | |||
|
|||
{{#moshi}}import {{modelPackage}}.*{{/moshi}} |
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.
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.
Changed. Thanks for the hint 👍
Please test with https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/java/petstore-with-fake-endpoints-models-for-testing-with-http-signature.yaml#L1927 instead. For oneOf implementation in Kotline, the oneOf impelmentation in |
a811156
to
498086c
Compare
@kvn-stgl adding support for oneOf is great, thanks for creating this PR 👍 |
Any updates? And I want to adopt the same logic for kotlinx.serialization |
Not that I'm aware of |
where I can look on reference implementation of polymorphic codegen? As I see in the PR author don't add template with inheritance of sealed class in subclasses |
@4brunu I'm going start working on resolving the issues in this PR. Where should I submit my work when I'm done? |
You should fork this repo, make the changes in your fork, and then create a PR back to this repo. |
Currently it's not possible to use Kotlin and Moshi with the oneOf-Keyword. I tried to implement the
PolymorphicJsonAdapterFactory
to support this feature.The reason why I set the target branch to
master
is because the current strategy will fail during runtime (it's not allowed with moshi to serialize a interface directly).I tested it with the following config:
Saddly in the current petstore samples are no discriminator, that's why I didn't add a sample. My plans for the future are to directly change the interface to a
sealed class
. Any recommendations for the strategy therefore?PR checklist
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*
.For Windows users, please run the script in Git BASH.
master
,5.1.x
,6.0.x
technical committee : @jimschubert @dr4ke616 @karismann @Zomzog @andrewemery @4brunu @yutaka0m