Skip to content
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

Omit discriminator property for Kotlin serialization #342

Conversation

ulrikandersen
Copy link
Collaborator

Discriminated oneOf models generated with Kotlin serialization will currently not work because the discriminator property causes a name conflict with Kotlin Serialization's class discriminator:

Sealed class 'subclass' cannot be serialized as base class 'com.example.models.Superclass' because it has property name that conflicts with JSON class discriminator 'type'.

The issue was not discovered earlier because the OpenAPI spec for end-2-end test for Kotlin serialization models did not define the discriminator property on the child schemas. This PR adds the missing type property to the test.

In order for the generated code to be valid with Kotlin serialization we must omit the property in the models. The solution in this PR is to simply skip adding the property as part of the code generation process.

Skipping properties in the generation process has the side effect that if the discriminator is the only property on the schema we are left with a class with zero properties which must then converted to an object (example). It would be more clean to filter out the property before the generation step, but I am not sure where we would hook in in order to do so.

I am really open to feedback on this approach and to any suggestions for improvements.

Generating the discriminator property causes a name conflict with Kotlin Serialization's class discriminator.

Example: `Sealed class 'subclass' cannot be serialized as base class 'com.example.models.Superclass' because it has property name that conflicts with JSON class discriminator 'type'.`
Copy link
Owner

@cjbooms cjbooms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cjbooms cjbooms merged commit 6b32b43 into cjbooms:master Dec 20, 2024
1 check passed
@ulrikandersen ulrikandersen deleted the kotlin-serialization-discriminator-property branch December 20, 2024 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants