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

Incorrect example validation with oneOf and discriminator #1346

Closed
davidkvc opened this issue Sep 18, 2020 · 9 comments
Closed

Incorrect example validation with oneOf and discriminator #1346

davidkvc opened this issue Sep 18, 2020 · 9 comments
Labels
t/bug Something isn't working

Comments

@davidkvc
Copy link

Describe the bug
When example of object defined as oneOf with discriminator is validated the discriminator.propertyName is not used to validate the example against specific schema from the oneOf array.

To Reproduce

  1. Given this OpenAPI/AsyncAPI document
openapi: '3.0.2'
info:
  title: API Title
  version: '1.0'
servers:
  - url: https://api.server.test/v1
paths:
  /test:
    get:
      operationId: getTest

      responses:
        '200':
          content:
            application/json:
              schema:
                properties:
                  Items:
                    items:
                      $ref: '#/components/schemas/Item'
                    type: array
                required:
                  - Items
                type: object
              example:
                Items:
                  - Type: Standard
                    Name: test1
                    Id: 1
                  - Type: Extra
                    Name: test2
                    Id: 2
          description: Success

components:
  schemas:
    Item:
      discriminator:
        mapping:
          Standard: '#/components/schemas/Item/oneOf/1'
          Extra: '#/components/schemas/Item/oneOf/0'
        propertyName: Type
      oneOf:
        - properties:
            Name:
              type: string
            Type:
              type: string
            Id:
              type: number
          required:
            - Type
            - Id
            - Name
          type: object
        - properties:
            Name:
              type: string
            Type:
              type: string
            Id:
              type: number
            Extra:
              type: string
          required:
            - Type
            - Id
            - Name
            - Extra
          type: object
      required:
        - Type
  1. Run this CLI command spectral lint the_document.yml
  2. No error is produced even though the example response is not valid. Second element in the Items array is not valid according to schema definition for objects with Type=Extra

Expected behavior
I expect spectral to consider discriminator definition when validating schemas with oneOf definition

Environment:

  • Library version: 5.5.0

Additional context
This is even worse in situations when you meant to specify an example for the first schema in oneOf array but you get an error that your example is not valid according to the second schema in oneOf array. This happens because spectral apparently ignores discriminator definition and just validates against all of the schema definitions in oneOf array until it find one that is valid. Otherwise it generates error resulting from the validation of the last schema in the oneOf array. This leads to very confusing error messages. This is evident when we slightly edit the document:

openapi: '3.0.2'
info:
  title: API Title
  version: '1.0'
servers:
  - url: https://api.server.test/v1
paths:
  /test:
    get:
      operationId: getTest

      responses:
        '200':
          content:
            application/json:
              schema:
                properties:
                  Items:
                    items:
                      $ref: '#/components/schemas/Item'
                    type: array
                required:
                  - Items
                type: object
              example:
                Items:
                  - Type: Standard
                    Name: null
                    Id: 1
                  - Type: Extra
                    Name: test2
                    Id: 2
          description: Success

components:
  schemas:
    Item:
      discriminator:
        mapping:
          Standard: '#/components/schemas/Item/oneOf/1'
          Extra: '#/components/schemas/Item/oneOf/0'
        propertyName: Type
      oneOf:
        - properties:
            Name:
              type: string
            Type:
              type: string
            Id:
              type: number
          required:
            - Type
            - Id
            - Name
          type: object
        - properties:
            Name:
              type: string
              nullable: true
            Type:
              type: string
            Id:
              type: number
            Extra:
              type: string
          required:
            - Type
            - Id
            - Name
            - Extra
          type: object
      required:
        - Type

Linting this schema generates error: 27:21 error oas3-valid-oas-content-example `0` property should have required property `Extra` . As previously explained this error message is in my opinion wrong and can be very hard to decipher in complex schemas/examples. I would expect error message saying that Name property can't be null since the example should have been validated against the Standard schema only.

@davidkvc davidkvc added the t/bug Something isn't working label Sep 18, 2020
@philsturgeon
Copy link
Contributor

We don't support discriminator object, and your usage of it with oneOf is pretty wild. I don't think I've ever seen anything like that!

We've got an issue for tracking subpar error messages coming out of AJV where we try and catch specific use cases and turn them into something more useful, and the 0 property (first item in the array) is already in there, so follow #1072 for an update on when that error message is improved.

@davidkvc
Copy link
Author

Does that mean that there is no plan to support discriminator even in the future ?

@philsturgeon
Copy link
Contributor

It’s probably being deprecated so it’s not high on the priority list. There’s alternative ways of doing whatever you’re trying to do without using discriminator. OAI/OpenAPI-Specification#2143

@davidkvc
Copy link
Author

davidkvc commented Oct 2, 2020

I went through the discussion in the OpenAPI spec issue and it looks like it might be a while until discriminator is deprecated, if ever, because without it it is quite difficult to clearly express union types. It's unfortunate that you are not planning to support discriminator for schema validation. We use it for some schemas due to it's clear expression of intent and also because redoc uses discriminator to render the schema definition in much more user friendly way compared to if we just used oneOf + constant values.

@philsturgeon
Copy link
Contributor

philsturgeon commented Oct 5, 2020

Yeah I hear that. There's all sorts of complications with potentially implementing this feature, as at its core most of the JSON Schema / OpenAPI Schema validation is done with AJV, and AJV does not support discriminator.

ajv-validator/ajv#1119

We could put a bunch of time into supporting this feature, and requests do come up from time to time (3 times in 18 months f I remember rightly) but there are other bits of functionality more regularly requested, for things that aren't potentially getting chopped in the near/medium future.

Prioritization of features is tough, but whenever functionality is wanted in an open source project which the maintainers do not have the bandwidth to focus on, the alternative option is to send a pull request! Or you could write a custom function that handles it.

Best of luck with whichever option you chose! 🙌

@epoberezkin
Copy link

@philsturgeon do you think there is a value in implementing ajv-validator/ajv#1119? Quite a few upvotes there...

@philsturgeon
Copy link
Contributor

@epoberezkin absolutely. Now that OpenAPI Schema Objects are just another JSON Schema dialect, you could easily add support for OAS just like adding support for another draft of JSON Schema.

https://spec.openapis.org/oas/3.1/dialect/base

this is based on 2020-12 which AJV doesn’t have yet, but if you can add those two dialects, then you don’t have to worry about any awkward “OpenAPI feature in a JSON Schema validator” weirdness, you can just add support for the dialect an it’s enabled by the schema file referencing it in $schema.

@epoberezkin
Copy link

Thank you, I will add it. Will OpenAPI schema support JTD at some point then? Ajv supports it now.

@philsturgeon
Copy link
Contributor

Maybe in the future, OpenAPI has a proposal for alternativeSchemas which would allow XML Schema and maybe Protobuf and so that would be the most logical place for JTD to try and slow in if anyone is interested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants