-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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
Proposal to improve wording of 'discriminator object' section #2247
Comments
@tedepstein, in #2141 two of your sentences caught my attention: # Require type==obj1, so a standard JSON schema validator can discriminate.
# This duplicates the discriminator logic, so should not be required with
# an OAS-compliant validator.
type:
enum:
- obj1 I think you are providing two clarifications points about the OAI spec:
Am I interpreting your statements correctly? IMO item (1) is not controversial. Item (2) is controversial. Whenever we have discussion amongst tooling developers, there are disagreements about the correct interpretation of the discriminator object section of the OAI spec. In my discussions, I hear two different interpretations of the OAI spec:
It would be great to clarify the discriminator object section of the OAI spec to eliminate the ambiguity. Maybe the TSC already has a very clear interpretation, in which case it's simply a matter of improving the wording. Or maybe there are a few points that need further discussion, I don't know. |
Yes, functionally equivalent for validation purposes. But not necessarily equivalent for code generators and other processors, unless those processors use a validator to determine the subtype. My other comments in #2141 explain this in more detail.
I think the first interpretation is correct. Wherever JSON Schema and OAS Schema Object have syntax in common, the semantics should be exactly the same. A The A code generator or some other processor may not claim to be a validator, and in that case, it may choose not to enforce The comments you quoted were from an example of a "hybrid" schema that I wrote in order to illustrate how a single schema could use native JSON Schema for validation purposes, but also include So a schema that uses |
ok. In that case IMO there is one recommendation for OAS authors that could greatly improve the runtime performance of the validation process and reduce CPU amplification attacks. Assume all BTW, declaring a single enum value is particularly helpful when the If on the other hand, the Don't you think some of these considerations and recommendations should be in the OAI spec? The spec is very light on that topic. For example, every RFC must include a security consideration section with guidelines about DoS attacks and countermeasures.
In our OAS docs we use discriminator and
|
After OAS 3.1 catches up to the most recent JSON Schema, which supports modular extension vocabularies, we expect additional work on one or more code generation vocabularies, which will (hopefully) replace Your proposal can be implemented in modern JSON Schema by using and type: object
oneOf:
- if: {properties: {foo: {const: x}}}
then: {$ref: #/components/schemas/a}
else: false
- if: {properties: {foo: {const: y}}}
then: {$ref: #/components/schemas/b}
else: false
- if: {properties: {foo: {const: z}}}
then: {$ref: #/components/schemas/c}
else: false (the This makes existing validators only perform a cheap |
I liked the proposal you had two years ago, it provided a simple, intuitive, declarative syntax that could be used to express common patterns including polymorphism. Now it looks like the extended vocabulary will be based on imperative syntax with control flows. On one hand I can see control flows introduce powerful constructs. For example, this could be used (and abused) to encode validation rules that span across multiple properties. But this is a very convoluted way as a grammar to express polymorphism with efficiency in mind; the OAS author will now be responsible for writing documents with a control flow logic to achieve performance goals. In practice there will be performance discrepancies across tools. This will open the door for people writing OAI compilers and optimizers because potentially the control flow logic could be very complex for humans to write efficiently. I'm concerned this is going down the path of adding more and more imperative logic.
Hmm, the idea of control-flow being a modern way to express polymorphism. |
@sebastien-rosset I suspect what I proposed two years ago would also still work. There's another approach in Appendix E of draft 2019-09. The reason I brought up the |
Some concepts can be thought of in terms of control flow, but hopefully that does not imply these concepts should be modeled using keywords that are typically found in imperative programming. Today, most if not all of the concepts in JSON schema and OAI spec are specified declaratively. This provides a concise and intuitive way for authors to express intent and consumers to reason about the intent. From the tooling perspective, IMO there has been a good trade-off to make the implementation relatively easy. I am concerned using if/then/else is a non-intuitive, overly verbose approach to express polymorphism. Polymorphism is very a common design pattern so hopefully the schema can have constructs to specify concisely. As a comparison point, polymorphism is expressed declaratively in many modeling tools, programming languages and other specification languages. As a secondary consideration, the if/then/else construct would make it significantly harder for parsers, validators and code generators to implement. Just to be clear, I'm not arguing the extended vocabulary should never support if/then/else, I can see benefits for other use cases, or possibly to support unusual subtyping rulesets.
|
Now that 3.1 RC-0 has been released, I see 3.1 RC-0 has the following text which is incorrectly referring to the 3.0 release:
My suggestion is to fix and improve the 3.1 spec, regardless of what may come in the next release. |
@sebastien-rosset is there a summary of what you think needs to be done now, based on the current state of the in-development 3.0.4 and 3.1.1 releases? I see this is mostly pre-3.1.0 and it's a lot to go through to try to figure out what might or might not apply. |
This issue has been labeled with |
It would be worthwhile to clarify the wording of the discriminator object section in the OAI specification, regardless of the long term direction.
Some of us have repeatedly done a line-by-line review of the discriminator object section in the existing OAI spec; unfortunately different people cannot seem to agree on a single interpretation, or they say the spec is ambiguous. As a result, implementations differ on the validation logic (and also in related use cases such as unmarshaling). I am aware of the many issues about discriminators, including #2143, but I haven't seen a proposal to improve the wording.
With this in mind, we've tried to identify the sentences in the spec that cause ambiguity, explain why we think there are ambiguities, and propose ways to improve the wording. We used a PR to hopefully trigger a discussion: #2216
Even if discriminator is completely removed, I think it's still worth clarifying because products tend to stick to an OAS version for a long time, and it would help to bring consistency across tools.
The text was updated successfully, but these errors were encountered: