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

Generators shouldn't add required discriminator property as a constructor parameter in generated models with inheritance #1168

Closed
runenielsen opened this issue Aug 16, 2023 · 2 comments · Fixed by #1167

Comments

@runenielsen
Copy link

runenielsen commented Aug 16, 2023

Expected Behavior

Required discriminator properties in models with inheritance should not be part of the constructor parameters, since they are not required to be set (they are generated automatically based on the class on serialization).

Currently it is actually a problem, if they are set, since the property will then be added twice to the generated JSON on serialization due to a bug in Micronaut Serialization (reported here: micronaut-projects/micronaut-serialization#550).

Actual Behaviour

Required discriminator properties are added as constructor parameters for generated models with inheritance.

Steps To Reproduce

  1. Clone the project https://github.com/runenielsen/discriminatorgeneration
  2. Build and inspect the generated classes to see, that the discriminator property type is added as a constructor parameter.
  3. Run the test SerdeWithDiscriminatorTest and see the test case testSerializationForDetailedBookInfoWithDiscriminatorSet fail due to the discriminator property being added twice in the generated JSON, when it is set in the model. And see that the test case testSerializationForDetailedBookInfoWithoutDiscriminatorSet works correctly, when it is not set in the model. And lastly notice that the property is not set in the model on deserialization in the test case testDeserializationForDetailedBookInfo.

I'm aware that this issue is sort of in between a bug and a feature request, since it is mostly an annoyance to have a property in the constructor, that doesn't need to be set, and the main bug is actually in Micronaut Serialization. But I figured, I'd report it as a bug, since the fields in the Bug Report issue type fit better 🙂

Environment Information

OS: MacOS
JDK: 17

Example Application

https://github.com/runenielsen/discriminatorgeneration

Version

4.0.3

@runenielsen runenielsen changed the title Change generators, so required discriminator property is not added as constructor parameter in generated models with inheritance Generators shouldn't add required discriminator property as a constructor parameter in generated models with inheritance Aug 16, 2023
altro3 added a commit to altro3/micronaut-openapi that referenced this issue Aug 16, 2023
Remove discriminator from required properties constructor.

Fixed micronaut-projects#1163
Fixed micronaut-projects#1168
@altro3
Copy link
Collaborator

altro3 commented Aug 16, 2023

@runenielsen Fixed it here too (two fixes in one PR): #1167

@runenielsen
Copy link
Author

@altro3 : Thanks 💪

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 a pull request may close this issue.

2 participants