Validate discriminator values

[seveneves]

Tapir version: 1.10.12

Scala version: 3.3.3

Given following model

implicit val cfg: Configuration = Configuration.default.withDiscriminator("pet")

sealed trait Pet derives tapir.Schema

case class Cat(
  name: String,
) extends Pet

case class Dog(
  name: String,
  bites: Boolean,
) extends Pet

case class Rat(
  name: String
) extends Pet

It will have the following OpenApi specification generated

openapi: 3.1.0
info:
  title: Pet
  version: Pet
paths: []
components:
  schemas:
    Cat:
      title: Cat
      type: object
      required:
      - name
      - pet
      properties:
        name:
          type: string
        pet:
          type: string
    Dog:
      title: Dog
      type: object
      required:
      - name
      - bites
      - pet
      properties:
        name:
          type: string
        bites:
          type: boolean
        pet:
          type: string
    Pet:
      title: Pet
      oneOf:
      - $ref: '#/components/schemas/Cat'
      - $ref: '#/components/schemas/Dog'
      - $ref: '#/components/schemas/Rat'
      discriminator:
        propertyName: pet
        mapping:
          Cat: '#/components/schemas/Cat'
          Dog: '#/components/schemas/Dog'
          Rat: '#/components/schemas/Rat'
    Rat:
      title: Rat
      type: object
      required:
      - name
      - pet
      properties:
        name:
          type: string
        pet:
          type: string

Schemas for Cat and Rat are essentially the same and only can be distinguished by discriminator value pet. However, it seems that OpenAPI specification does not require discriminator to actually be enforced when validating oneOf type of Schema definition.

This is answered in the discussion of OpenAPI specification https://github.com/OAI/OpenAPI-Specification/discussions/3608 where the suggestion is to use anyOf instead of oneOf to make validation work (a library I use does fail the validation of oneOf because Rat and Cat are the similar schemas structurally).

One easy fix for a such problem is to add a validator of type enum to each discriminator field pet. So then the definition would become as follows. Such validation will make sure that type Cat and Rat are not subsets of each other.

    Cat:
      title: Cat
      type: object
      required:
      - name
      - pet
      properties:
        name:
          type: string
        pet:
          type: string
          enum: [ Cat ]

I would like to make a suggestion to extend sttp.tapir.generic.Configuration by introducing a new flag as validateDiscriminatorValue: Boolean = false and then add enum validator in method sttp.tapir.SchemaType.SCoproduct#addDiscriminatorField.

I can submit a change request if this is accepted

[zorba128]

Please take a look at related issue I raised some time ago: https://github.com/softwaremill/tapir/issues/3765 https://github.com/swagger-api/swagger.io/issues/348

[adamw]

It seems there's a couple of variants, in which we can render discriminators & mappings. From what I understand these are:

1. current "simple" mapping using oneOf + discriminator, plain types
2. current mapping + each variant with an enum validator on the discriminator field
3. keeping the current mapping, except for using anyOf instead of oneOf,
4. using const schema attributes for the discriminator fields

It would be great to be able to use just a single representation, or at least constraint the possibilities to a set of "best ones". Though - which would those "best ones" be?

(btw. - I think discriminators are useful if only for the fact that code generators might use them to create deserialization code efficiently)

[zorba128]

Const is just more explicit representation of what single-value enum does.

See https://github.com/swagger-api/swagger.io/issues/348#issuecomment-2111886039

I actually believe having discriminator property named at the generic level, and having const constraint at leafs is enough to derive all the representations.

Maybe reasoning like that:

• to me it seems polymorphism we're trying to model naturally suggests oneOf; its either Cat or Dog or Rat, not mix of above, not "pick first to match"
• to make them distinguishable - sometimes it happens naturally (when using different attributes), but sometimes we need hint
• adding special field to tell the type to use is common practise, usually it is added even in places where it is not really needed
• just having this special field added and constrained to specific value to each product's schema is enough to make everything work with oneOf
• additionally having this field pointed at generic level enforces rigid structure, allowing to make quick lookups, generate mappings table, optimize parsers (by reading this field first)
• having it modeled this way we can derive any representation needed by specific standard; like current problem of generating invalid async api, as it does not allow to specify mapping table at all

In the end - for me - best final representation should match model described above - just oneOf+const, and named and required discriminator field. I have no idea why the whole discriminator mapping idea was added to openapi - if one can have exactly same functionality and performance with just one additional attribute attached to discriminator field definition. See how nicely this all scales:

1. oneOf, property with const value at each product schema
2. as above, additionally this common property defined and marked as required at generic coproduct level
3. as above, with discriminator property additionally marked as discriminator

First is enough to parse/validate correctly. Second is more strict about structure. Third allows to build mapping tables and optimize.

[seveneves]

In our use case, we use tapir for the backend and generate TypeScript clients based on the OpenApi Specification generated out of Tapir schema. So having "one-to-one" strictness of type definitions in Scala with what TypeScript is allowed to do would be the "best one" solution to me.

And I think this can be only achieved with either enum validation or constant discriminator field.

[adamw]

ok, thank you very much for the detailed explanations. Let's go with the const validator, as I think it's the most precise. Now we just need an implementation :)

[seveneves]

Now we just need an implementation :)

@adamw looking briefly into the support of const in tapir Schema, it is not supported at all.

Could you please leave some pointers of how to add support for const. And what to be aware of when adding a new feature like const support? I assume new implementation of SchemaType will be required that just wraps a primitive value with a type definition.

[zorba128]

Please see here #3765

We implemented this with additional Schema.const: Option[(T, Option[Any])] property, but did not manage to get binary compatibilty, so dropped this idea for some pre/postprocessing trickery that allowed us to get desired effect in production without changing tapir.

It would be nice to simply have Schema.const attribute supported.

[kamilkloch]

@seveneves @adamw I reopened our take on const: https://github.com/softwaremill/tapir/pull/3763.