cdimascio / express-openapi-validator

🦋 Auto-validates api requests, responses, and securities using ExpressJS and an OpenAPI 3.x specification
MIT License
919 stars 210 forks source link

Option removeAdditional with allOf #696

Open YaakovR opened 2 years ago

YaakovR commented 2 years ago

I refrained from labeling this a Bug report or Feature request, since it may be my own syntactic error 😄.

OpenApiValidator config:

app.use(OpenApiValidator.middleware({
  apiSpec: openapiSpec,
  validateRequests: false,
  validateResponses: {
    removeAdditional: 'all',
    coerceTypes: true,
  },
  validateApiSpec: true,
  validateSecurity: false,
}));

OpenAPI path:

get:
  responses:
    '200':
      content:
        application/json:
          schema:
            type: object
            properties:
              data:
                type: array
                items:
                  allOf:
                  - "$ref": "#/components/schemas/User"
                  - type: object
                    properties:
                      user_group:
                        "$ref": "#/components/schemas/UserGroup"

OpenAPI schema

User:
  type: object
  properties:
    id:
      type: integer
      readOnly: true
    name:
      type: string

UserGroup:
  type: object
  properties:
    id:
      type: integer
      readOnly: true
    name:
      type: string

Expected JSON return data

{
  "data": [
    {
        "id": 6,
        "name": "Abraham",
        "user_group": {
          "id": 2,
          "name": "Castle"
        }
    },
    {
        "id": 4,
        "name": "Isaac",
        "user_group": {
          "id": 2,
          "name": "Castle"
        }
    }
  ]
}

Actual JSON return data

{
    "data": [
        {},
        {}
    ]
}

I'm wondering why removeAdditional: 'all' empties the entire objects. Everything works fine when removing the removeAdditional: 'all'. In addition, all is good when writing out the schema instead of referencing. Does express-openapi-validator not support allOf?

The schema seems to be valid since the validateApiSpec: true passes. Also, I'm using swagger-ui-express, and it is giving no issues interpreting the schemas inside of the allOf.

I'd appreciate if someone can look at this and advise if there is something I should be doing differently or if this is an issue with swagger-ui-express.

Thank you very much!!

tmyl123 commented 2 years ago

Hi, meeting same things here, using the following spec with removeAdditional: 'all'

paths:
  /pets:
    post:
      description: Creates a new pet in the store.
      operationId: addPet
      requestBody:
        description: Pet to add to the store
        required: true
        content:
          application/json:
            schema:
              allOf:
                - $ref: '#/components/schemas/Pet'
                - $ref: '#/components/schemas/Pet_Extend'
      responses:
        '200':
          description: pet response
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/Pet'

components:
  schemas:
    Pet:
      required:
        - id
        - name
        - type
      properties:
        id:
          readOnly: true
          type: number
        name:
          type: string
        tag:
          type: string
        type:
          type: string
          enum:
            - dog
            - cat

    Pet_Extend:
      properties:
        age:
          type: number

every else fields has been removed except the id field which has been add with the example backend server.

curl -s -XPOST -H 'Content-Type: Application/json' -H 'X-API-Key: 234' localhost:3000/v1/pets -d '{"name": "A", "type": "dog", "age": 3, "foo": "bar"}'| jq .
{
  "id": 4
}

my guess is when calling removeAdditional, it's behavior is checking schemas one by one, not merging them before checking.

so in your snippet, property user_group has been treated as additional for schema User; while id and name has been treated as additional for schema UserGroup, that's why result in empty object.

yet this is my guessing though, not sure if it's expected behavior here, wait for further suggestion :sweat_smile:

Dan1224444 commented 2 years ago

I also believe this to be a bug, not a feature, or due to your own syntactic error according to https://swagger.io/docs/specification/data-models/oneof-anyof-allof-not/#allof

weakit commented 1 year ago

Facing the same issue, can confirm that it doesn't work with an allOf

even otherwise, the whole feature doesn't behave very nicely

philbegg commented 7 months ago

agreed facing the same issue with allOf