-
Notifications
You must be signed in to change notification settings - Fork 9.1k
enforce existence of composite applicator keyword adjacent to "discriminator" #3137
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
enforce existence of composite applicator keyword adjacent to "discriminator" #3137
Conversation
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 wish the spec said that discriminator
should be ignored if there is no anyOf
/oneOf
/allOf
. That would be more inline with JSON Schema conventions, but that's not what it says, so this change should be included.
schemas/v3.1/meta/base.schema.json
Outdated
"if": { | ||
"required": [ "discriminator" ] | ||
}, | ||
"then": { | ||
"anyOf": [ | ||
{ "required": [ "oneOf" ] }, | ||
{ "required": [ "anyOf" ] }, | ||
{ "required": [ "allOf" ] } | ||
] | ||
}, |
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 suggest going with dependentSchemas
for this. The end result is the same, but it's a little more concise.
"if": { | |
"required": [ "discriminator" ] | |
}, | |
"then": { | |
"anyOf": [ | |
{ "required": [ "oneOf" ] }, | |
{ "required": [ "anyOf" ] }, | |
{ "required": [ "allOf" ] } | |
] | |
}, | |
"dependentSchemas": { | |
"discriminator": { | |
"anyOf": [ | |
{ "required": [ "oneOf" ] }, | |
{ "required": [ "anyOf" ] }, | |
{ "required": [ "allOf" ] } | |
] | |
} | |
}, |
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 wish the spec said that
discriminator
should be ignored if there is noanyOf
/oneOf
/allOf
. That would be more inline with JSON Schema conventions, but that's not what it says, so this change should be included.
Indeed. This would be worthy of reconsidering for v3.2.
I suggest going with
dependentSchemas
for this. The end result is the same, but it's a little more concise.
Heh, I considered dependentRequired
but totally forgot about dependentSchemas
! :p
…minator" "The discriminator object is legal only when using one of the composite keywords oneOf, anyOf, allOf." https://spec.openapis.org/oas/v3.1.0#discriminator-object
bfd2767
to
b3a612d
Compare
@OAI/tsc Can we get another set of eyes and a merge? |
@darrelmiller I'll reread up on dependentSchemas tomorrow unless someone else gets there first. I'm sure it's OK. |
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.
LGTM, thanks.
Revert enforce existence of composite applicator keyword adjacent to "discriminator" #3137
The requirement that oneOf, anyOf or allOf is required was added to the schema in OAI/OpenAPI-Specification#3137 and then reverted for v3.1.1 in OAI/OpenAPI-Specification#3405.
"The discriminator object is legal only when using one of the composite keywords oneOf, anyOf, allOf."
https://spec.openapis.org/oas/v3.1.0#discriminator-object