swagger-api / swagger.io-docs

The content of swagger.io
https://swagger.io/docs/
1.56k stars 397 forks source link

'Inheritance and Polymorphism' examples, discriminator usage #348

Open zorba128 opened 4 months ago

zorba128 commented 4 months ago

I'm actually trying to fix openapi/asyncapi generation for my project, and got stuck a bit looking at examples in the Inheritance and Polymorphism section.

First pls take a look at following simple example

  "$defs" : {
    "Apple" : {
      "title" : "Apple",
      "type" : "object",
      "required" : [
        "weight",
        "isSweet",
        "name"
      ],
      "properties" : {
        "weight" : {
          "type" : "number",
          "format" : "double"
        },
        "isSweet" : {
          "type" : "boolean"
        },
        "name" : {
          "type" : "string"
        }
      }
    },
    "Plum" : {
      "title" : "Plum",
      "type" : "object",
      "required" : [
        "weight",
        "name"
      ],
      "properties" : {
        "weight" : {
          "type" : "number",
          "format" : "double"
        },
        "name" : {
          "type" : "string"
        }
      }
    }
  },
  "title" : "Fruit",
  "oneOf" : [
    {
      "$ref" : "#/$defs/Apple"
    },
    {
      "$ref" : "#/$defs/Plum"
    }
  ],
  "discriminator" : {
    "propertyName" : "name",
    "mapping" : {
      "apple" : "#/$defs/Apple",
      "plum" : "#/$defs/Plum"
    }
  }
}

Now pls take a look at Apple schema.

Note this is not schema of Apple itself. This is schema of Apple in context of Fruit coproduct. It contains name property, that is needed only for apples stored next to other fruits. But note this schema alone is incomplete. All apples in the fruits basket should have {"name": "apple"} property set. Apple that has any other name is invalid. Shouldn't Apple schema not only define the name property, but also restrict it to one and only one possible value?

Note - if one does it like that - oneOf + const for name property is perfectly enough to represent Fruit, there is no need to use discriminant at all. And it would then make three representations of polymorphism possible in same way:

All those boil up to having specialized schemas that are incompatible with each other (what either happens naturally, or requires some additional work like additional nesting/discriminator), and oneOf on top of that. And specification is local - it is enough to know Apple schema to produce valid apple fruit entry.

Versus approach with discriminant only, and no const restriction, where one needs schemas both for Apple and its parent Fruit in order to know what to fill into the name property.

Now - if suggestion above is followed you get few nice features for free. Apart from Apple schema becoming standalone, self describing and producing valid Apple fruits, the whole discriminant becomes optional. If provided - it is just optimization hint, allowing quick lookup of target schema to parse/validate data against, so that one does not have to iterate over all the elements of oneOf enumeration.

This ticket is bit reworded discussion from issue reported for Tapir project.

So my suggestions are

marcin

zorba128 commented 4 months ago

Also note what would happen if specification would not only require presence of discriminator field in product schemas, but also require those to be constrained with const. In that case the whole part of specs related to discriminator mappings would become unnecessary. No need to specify mappings, no need to assume "schema name is used by default". Discriminator could just simply point at name of property, and validator/codec generator can actually discover actual mapping just looking at discriminator name, and traversing schemas listed in oneOf.

I think this would simplify specs alot.