Skip to content
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

Allow discriminator to be optional #4339

Open
wants to merge 7 commits into
base: v3.2-dev
Choose a base branch
from
Open

Allow discriminator to be optional #4339

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f1d5629
First draft of optional discriminator feature
mikekistler Feb 6, 2025
6c91a0b
Address comments from PR review
mikekistler Feb 9, 2025
9faf031
Apply suggestions from PR review
mikekistler Feb 11, 2025
7ebde14
Use defaultMapping keyword in discriminator
mikekistler Feb 11, 2025
94700fd
Prefer Discriminator Object terminology
mikekistler Feb 14, 2025
d36a8ff
More updates for PR review comments
mikekistler Feb 15, 2025
27216de
Add back polymorphic example using allOf
mikekistler Feb 15, 2025
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
18 changes: 9 additions & 9 deletions src/oas.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2877,7 +2877,7 @@ These keywords can be used to describe polymorphism, where a single field can ac

The OpenAPI specification extends the JSON Schema support for polymorphism by adding the [`discriminator`](#schema-discriminator) field.
When used, the `discriminator` indicates the name of the property that hints which schema of an `anyOf` or `oneOf` is expected to validate the structure of the model.
The `discriminator` property may be defined as required or optional, but when defined as an optional property the `discriminator` field must include a `default` field that specifies which schema of the `anyOf` or `oneOf` is expected to validate the structure of the model.
The discriminating property may be defined as required or optional, but when defined as an optional property the `discriminator` field must include a `default` field that specifies which schema of the `anyOf` or `oneOf` is expected to validate the structure of the model.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The discriminating property may be defined as required or optional, but when defined as an optional property the `discriminator` field must include a `default` field that specifies which schema of the `anyOf` or `oneOf` is expected to validate the structure of the model.
The discriminating property may be defined as required or optional, but when defined as an optional property the `discriminator` schema must include a `default` field that specifies which schema of the `anyOf` or `oneOf` is expected to validate the structure of the model.


There are two ways to define the value of a discriminator for an inheriting instance.

Expand Down Expand Up @@ -3372,9 +3372,9 @@ Note that `discriminator` MUST NOT change the validation outcome of the schema.

| Field Name | Type | Description |
| ---- | :----: | ---- |
| <a name="property-name"></a>propertyName | `string` | **REQUIRED**. The name of the property in the payload that will hold the discriminating value. This property may be defined as required or optional, but when defined as an optional property the `discriminator` field must include a `default` field that specifies which schema is expected to validate the structure of the model. |
| <a name="property-name"></a>propertyName | `string` | **REQUIRED**. The name of the discriminating property in the payload that will hold the discriminating value. The discriminating property may be defined as required or optional, but when defined as optional the `discriminator` field must include a `default` field that specifies which schema is expected to validate the structure of the model. |
| <a name="discriminator-mapping"></a> mapping | Map[`string`, `string`] | An object to hold mappings between payload values and schema names or URI references. |
| <a name="default"></a> default | `string` | The schema name or URI reference to a schema that is expected to validate the structure of the model when the `discriminator` property is not present in the payload. |
| <a name="default"></a> default | `string` | The schema name or URI reference to a schema that is expected to validate the structure of the model when the discriminating property is not present in the payload. |

This object MAY be extended with [Specification Extensions](#specification-extensions).

Expand Down Expand Up @@ -3402,13 +3402,13 @@ To ensure that an ambiguous value (e.g. `"foo"`) is treated as a relative URI re
Mapping keys MUST be string values, but tooling MAY convert response values to strings for comparison.
However, the exact nature of such conversions are implementation-defined.

##### Optional `discriminator` property
##### Optional discriminating property

When the `discriminator` property is defined as optional, the `discriminator` field must include a `default` field that specifies a schema that is expected to validate the structure of the model when the `discriminator` property is not present in the payload. This allows the schema to still be validated correctly even if the discriminator property is missing.
When the discriminating property is defined as optional, the `discriminator` field must include a `default` field that specifies a schema that is expected to validate the structure of the model when the discriminating property is not present in the payload. This allows the schema to still be validated correctly even if the discriminator property is missing.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
When the discriminating property is defined as optional, the `discriminator` field must include a `default` field that specifies a schema that is expected to validate the structure of the model when the discriminating property is not present in the payload. This allows the schema to still be validated correctly even if the discriminator property is missing.
When the discriminating property is defined as optional, the `discriminator` schema must include a `default` field that specifies a schema that is expected to validate the structure of the model when the discriminating property is not present in the payload. This allows the schema to still be validated correctly even if the discriminator property is missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think changing "field" to "schema" is right here. Can you explain why you have suggesting this change?

Copy link
Contributor

Choose a reason for hiding this comment

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

the discriminator keyword defines a schema. It's not a standalone field. It requires the propertyName keyword per the spec.

discriminator:
  propertyName: thing
  default: Dog # < your new keyword

vs

discriminator: ??

Copy link
Contributor

Choose a reason for hiding this comment

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

there were a few locations where this wasn't updated. They are in the Outdated conversations below. Is that an oversight or intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can change discriminator field to discriminator keyword. Would that address your concerns?

Copy link
Contributor

Choose a reason for hiding this comment

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

the reason I raised this is because the discriminator field is not actually a thing. it's a schema. You can't use the field by itself, as shown in my example. Referring to it as a field is not correct, imo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jeremyfiel I don't know what you mean when you say "the discriminator field is not actually a thing". discriminator is a "fixed field" defined by OpenAPI, so by our established terminology, we refer to it as a "field".

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand the field is described by OAS, but the field itself is a schema. It's never a standalone field such as additionalProperties: false or additionalProperties: {} where it can be both.

In this reference language, I believe the discriminator field should be referenced as a schema because you are referencing the behavior of the field. The use of field is correct when referencing the defaultMapping requirement.

When the discriminating property is defined as optional, the discriminator schema must include a defaultMapping field that specifies a 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.

Okay ... I think I see a way to resolve this. What we are talking about here is the "Discriminator Object" -- the value of the discriminator field. So we could use "Discriminator Object" rather than "discriminator field`" in the places you have flagged. I think this would be consistent with language/terminology elsewhere in the spec. Would this work for you @jeremyfiel ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good! 👍


The primary use case for an optional `discriminator` property is to allow a schema to be extended with a discriminator without breaking existing clients that do not provide the discriminator property.
The primary use case for an optional discriminating property is to allow a schema to be extended with a discriminator without breaking existing clients that do not provide the discriminator property.
mikekistler marked this conversation as resolved.
Show resolved Hide resolved

Typically the schema specified in the `default` field will specify that the `discriminator` property is not present, e.g.
Typically the schema specified in the `default` field will specify that the discriminating property is not present, e.g.

```yaml
OtherPet:
Expand All @@ -3417,7 +3417,7 @@ OtherPet:
required: ['petType']
```

This will prevent the default schema from validating a payload that includes the `discriminator` property, which would cause a validation of the payload to fail when polymorphism is described using the `oneOf` JSON schema keyword.
This will prevent the default schema from validating a payload that includes the discriminating property, which would cause a validation of the payload to fail when polymorphism is described using the `oneOf` JSON schema keyword.

##### Examples

Expand Down Expand Up @@ -3476,7 +3476,7 @@ Here the discriminating value of `dog` will map to the schema `#/components/sche

When used in conjunction with the `anyOf` construct, the use of the discriminator can avoid ambiguity for serializers/deserializers where multiple schemas may satisfy a single payload.

When the `discriminator` property is defined as optional, the `discriminator` field must include a `default` field that specifies a schema of the `anyOf` or `oneOf` is expected to validate the structure of the model when the `discriminator` property is not present in the payload. This allows the schema to still be validated correctly even if the discriminator property is missing.
When the discriminating property is defined as optional, the `discriminator` field must include a `default` field that specifies a schema of the `anyOf` or `oneOf` is expected to validate the structure of the model when the discriminating property is not present in the payload. This allows the schema to still be validated correctly even if the discriminator property is missing.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
When the discriminating property is defined as optional, the `discriminator` field must include a `default` field that specifies a schema of the `anyOf` or `oneOf` is expected to validate the structure of the model when the discriminating property is not present in the payload. This allows the schema to still be validated correctly even if the discriminator property is missing.
When the discriminating property is defined as optional, the `discriminator` schema must include a `default` field that specifies a schema of the `anyOf` or `oneOf` is expected to validate the structure of the model when the discriminating property is not present in the payload. This allows the schema to still be validated correctly even if the discriminator property is missing.


For example:

Expand Down