OAI / OpenAPI-Specification

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

Discriminator field (unnecessarily) required #708

Closed dmejia closed 4 months ago

dmejia commented 8 years ago

Is it possible to remove the restriction of making the field that acts as discriminator required (and assume that when not returned, the object is just of the 'base' type defined in the schema)?

i.e (using the Pet example from the docs)

Schema:

(...)
properties: 
    pets: 
       type: array
       items:
           type: Pet

Payload:

{
   pets: [
   {
      name: "Some generic pet" 
   },
   {
      @type: Dog
      name: "Some dog"
      packSize: ...
   },
   {
      @type: Cat
      name: "Some cat"
      huntingSkill: ...
   },
   {
       name: "Some other generic pet"
   }
}

The first and third items are just of type "Pet" which would be explicit from the schema of the "pets" field. "Pet" would therefore define the @type field but it won't be required. We use schema.org and rely on polymorphic schemas extensively but we currently only return a type discriminator when is not obvious from the schema (i.e when returning a subtype). Unless I misunderstand it, adhering to the spec would require us to return @type everywhere which would significantly increase payload size (and thus would prob be a non-starter).

Let me know if you have any thoughts or reasons to not go this route.

ePaul commented 8 years ago

Related: #707.

dmejia commented 8 years ago

Any comments on this?

ralfhandl commented 8 years ago

I like this proposal, OData also omits the @odata.type field for instances of the base type.

webron commented 8 years ago

Tackling PR: #741

Relequestual commented 6 years ago

@webron #741 was closed in favour of merged #894. Does that mean that this is now resolved? Appears not as of current branch:v3.0.2-dev

Is this being tracked on a new PR?

As an aside, having the ability to shortcut any/all/one Of sounds like a nice thing! Maybe we should look at putting this into JSON Schema itself!?

handrews commented 6 years ago

@Relequestual discriminator does not fit well with other aspects of JSON Schema- I looked into it while we considered if/then/else and again during the whole unevaluatedProperties saga. I'd have to do some digging to explain exactly why.

Relequestual commented 6 years ago

No, that's fine. The same functionality can be provided using if/then/else.

tmtron commented 4 years ago

another use-case for an optional discriminator is API-evolution.

i.e. we may have an API that only supports one type (e.g. Dogs) and thus has no discriminator But later we need to add support for Cats

Then we can change the API in a backwards compatible way

handrews commented 4 months ago

@OAI/tsc review request: Do we want to consider this for 3.2+? We are already tracking discriminator replacement in Moonwalk so if we don't want to do this in 3.x we should close it.

lornajane commented 4 months ago

I'm in favour of closing. I think if we didn't do it in the first 8 years of this issue's lifetime, we should probably make 4.0 the host for changes in this area.

handrews commented 4 months ago

Two TSC votes to close, so I'm going to close this as that's our threshold for merging PRs.