Improve "Discriminator object" section of the OAS specification. #2216
Conversation
| @@ -2689,10 +2689,10 @@ This object MAY be extended with [Specification Extensions](#specificationExtens | |||
|
|
|||
| The discriminator object is legal only when using one of the composite keywords `oneOf`, `anyOf`, `allOf`. | |||
|
|
|||
| In OAS 3.0, a response payload MAY be described to be exactly one of any number of types: | |||
| A payload MAY be described to be exactly one of any number of types: | |||
There was a problem hiding this comment.
Remove "In OAS 3.0" words:
- It is the 3.1 spec, not 3.0
- There is no need to start every sentence with "In OAS X.Y". The entire document is about a specific version of the OAS spec (in this case 3.1).
| @@ -2689,10 +2689,10 @@ This object MAY be extended with [Specification Extensions](#specificationExtens | |||
|
|
|||
| The discriminator object is legal only when using one of the composite keywords `oneOf`, `anyOf`, `allOf`. | |||
|
|
|||
| In OAS 3.0, a response payload MAY be described to be exactly one of any number of types: | |||
| A payload MAY be described to be exactly one of any number of types: | |||
There was a problem hiding this comment.
I suggest removing the word "response" both in the sentence and YAML example, because the discriminator can be used in the request and response. I know this is just an example, but IMHO it's better to show examples that cover as many use cases as possible
|
|
||
| When using the discriminator, _inline_ schemas will not be considered. |
There was a problem hiding this comment.
I am proposing to reword this sentence because it is ambiguous.
Problem 1: the verb "consider" is not conducive to normative precision. consider: "to think carefully about (something), typically before making a decision." "take (something) into account when making an assessment or judgment."...
Problem 2: does this sentence mean the discriminator is not required when using inline schema?The sentence is too ambiguous to conclude decisively, especially when you put that sentence side-by-side with this other sentences:
The
discriminatorfield MUST be a required field in the schema object.
and:
The expectation now is that a property with name
petTypeMUST be present in the response payload.
| @@ -2334,7 +2334,6 @@ The OpenAPI Specification allows combining and extending model definitions using | |||
| While composition offers model extensibility, it does not imply a hierarchy between the models. | |||
| To support polymorphism, the OpenAPI Specification adds the `discriminator` field. | |||
| When used, the `discriminator` will be the name of the property that decides which schema definition validates the structure of the model. | |||
| As such, the `discriminator` field MUST be a required field. | |||
There was a problem hiding this comment.
I have moved this statement in the "Discriminator Object" section. Grouping all the normative statements in one place helps to understand the specification and how multiple normative statements interact with each other.
| @@ -2687,23 +2691,23 @@ Field Name | Type | Description | |||
|
|
|||
| This object MAY be extended with [Specification Extensions](#specificationExtensions). | |||
|
|
|||
| The discriminator object is legal only when using one of the composite keywords `oneOf`, `anyOf`, `allOf`. | |||
| ##### Discriminator Object Example | |||
There was a problem hiding this comment.
Added this section title to clearly differentiate between the normative statements versus the example section.
| When using the discriminator, _inline_ schemas will not be considered. | ||
| The discriminator object is legal only when using one of the composite keywords `oneOf`, `anyOf`, `allOf`. Both _inline_ and references are valid child schemas. | ||
| When a child schema is a reference, the discriminator property _MUST_ be present in the payload. | ||
| A discriminator MAY act as a "hint" to shortcut validation and selection of the matching schema which may be a costly operation, depending on the complexity of the schema. |
There was a problem hiding this comment.
I moved this sentence up because it was in the middle of an example. Isn't this sentence always true regardless of the specific example that was provided? If so, I think it would make sense to put it outside the example section and along with other normative statements.
In addition, I have added one more proposed sentence. The spec should explicitly mention that even if the discriminator is not used as a hint (since the above sentence uses "MAY"), the payload must be validated against the JSON schema.
There was a problem hiding this comment.
How about using one of the composite keywords -> using one or more of the composite keywords
You can have a schema that includes allOf and oneOf where the payload conforms to both definitions, the allOf validation passes and those properties are loaded in as additional properties in the chosen oneOf schema.
There was a problem hiding this comment.
How about
using one of the composite keywords->using one or more of the composite keywords
You can have a schema that includes allOf and oneOf where the payload conforms to both definitions, the allOf validation passes and those properties are loaded in as additional properties in the chosen oneOf schema.
Thank you I have updated the sentence accordingly.
| When a child schema is a reference, the discriminator property _MUST_ be present in the payload. | ||
| A discriminator MAY act as a "hint" to shortcut validation and selection of the matching schema which may be a costly operation, depending on the complexity of the schema. | ||
| Even when the discriminator is used as shortcut hint, the payload MUST still be validated against the JSON schema. | ||
| When a child schema is specified _inline_, the discriminator property is not required to be present in the payload. Validation is applied to determine if the payload matches the child JSON schema. |
There was a problem hiding this comment.
As a special case, what if all child schemas are specified inline? Does this mean the discriminator is completely ignored and has no effect?
| When a child schema is a reference, the discriminator property _MUST_ be present in the payload. | ||
| A discriminator MAY act as a "hint" to shortcut validation and selection of the matching schema which may be a costly operation, depending on the complexity of the schema. | ||
| Even when the discriminator is used as shortcut hint, the payload MUST still be validated against the JSON schema. | ||
| When a child schema is specified _inline_, the discriminator property is not required to be present in the payload. Validation is applied to determine if the payload matches the child JSON schema. |
There was a problem hiding this comment.
This is a proposal to clarify When using the discriminator, inline schemas will not be considered. I am not claiming my proposed change is necessarily correct AS IS. The problem is that we find that sentence ambiguous. I wrote something in the hope that it can incrementally lead to a better wording of the spec.
| oneOf: | ||
| - type: 'null' |
There was a problem hiding this comment.
One special case is the "null" type, which is introduced in 3.1. I am suggesting to provide one "null" type example to present the implications of the "null" type when it is used along with a discriminator object and oneOf/anyOf. There is only one sentence in this 3.1 draft spec that mentions the "null" type.
My understanding is this schema is supported, and it is the new recommended approach in lieu of the deprecated "nullable". But the "null" type does not have a discriminator field in its payload, so adding it as an example would help to clarify that it's ok to use.
On one hand, the "null" type does not have a discriminator value which breaks the normative statement about the discriminator being a required field. But on the other hand, it seems to be valid because of the following sentence: When using the discriminator, inline schemas will not be considered.
Similarly, it would be great to provide a discriminator example with inline primitive types, e.g. string, boolean:
MyType:
oneOf:
- type: 'null'
- type: string
- $ref: '#/components/schemas/Cat'
- $ref: '#/components/schemas/Dog'
- $ref: '#/components/schemas/Lizard'
discriminator:
propertyName: petType propertyName: petType
versions/3.1.0.md
Outdated
| @@ -2677,7 +2676,14 @@ components: | |||
|
|
|||
| When request bodies or response payloads may be one of a number of different schemas, a `discriminator` object can be used to aid in serialization, deserialization, and validation. The discriminator is a specific object in a schema which is used to inform the consumer of the document of an alternative schema based on the value associated with it. | |||
|
|
|||
| When using the discriminator, _inline_ schemas will not be considered. | |||
| The discriminator object is legal only when using one of the composite keywords `oneOf`, `anyOf`, `allOf`. Both _inline_ and references are valid child schemas. | |||
There was a problem hiding this comment.
The normative statements and examples in this section assume the child schemas are objects. What about string, integer, boolean, array, number and "null" type? On one hand, these types should not be allowed because the discriminator field MUST be a required field. On the other hand, these types seem to be supported as inline schemas, because When using the discriminator, inline schemas will not be considered. But why should primitive types be supported only as inline types?
This PR attempts to clarify the Discriminator Object section of OAS 3.1. Discriminators seem to cause a number of questions, confusion and interoperability issues. I think some of these questions could be addressed by formulating the normative statements more precisely. I am aware of #2143 but a goal of deprecating the discriminator object is not in conflict with improving the existing specification.