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

Aggregation of allOf combined with additionalProperties false not working - Response validation #239

Open
luboskriz opened this issue Feb 21, 2020 · 8 comments

Comments

@luboskriz
Copy link

I am getting the following error when validating response for a /route endpoint (see schema and the response example below)

Error: .response should NOT have additional properties, .response should NOT have additional properties
    at ResponseValidator._validate (C:\Code\automation\node_modules\express-openapi-validator\dist\middlewares\openapi.response.validator.js:83:32)
    at C:\Code\automation\node_modules\express-openapi-validator\dist\middlewares\openapi.response.validator.js:27:29
    at ServerResponse.json_hook (C:\Code\automation\node_modules\express-openapi-validator\dist\framework\modded.express.mung.js:35:24)
    at ServerResponse.send (C:\Code\automation\node_modules\express\lib\response.js:158:21)
    at c:\Code\automation\src\dataserver\Server.ts:268:25
    at Layer.handle [as handle_request] (C:\Code\automation\node_modules\express\lib\router\layer.js:95:5)
    at next (C:\Code\automation\node_modules\express\lib\router\route.js:137:13)
    at Route.dispatch (C:\Code\automation\node_modules\express\lib\router\route.js:112:3)

Expected result: The validation should pass

Example 1 - Demonstrates the issue

Having the following schema, the below response causes validation error - no additional properties
Schema:

---
  openapi: "3.0.0"
  info:
    title: "Test for allOf"
    version: "1"
  paths:
    /route:
      get:
        responses:
          200:
            description: ""
            content:
              application/json:
                schema:
                  $ref: "#/components/schemas/RouteType"
  components:
    schemas:
      RouteType:
        allOf:
          -
            $ref: "#/components/schemas/Type1"
          -
            $ref: "#/components/schemas/Type2"
      Type1:
        type: "object"
        additionalProperties: false
        properties:
          property1:
            type: "integer"
      Type2:
        type: "object"
        additionalProperties: false
        properties:
          property2:
            type: "string"

Response:

---
  property1: 1
  property2: SomeString

Example 2 - No issue

If there is just one item in allOf collection, it validates correctly:

Schema:

---
  openapi: "3.0.0"
  info:
    title: "Test for allOf"
    version: "1"
  paths:
    /route:
      get:
        responses:
          200:
            description: ""
            content:
              application/json:
                schema:
                  $ref: "#/components/schemas/RouteType"
  components:
    schemas:
      RouteType:
        allOf:
          -
            $ref: "#/components/schemas/Type1"
      Type1:
        type: "object"
        additionalProperties: false
        properties:
          property1:
            type: "integer"

Response:

---
  property1: 1
@cdimascio cdimascio added the bug Something isn't working label Feb 24, 2020
@cdimascio
Copy link
Owner

cdimascio commented Feb 24, 2020

@luboskriz thanks for the issue. i'll have a look.

@luboskriz
Copy link
Author

Hi @cdimascio I am happy that you will look at the issue!

I am working on a larger rapidly growing project and this is a no go blocker for us. I tried couple workarounds, but none of them works for me completely.

I would need to have this fixed as soon as possible, so if it is not a priority for you and you could give me some hints, I would like to help fixing it, even though I am not definitely such expert as you are. I have already tried, but the code took me to the ajv internals and this was too much for me.

Thanks for any help! I appreciate your work, I really like the library overall.

@cdimascio
Copy link
Owner

cdimascio commented Feb 29, 2020

@luboskriz this issue occurs for both request and response validation. it's an issue in AJV itself. Though arguably, its how JSON Schema works - See the thread here ajv-validator/ajv#837

All in all, a workaround is to remove additonalProperties: false or follow Evgeny's suggestion. That is:

There are many ways it can be achieved, in all cases you would have to repeat all allowed property names on the top level. This question is not ajv specific, it is related to general JSON schema specification usage.

@cdimascio cdimascio removed the bug Something isn't working label Feb 29, 2020
@cdimascio cdimascio changed the title Aggregation of allOf not supported - Response validation Aggregation of allOf combined with additionalProperties false not working - Response validation Mar 5, 2020
@luboskriz
Copy link
Author

Thanks for the investigation, @cdimascio, appreciate it. I will implement some workaround.

@cdimascio
Copy link
Owner

cdimascio commented Mar 8, 2020

FYI, i saw this today, json-schema-merge-all-of. I will plan to look into it. Im curious if it might help this situation. If so, perhaps, we can leverage it within the validator.

It seems to tackle this issue as well

Option to override common impossibility like adding properties when using additionalProperties: false

@asafMasa
Copy link

@cdimascio is there any progress with this issue?

@AlekseyMko
Copy link

Still facing this issue

@patricktyndall
Copy link

Seeing this issue as well.

Our API schema is "composed" by combining lower level types into high level complex response types (using allOf), and this bug kills all our plans to use this library to help validate responses.

Anybody found a good workaround?

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

5 participants