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.

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

OpenAPI Discriminator support #31

Open
slinkydeveloper opened this issue Dec 18, 2020 · 5 comments
Open

OpenAPI Discriminator support #31

slinkydeveloper opened this issue Dec 18, 2020 · 5 comments
Labels
enhancement New feature or request

Comments

@slinkydeveloper
Copy link
Member

This issue is intended to collect thoughts about an eventual discriminator support. https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.3.md#schemaObject

@slinkydeveloper
Copy link
Member Author

There is an open PR #18 and several issues on vertx-web repo, like vert-x3/vertx-web#1820 (some from the old vertx-web-api-contract).

@slinkydeveloper
Copy link
Member Author

So discriminator is a though keyword to handle, and the spec is not really clear on how this should be implemented.

Optional keyword

First and foremost, the discriminator keyword is optional:

In OAS 3.0, a response payload MAY be described to be exactly one of any number of types which means the payload MUST, by validation, match exactly one of the schemas described by Cat, Dog, or Lizard. In this case, a discriminator MAY act as a "hint" to shortcut validation and selection of the matching schema which may be a costly operation, depending on the complexity of the schema

From this spec sentence, my understanding is that, when using discriminator together with oneOf, you must go through all the oneOf schemes to strictly check if the input matches only one of the schemes or another (which makes vert-x3/vertx-web#1820 example technically invalid anyway, because even if there is a discriminator keyword, both schemes are valid).
While using anyOf together with discriminator, it might be useful to use discriminator to speedup a bit the validation: the validator might first check the schema that matches the discriminator and then it checks the other schemes. But it stills seems optional looking at the spec:

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

Schema name

Now, the real issue with discriminator is that, when using it with anyOf or oneOf, it's not clear how to infer the value to match with the property type. In the paragraph https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.3.md#composition-and-inheritance-polymorphism it states to use the schema name, although the definition of schema name is not really clear, because then it shows these examples:

MyResponseType:
  oneOf:
  - $ref: '#/components/schemas/Cat'
  - $ref: '#/components/schemas/Dog'
  - $ref: '#/components/schemas/Lizard'
  discriminator:
    propertyName: petType
components:
  schemas:
    Pet:
      type: object
      required:
      - petType
      properties:
        petType:
          type: string
      discriminator:
        propertyName: petType
        mapping:
          dog: Dog
    Cat:
      allOf:
      - $ref: '#/components/schemas/Pet'
      - type: object
        # all other properties specific to a `Cat`
        properties:
          name:
            type: string
    Dog:
      allOf:
      - $ref: '#/components/schemas/Pet'
      - type: object
        # all other properties specific to a `Dog`
        properties:
          bark:
            type: string
    Lizard:
      allOf:
      - $ref: '#/components/schemas/Pet'
      - type: object
        # all other properties specific to a `Lizard`
        properties:
          lovesRocks:
            type: boolean

And also there is a keyword in json schema called name that might fit that purpose, and the keyword id too.... So I frankly have no clue how that should be inferred. The second case in particular relies on the fact that the schema is defined in the context of components/schemes, which is very specific to openapi 3 (so it requires some "preprocessing" of the schema in vertx-web-openapi).

Inside allOf

Now, there is a difference in the spec on the context where it uses the discriminator. For oneOf and anyOf the discriminator is used outside the "validating" schemas context, while for allOf the keyword is used in the "base schema" context (the example is above):

To avoid redundancy, the discriminator MAY be added to a parent schema definition, and all schemas comprising the parent schema in an allOf construct may be used as an alternate schema.

First, there is no such "parent schema" in Json Schema, allOf is just an and of different schema validation results. So if for some reason a schema is composed by 3 subschemes connected with allOf and 2 of them has the discriminator, what the validator should do is unclear.
But even in the simplest case, it's not clear how discriminator helps the validator in allOf.

Let's assume we need to validate a payload on this schema:

MyResponseType:
  oneOf:
  - $ref: '#/components/schemas/Cat'
  - $ref: '#/components/schemas/Dog'
  - $ref: '#/components/schemas/Lizard'

Where Cat, Dog and Lizard are defined using the example above with allOf and discriminator in Pet. The fact that there is a discriminator doesn't make any difference to the validator, because the schemas are independent by design, so MyResponseType won't have any knowledge that there is a discriminator deep nested in oneOf -> Cat -> allOf -> Pet.

That's why I think this particular use case of allOf + discriminator is practically unimplementable in this json validator (and most other validators outside there).

Corner cases

  • What's the "name" of the last schema here, assuming it doesn't contain name, id or whatever other "schema name" value? schema.json?
MyResponseType:
  oneOf:
  - $ref: '#/components/schemas/Cat'
  - $ref: '#/components/schemas/Dog'
  - $ref: '#/components/schemas/Lizard'
  - $ref: 'https://gigantic-server.com/schemas/Monster/schema.json'
  discriminator:
    propertyName: petType
  • When you define directly the schemes in the oneOf array, without using $refs and without name and $id, how do I handle discriminator?

  • How do I assign mappings when there is the mapping keyword but is incomplete?

MyResponseType:
  oneOf:
  - $ref: '#/components/schemas/Cat'
  - $ref: '#/components/schemas/Dog'
  - $ref: '#/components/schemas/Lizard'
  - $ref: 'https://gigantic-server.com/schemas/Monster/schema.json'
  discriminator:
    propertyName: petType
    mapping:
      dog: '#/components/schemas/Dog'
      monster: 'https://gigantic-server.com/schemas/Monster/schema.json'

Can we do something about it?

Nevertheless, I see one use case trivially implementable here, although I'm not sure it really makes sense to implement it: When using the anyOf keyword containing only $ref and when the discriminator has explicit mappings for all the schemes, then we can easily use discriminator to speedup the validation:

MyResponseType:
  anyOf:
  - $ref: '#/components/schemas/Cat'
  - $ref: '#/components/schemas/Dog'
  - $ref: '#/components/schemas/Lizard'
  - $ref: 'https://gigantic-server.com/schemas/Monster/schema.json'
  discriminator:
    propertyName: petType
    mapping:
      dog: '#/components/schemas/Dog'
      cat: '#/components/schemas/Dog'
      lizard: '#/components/schemas/Lizard'
      monster: 'https://gigantic-server.com/schemas/Monster/schema.json'

Although, I personally think just supporting this use case is not worth it.

@JonasTaulien
Copy link

JonasTaulien commented Mar 8, 2021

Hello, we really would like to use discriminator in our Vert.x application for validating the integrity of requests. We are using Vert.x Web OpenApi and would love the integration of discriminator together with mapping :)

