Skip to content

Improve "Discriminator object" section of the OAS specification. #2216

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.

Sign up for GitHub

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

Closed
wants to merge 11 commits into from
Closed

Improve "Discriminator object" section of the OAS specification. #2216

Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
343b5e8
Improve OAS spec for discriminator
sebastien-rosset Apr 26, 2020
0e8a077
Improve OAS spec for discriminator
sebastien-rosset Apr 26, 2020
2efae8d
Improve OAS spec for discriminator
sebastien-rosset Apr 26, 2020
591bf43
Improve OAS spec for discriminator
sebastien-rosset Apr 26, 2020
9d4acf1
Improve OAS spec for discriminator
sebastien-rosset Apr 26, 2020
47dd48a
Improve OAS spec for discriminator
sebastien-rosset Apr 26, 2020
fd5c9e3
Improve OAS spec for discriminator
sebastien-rosset Apr 26, 2020
fa38c08
Improve OAS spec for discriminator
sebastien-rosset Apr 26, 2020
501b829
Improve OAS spec for discriminator
sebastien-rosset Apr 26, 2020
3edee8d
Change sentence to take Justin's feedback into consideration
sebastien-rosset May 1, 2020
172b357
Merge branch 'v3.1.0-dev' of https://github.com/OAI/OpenAPI-Specifica…
sebastien-rosset Aug 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions versions/3.1.0.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Contributor Author

@sebastien-rosset sebastien-rosset Apr 26, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

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.

There are two ways to define the value of a discriminator for an inheriting instance.
- Use the schema name.
- Override the schema name by overriding the property with a new value. If a new value exists, this takes precedence over the schema name.
Expand Down Expand Up @@ -2675,9 +2674,14 @@ components:

#### <a name="discriminatorObject"></a>Discriminator Object

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 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.
Copy link
Contributor Author

@sebastien-rosset sebastien-rosset Apr 26, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

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 discriminator field MUST be a required field in the schema object.

and:

The expectation now is that a property with name petType MUST be present in the response 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.
Copy link
Contributor Author

@sebastien-rosset sebastien-rosset Apr 26, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link

@spacether spacether May 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

The discriminator `propertyName` field MUST be a required field in the schema object. However, as shown below, the discriminator is not required to be present in the payload for _inline_ schemas.

The discriminator object is legal only when using one of the composite keywords `oneOf`, `anyOf`, `allOf`. The child schemas MAY be a mix of _inline_ or references schemas.
When a child schema is specified as a reference, the discriminator property _MUST_ be present in the payload.
When a child schema is specified _inline_, the discriminator property is not required in the payload. Validation is applied to determine if the payload matches the child JSON schema.

##### Fixed Fields
Field Name | Type | Description
Expand All @@ -2687,23 +2691,21 @@ 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`.

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:
Copy link
Contributor Author

@sebastien-rosset sebastien-rosset Apr 26, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Remove "In OAS 3.0" words:

  1. It is the 3.1 spec, not 3.0
  2. 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).

Copy link
Contributor Author

@sebastien-rosset sebastien-rosset Apr 26, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

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


```yaml
MyResponseType:
MyType:
oneOf:
- $ref: '#/components/schemas/Cat'
- $ref: '#/components/schemas/Dog'
- $ref: '#/components/schemas/Lizard'
```

which means the payload _MUST_, by validation, match exactly one of the schemas described by `Cat`, `Dog`, or `Lizard`. In this case, 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. We can then describe exactly which field tells us which schema to use:
which means the payload _MUST_, by validation, match exactly one of the schemas described by `Cat`, `Dog`, or `Lizard`. We can then describe exactly which field tells us which schema to use:


```yaml
MyResponseType:
MyType:
oneOf:
- $ref: '#/components/schemas/Cat'
- $ref: '#/components/schemas/Dog'
Expand All @@ -2712,7 +2714,7 @@ MyResponseType:
propertyName: petType
```

The expectation now is that a property with name `petType` _MUST_ be present in the response payload, and the value will correspond to the name of a schema defined in the OAS document. Thus the response payload:
The expectation now is that a property with name `petType` _MUST_ be present in the payload, and the value will correspond to the name of a schema defined in the OAS document. Thus the payload:

```json
{
Expand All @@ -2726,7 +2728,7 @@ Will indicate that the `Cat` schema be used in conjunction with this payload.
In scenarios where the value of the discriminator field does not match the schema name or implicit mapping is not possible, an optional `mapping` definition MAY be used:

```yaml
MyResponseType:
MyType:
oneOf:
- $ref: '#/components/schemas/Cat'
- $ref: '#/components/schemas/Dog'
Expand Down