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

Codegen: serialize empty arrays and require them (jsoniter fix) #3655

Conversation

hughsimpson
Copy link
Contributor

@hughsimpson hughsimpson commented Apr 4, 2024

Direct follow-up to #3647 - uses new tests to validate implied expectations around required array fields. 'Real' diff is just the last commit here.

Whether this is really the right behaviour may be up for debate, although the new behaviour is the same as that expected by the non-discriminated-oneOf validation code, which will otherwise need to be made more convoluted/draconian.

reqJsonBody shouldEqual """{"d":23.4,"i":123,"s":"a string"}"""
respJsonBody shouldEqual """{"d":23.4,"i":123,"s":"a string+SubtypeWithoutD3"}"""
reqJsonBody shouldEqual """{"absent":null,"d":23.4,"i":123,"s":"a string"}"""
respJsonBody shouldEqual """{"absent":null,"d":23.4,"i":123,"s":"a string+SubtypeWithoutD3"}"""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Arguably could attempt to configure circe to omit these for great consistency; however I feel like that's going too far, and I think it's reasonable behaviour that can sanely differ between json lib implementations.

@hughsimpson hughsimpson force-pushed the codegen_serialize_empty_arrays_and_require_them-jsoniter-fix branch from d44ac64 to 813376a Compare April 5, 2024 07:35
@kciesielski kciesielski merged commit a7abd3b into softwaremill:master Apr 9, 2024
23 checks passed
@hughsimpson hughsimpson deleted the codegen_serialize_empty_arrays_and_require_them-jsoniter-fix branch April 9, 2024 14:40
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