Our use case:

components:
  schemas:
    Spec:
      type: object
      properties:
        specType:
          type: string
          enum:
            - "SPEC_A"
            - "SPEC_B"
      required:
        - specType
      discriminator:
        propertyName: specType
        mapping:
          SPEC_A: '#/components/schemas/SpecA'
          SPEC_B: '#/components/schemas/SpecB'

    SpecA:
      allOf:
        - $ref: '#/components/schemas/Spec'
        - type: object
          properties:
            someAdditionalProperty:
              type: string
          required:
            - someAdditionalProperty
    SpecB:
      allOf:
        - $ref: '#/components/schemas/Spec'
        - type: object
          properties:
            someOtherAdditionalProperty:
              type: integer
              minimum: 0
          required:
            - someOtherAdditionalProperty

So that the following bodies are considered valid:

  • {
        "specType": "SPEC_A",
        "someAdditionalProperty": "abc"
    }
  • {
        "specType": "SPEC_B",
        "someOtherAdditionalProperty": 99
    }

But these are considered invalid:

  • {
        "specType": "SPEC_A",
        "someOtherAdditionalProperty": 100
    }
  • {
        "specType": "SPEC_B"
    }
  • {
        "specType": "SPEC_B",
        "someOtherAdditionalProperty": -700
    }

@slinkydeveloper
Copy link
Member Author

slinkydeveloper commented Mar 16, 2021

You can easily achieve the same result without discriminator, just adding specType to both SpecA and SpecB:

components:
  schemas:
    Spec:
      type: object
      properties:
        # Some common properties
      required:
        - specType

    SpecA:
      allOf:
        - $ref: '#/components/schemas/Spec'
        - type: object
          properties:
            specType:
              const: "SPEC_A"
            someAdditionalProperty:
              type: string
          required:
            - someAdditionalProperty
    SpecB:
      allOf:
        - $ref: '#/components/schemas/Spec'
        - type: object
          properties:
            specType:
              const: "SPEC_B"
            someOtherAdditionalProperty:
              type: integer
              minimum: 0
          required:
            - someOtherAdditionalProperty

You can keep the discriminator, definition if you want to, because it will be just ignored by the schema parser.

The case you proposed is exactly one of those really hard situations we can't easily implement #31 (comment)

@cbornet
Copy link

cbornet commented Sep 16, 2022

One of the use case I see of discriminator+oneOf is when reporting validation errors to the user. If none of the schema match, you probably want to report only why the discriminator-specified schema didn't match.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants