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 composition breaks with additionalProperties: false #51

Open
tariq-tb opened this issue Jun 4, 2019 · 6 comments
Open

Schema composition breaks with additionalProperties: false #51

tariq-tb opened this issue Jun 4, 2019 · 6 comments

Comments

@tariq-tb
Copy link

tariq-tb commented Jun 4, 2019

Schema composition doesn't appear to work with additionalProperties: false.

openapi.yaml

openapi: 3.0.1
components:
  schemas:
    Pet:
      type: object
      properties: 
        name:
          type: string
        age:
          type: integer
      required: 
        - name
        - age
      additionalProperties: false
    Dog:
      allOf: 
        - $ref: '#/components/schemas/Pet'
        - type: object
          properties:
            bark:
              type: boolean
          required:
            - bark
      additionalProperties: false
      
paths:
  /simple:
    post:
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Pet'
      responses:
        '200':
          content:
            application/json:
              schema:
                type: object
  /complex:
    post:
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Dog'
      responses:
        '200':
          content:
            application/json:
              schema:
                type: object

Simple schema

  1. Hitting /simple with the following works as expected.
{
    "name": "hunter",
    "age": 12,
}
  1. Including an extra property foo in the request...
{
    "name": "hunter",
    "age": 12,
    "foo": true
}

...returns an error as expected because additionalProperties: false is set on the Pet schema.

{
    "error": {
        "name": "ValidationError",
        "message": "Error while validating request: request.body should NOT have additional properties",
        "data": [
            {
                "keyword": "additionalProperties",
                "dataPath": ".body",
                "schemaPath": "#/properties/body/additionalProperties",
                "params": {
                    "additionalProperty": "foo"
                },
                "message": "should NOT have additional properties"
            }
        ]
    }
}

Complex schema with composition

On the other hand, hitting /complex with the following...

{
    "name": "hunter",
    "age": 12,
    "bark": true
}

...returns an unexpected error.

{
    "error": {
        "name": "ValidationError",
        "message": "Error while validating request: request.body should NOT have additional properties",
        "data": [
            {
                "keyword": "additionalProperties",
                "dataPath": ".body",
                "schemaPath": "#/properties/body/additionalProperties",
                "params": {
                    "additionalProperty": "name"
                },
                "message": "should NOT have additional properties"
            }
        ]
    }
}
@keepersmith
Copy link

keepersmith commented Aug 26, 2019

I discovered this as well, and it is more of an issue with OAS than express-openapi-validate.
To get it to work, you need to put in stubs for properties of Pet, and use additionalProperties: false in the object explicity defined in Dog; like so:

Dog:
  allOf: 
    - $ref: '#/components/schemas/Pet'
    - type: object
      properties:
        bark:
          type: boolean
        name: {}
        age: {}
      required:
        - bark
        - name
        - age
      additionalProperties: false

@tariq-tb
Copy link
Author

tariq-tb commented Sep 3, 2019

I agree this might not be an issue with express-openapi-validate, just opening up the discussion here, happy to move it elsewhere. Tried your solution, but it did not resolve the issue.

Using your suggestions:

openapi: 3.0.1
components:
  schemas:
    Pet:
      type: object
      properties: 
        name:
          type: string
        age:
          type: integer
      required: 
        - name
        - age
      additionalProperties: false
    Dog:
      allOf: 
        - $ref: '#/components/schemas/Pet'
        - type: object
          properties:
            bark:
              type: boolean
            name: {}
            age: {}
          required:
            - bark
            - name
            - age
          additionalProperties: false
      # additionalProperties: false

...now results in:

{
    "error": {
        "name": "ValidationError",
        "message": "Error while validating request: request.body should NOT have additional properties",
        "data": [
            {
                "keyword": "additionalProperties",
                "dataPath": ".body",
                "schemaPath": "#/properties/body/allOf/0/additionalProperties",
                "params": {
                    "additionalProperty": "bark"
                },
                "message": "should NOT have additional properties"
            }
        ]
    }
}

@thetumper
Copy link

If you remove the additionalProperties in the Pet schema, it will no longer complain about bark. Specifying that in Pet conflicts with the set of properties in the Dog schema.

@tariq-tb
Copy link
Author

That's correct, but then /simple allows extra properties which isn't ideal.

@thetumper
Copy link

thetumper commented Nov 14, 2019

True. Workaround would be to have Pet remain "generic" -- only used as ref by other types, which specify additionalProperties: false, and are the ones actually used.

I.e., specify this:


PetConcrete:
      allOf: 
        - $ref: '#/components/schemas/Pet'
        - type: object
          properties:
            name: {}
            age: {}
          additionalProperties: false

and then use PetConcrete for the /simple request body type, not Pet.

@tariq-tb
Copy link
Author

tariq-tb commented Nov 14, 2019

Confirmed that your workaround along with @keepersmith's change does work, for better or worse. Updated openapi.yaml below.

components:
  schemas:
    Pet:
      type: object
      properties: 
        name:
          type: string
        age:
          type: integer
      required: 
        - name
        - age
      # additionalProperties: false
    Dog:
      allOf: 
        - $ref: '#/components/schemas/Pet'
        - type: object
          properties:
            bark:
              type: boolean
            name: {}
            age: {}
          required:
            - bark
          additionalProperties: false
    PetConcrete:
      allOf: 
        - $ref: '#/components/schemas/Pet'
        - type: object
          properties:
            name: {}
            age: {}
          additionalProperties: false
paths:
  /simple:
    post:
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/PetConcrete'
      responses:
        '200':
          content:
            application/json:
              schema:
                type: object
  /complex:
    post:
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Dog'
      responses:
        '200':
          content:
            application/json:
              schema:
                type: object

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

No branches or pull requests

3 participants