-
Notifications
You must be signed in to change notification settings - Fork 400
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
[OpenAPI] case class field being an Option of an ADT are not rendered as nullable #3053
Conversation
@@ -988,7 +988,7 @@ object OpenAPIGen { | |||
} | |||
private def schemaReferencePath(nominal: TypeId.Nominal, referenceType: SchemaStyle): String = { | |||
referenceType match { | |||
case SchemaStyle.Compact => s"#/components/schemas/${nominal.typeName}}" |
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.
Nasty typo, which was sadly validated by the tests here https://github.com/zio/zio-http/pull/3053/files#diff-0b5e5bc79ca85db4957d0ab782e9509a15f69b70d84ffd76a15a712367a860f3L2234-L2236
036f03f
to
d7e6375
Compare
d7e6375
to
751bd12
Compare
| "optionalAdtField" : { | ||
| "anyOf": [ | ||
| { "type": "null" }, | ||
| { "$ref": "#/components/schemas/SealedTraitCustomDiscriminator" } | ||
| ] | ||
| } |
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.
We now correctly have the anyOf
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.
I wonder, if this should be anyOf, or if the field should just be missing from the required fields list. I'd tend to the second. Most Scala json serialization does not write the field if the option is None
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.
Maybe both. To be accept all possible encodings
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.
Done. They are now both nullable and non-required
1c8f636
to
a665740
Compare
Fixes #3052