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

Schema array type with anyOf and discriminator fails validation #402

Closed
rwrife opened this issue Apr 22, 2019 · 12 comments · Fixed by #951
Closed

Schema array type with anyOf and discriminator fails validation #402

rwrife opened this issue Apr 22, 2019 · 12 comments · Fixed by #951
Assignees
Labels
priority:p1 High priority but not blocking. Causes major but not critical loss of functionality SLA <=7days type:bug A broken experience
Milestone

Comments

@rwrife
Copy link
Member

rwrife commented Apr 22, 2019

I'm trying to validate an OpenAPI document that contains a schema object that is of type 'array' that can be an 'anyOf' that uses a discriminator to know which object goes into the array and it's failing validation. The document appears to be a valid OAS3 document and the Swagger UI has no issues rendering it, so I'm guessing it's an issue with OpenAPI.NET.

I'm getting validation errors similar to:

Composite Schema ChildTypeA must contain property specified in the discriminator type. [#/components/schemas/SampleResponse/properties/suggestedActions/items/anyOf]
Composite schema ChildTypeA must contain property specified in the discriminator type in the required field list. [#/components/schemas/SampleResponse/properties/suggestedActions/items/anyOf]

...for this document

openapi: 3.0.0
info:
  description: Service to test discriminator
  version: 1.0.0
  title: Test Service
servers:
  - url: 'https://localhost'
paths:
  /test:
    post:
      description: Sample post of SampleRequest
      requestBody:
        description: Returns the SampleResponse with multiple types
        required: true
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/SampleRequest'
      responses:
        '200':
          description: OK
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/SampleResponse'
components:
  schemas:
    SampleRequest:
      type: object
      required:
        - id
      properties:
        id:
          type: string
          description: Random ID
          example: '1513776099991'
    BaseType:
      type: object
      required:
        - type
      properties:
        type:
          type: string
          description: Suggested object type
    ChildTypeA:
      allOf:
        - $ref: '#/components/schemas/BaseType'
          properties:
            type:
              type: string
              enum:
                - typeA
      type: object
    ChildTypeB:
      allOf:
        - $ref: '#/components/schemas/BaseType'
          properties:
            type:
              type: string
              enum:
                - typeB
      type: object
    SampleResponse:
      type: object
      properties:
        suggestedActions:
          type: array
          items:
            anyOf:
              - $ref: '#/components/schemas/ChildTypeA'
              - $ref: '#/components/schemas/ChildTypeB'
            discriminator:
              propertyName: type
@PerthCharern
Copy link
Contributor

I think currently that rule only looks at the properties in the schema itself without delving into the properties in the anyOf/allOf. This is a bug in our validator.

@ma499
Copy link

ma499 commented Feb 27, 2020

Are there any plans to fix this bug? This validator is now being used by Azure API Management when deploying APIs, and we are no longer able to deploy updates to our existing API to production because of this problem.

@Robzilla
Copy link

Yes reported last November. Pretty big impact for me too. Apparently a fix is rolling out next week. See #429
Bit scary when something this big stops deployments for a few months.

@fabich
Copy link

fabich commented Apr 15, 2020

@PerthCharern any updates on this?

@darrelmiller darrelmiller added this to the Backlog milestone Apr 26, 2020
@ma499
Copy link

ma499 commented May 13, 2020

Is this actually a bug? I just read the relevant section of the latest Open API 3.0.3 specification which indicates that a discriminator field is actually required to be a 'required' field.

To support polymorphism, the OpenAPI Specification adds the discriminator field. When used, the discriminator will be the name of the property that decides which schema definition validates the structure of the model. As such, the discriminator field MUST be a required field. There are two ways to define the value of a discriminator for an inheriting instance.

This is not new, the same is specified for Open API 2.0.

Therefore, having read the specification, and thinking about the semantics of a discriminator field, I would argue that OpenAPI.NET's behaviour is actually correct and the official OpenAPI/Swagger parsers are the ones that actually have the bug as they are not strict enough to identify this problem.

I realise I hadn't understood the issue properly and this is indeed a bug in the parser that it is not traversing the anyOf link to look in the child schemas for the required discriminator field.

@darrelmiller darrelmiller modified the milestones: Backlog, 1.3 May 25, 2020
@himanigulati
Copy link

Is this issue still Open? We are dealing with the similar problem.

@amrith92
Copy link

@himanigulati Yes, I think this is still open.

@darrelmiller
Copy link
Member

Ok, I must be doing something wrong. I'm trying the above sample in the OpenAPIWorkbench tool from the master branch and I am not seeing the error...
image

@darrelmiller
Copy link
Member

I verified this again and I am not seeing any errors. Please re-open if you find this is still a problem.

@cnwta3
Copy link

cnwta3 commented Aug 18, 2021

I can still reproduce the error with the following reproducer specification.

openapi: 3.0.0
info:
  description: Service to test discriminator
  version: 1.0.0
  title: Test Service
servers:
  - url: 'https://localhost'
paths:
  /test:
    post:
      description: Sample post
      requestBody:
        required: true
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Parent'
      responses:
        '200':
          description: OK
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/Parent'
components:
  schemas:
    Parent:
      type: object
      oneOf:
         - $ref: '#/components/schemas/ChildA'
         - $ref: '#/components/schemas/ChildB'
      discriminator:
         propertyName: type
    ChildA:
      properties:
        type:
          type: string
      type: object
    ChildB:
      properties:
        type:
         type: string
      type: object

I get the following error:

"Schema Parent must contain property specified in the discriminator type in the required field list."

The specification validates correctly in Swagger Editor.

@darrelmiller darrelmiller modified the milestones: 1.3, 1.4 Mar 31, 2022
@darrelmiller darrelmiller reopened this Mar 31, 2022
@CarolKigoonya CarolKigoonya added the priority:p2 Medium. Generally has a work-around and a smaller sub-set of customers is affected. SLA <=30 days label Apr 26, 2022
@danehorak-callaway
Copy link

Can we get an update on the status of this fix? When can we expect a fix to be released?

@darrelmiller darrelmiller added priority:p1 High priority but not blocking. Causes major but not critical loss of functionality SLA <=7days and removed priority:p2 Medium. Generally has a work-around and a smaller sub-set of customers is affected. SLA <=30 days labels Jun 28, 2022
@baywet
Copy link
Member

baywet commented Jun 28, 2022

Hi everyone,
We just hit that issue recently with our internal tooling. After a quick chat with @darrelmiller here is the course of actions.

This code block needs improvements:

When the validation rule doesn't find the discriminator field in the current schema's required fields, it should recursively look into the anyOf/oneOf/allOf schemas (if any) before returning an error.

Ping @CarolKigoonya to bump the priority of this one given the current impact.

Additionally for implementers, #383 provides additional testing cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:p1 High priority but not blocking. Causes major but not critical loss of functionality SLA <=7days type:bug A broken experience
Projects
None yet
Development

Successfully merging a pull request may close this issue.