OAI / OpenAPI-Specification

The OpenAPI Specification Repository
https://openapis.org
Apache License 2.0
28.91k stars 9.07k forks source link

Clarification on `discriminator` usage on `allOf` schemas. #3424

Closed iglosiggio closed 4 months ago

iglosiggio commented 11 months ago

Hi, the spec says:

The discriminator object is legal only when using one of the composite keywords oneOf, anyOf, allOf.

What's the meaning behind something like this?

components:
  schemas:
    Cat:
      allOf:
      - $ref: '#/components/schemas/Pet'
      - $ref: '#/components/schemas/Furry'
      - $ref: '#/components/schemas/Cute'
      discriminator:
        propertyName: "petType"
    # ... descriptions of other schemas

I don't see an obvious intepretation for an schema that has both discriminator and allOf.

karenetheridge commented 11 months ago

allOf (or anyOf or oneOf) are expected, but they don't have to be at the same level -- there is one example in the spec at the same level and one where they are in separate definitions. The schema that requires them together is a mistake - see https://github.com/OAI/OpenAPI-Specification/pull/3405.

handrews commented 11 months ago

I think the oneOf or anyOf need to be in the same schema object as discriminator, but the allOf use case is different and is always in a different schema object. I think.

LasneF commented 11 months ago

is there a use case of having discriminator and allof ?

to me discriminator should only be use for the oneOf the usage of it inside an anyOf beeing quite alredy limited/corner case

discriminator could even be dropped pushing that schema with a oneOf must be completly exclusive by nature (ie a content cannot match 2 schema)

iglosiggio commented 11 months ago

discriminator could even be dropped pushing that schema with a oneOf must be completly exclusive by nature (ie a content cannot match 2 schema)

That's already what the specification says. Having a discriminator for oneOf doesn't help during validation because all variants have to be validated against the payload (exactly one should match).

iglosiggio commented 11 months ago

The schema that requires them together is a mistake - see #3405.

Sorry but what does "The discriminator object is legal only when using one of the composite keywords" means then?

Edit: Reading the linked PR I now understand that this part of the spec is being reworked 😅

LasneF commented 9 months ago

@handrews , @darrelmiller what about for OAS 4.0 or even for OAS 3.1 just dropping / deprecating discriminator at all , this is munch more a Json schema topic around schema composition building

pushing all schema definition to json schema looks a better split of concern , all about schema is delegated to a certain version of Json schema , vs OAS spec is about API

handrews commented 9 months ago

@LasneF OAS 4.0 a.k.a. Moonwalk is definitely separating concerns more thoroughly, including better separation between content schema formats and OAS "proper". There aren't any specifics on what that means yet, but it will be addressed in the coming year (note that there is a separate OAI/Moonwalk repository for that project).

Doing any such thing would be too big of a change for a 3.1.x, or even a 3.2, although of course in 3.1 JSON Schema is at least theoretically extensible with 3rd-party keywords.

handrews commented 4 months ago

PRs merged! closing.