Gi60s / openapi-enforcer

Apache License 2.0
94 stars 22 forks source link

Schema validation fails when using allOf and arrays specified in both. #110

Closed kristof-mattei closed 3 years ago

kristof-mattei commented 3 years ago

I'm hitting the weirdest error when trying to use openapi-enforcer where I get errors validating a Schema. This is the test code:

const Enforcer = require('openapi-enforcer')

Enforcer("./reproduce.yaml", { fullResult: true, useNewRefParser: true }).then(openapi => {
    const x = new Enforcer.v3_0.Schema(openapi.value.components.schemas.GetSomething)

    console.log(x.error);    
})

This is the schema:

openapi: 3.0.3
info:
  title: Test
  version: 0.0.1
  description: description
  termsOfService: "https://terms"
  contact:
    name: test
    url: "http://url"
    email: test@test.com
servers:
  - url: "https://server"
paths: {}
components:
  schemas:
    JsonApiBaseSingleResponse:
      type: object
      properties:
        data:
          $ref: "#/components/schemas/BasePointer"
    JsonApiBaseMultipleResponse:
      type: object
      properties:
        data:
          type: array
          items:
            $ref: "#/components/schemas/BasePointer"
    GetSomething: # THIS
      allOf:
        - $ref: "#/components/schemas/JsonApiBaseMultipleResponse"
        - properties:
            data:
              type: array
              items:
                $ref: "#/components/schemas/BasePointer"
    BasePointer: {}
  securitySchemes:
    JWTAuth:
      scheme: bearer
      type: http

If we run this code we get the following error:

node ./validate.js
[ EnforcerException: One or more errors exist in the Schema definition
    at: allOf > 0 > properties > data > items
      Properties not allowed: readOnly, writeOnly ]

Now I don't understand the error, I'm not using those properties.

BUT, there are some things that I can do to make them go away, which make me believe this is a library bug.

I can restructure the schema as follows:

  schemas:
    BasePointer: {} # MOVED TO THE TOP
    JsonApiBaseSingleResponse:
      type: object
      properties:
        data:
          $ref: "#/components/schemas/BasePointer"
    JsonApiBaseMultipleResponse:
      type: object
      properties:
        data:
          type: array
          items:
            $ref: "#/components/schemas/BasePointer"
    GetSomething: # THIS
      allOf:
        - $ref: "#/components/schemas/JsonApiBaseMultipleResponse"
        - properties:
            data:
              type: array
              items:
                $ref: "#/components/schemas/BasePointer"

Works like a charm! In this all I did was move BasePointer to the top.

Now, I can understand there is some issue with resolving refs, and this is causing the bug, BUT, if we take our original schema, and REMOVE something that isn't used, i.e. the JsonApiBaseSingleResponse and leave BasePointer at the bottom it ALSO works:

  schemas:
    # JsonApiBaseSingleResponse:
    #   type: object
    #   properties:
    #     data:
    #       $ref: "#/components/schemas/BasePointer"
    JsonApiBaseMultipleResponse:
      type: object
      properties:
        data:
          type: array
          items:
            $ref: "#/components/schemas/BasePointer"
    GetSomething: # THIS
      allOf:
        - $ref: "#/components/schemas/JsonApiBaseMultipleResponse"
        - properties:
            data:
              type: array
              items:
                $ref: "#/components/schemas/BasePointer"

    BasePointer: {}

So I'm not sure what is going wrong here.

Notice that this is a greatly reduced testcase just to show the bug. In reality what I'm trying to do is specifying a base type of objects for the items in the array of GetSomething, and the 2nd item in the allOf in GetSomething actually specifies a an object derived of BasePointer with more attributes.

Gi60s commented 3 years ago

Hey @Kristof-Mattei. Thanks for reaching out.

Looking at your first code example I see what is causing the problem. Here is your code example for reference:

const Enforcer = require('openapi-enforcer')

Enforcer("./reproduce.yaml", { fullResult: true, useNewRefParser: true }).then(openapi => {
    const x = new Enforcer.v3_0.Schema(openapi.value.components.schemas.GetSomething)

    console.log(x.error);    
})

On line 4 you attempt to create a Schema component instance by using openapi.value.components.schemas.GetSomething. The problem is that the openapi parameter from line 3 is already a fully built tree of instantiated components.

In other words, you shouldn't try to create a new Schema component instance from openapi.value.components.schemas.GetSomething because it already is a new Schema component instance.

When a component definition is turned into an enforcer component instance some properties and methods are applied to it. So, when you passed an instance of the Schema instead of a Schema definition it had properties that would cause it to be invalid.

Here is your example corrected:

const Enforcer = require('openapi-enforcer')

Enforcer("./reproduce.yaml", { fullResult: true, useNewRefParser: true }).then(openapi => {
    const x = openapi.value.components.schemas.GetSomething
})

Let me know if this does not fix it for you. I'll close this issue unless I hear otherwise that it's still a problem, but feel free to keep asking questions on this thread as needed.

kristof-mattei commented 3 years ago

@Gi60s Thank you for the swift explanation!