Gi60s / openapi-enforcer

Apache License 2.0
94 stars 22 forks source link

Error validating spec which has an example containing a list of items #70

Closed Klaus-Nji-sp closed 4 years ago

Klaus-Nji-sp commented 4 years ago

Awesome library btw but I have having an issue with a spec passing validation if examples contain arrays. For example, consider this spec:

responses:
    '200':
      description: List of matching documents.
      content:
        application/json:
          schema:
            type: array
            items:
              $ref: '../schemas/response.yaml'
          examples:
            cars:
              $ref: '../schemas/examples/cars.yaml'

If cars.yaml is empty, I do not get an error.

If cars.yaml has this content:

- "volvo xc90"
- "bmw i8"

I get this validation error:

at: /searchForCars > post > responses > 200 > content > application/json
      at: examples
        at: cars
          Unable to deserialize value
            Expected an array. Received: [object Object]

I have not been able to find the correct content for this file.

This may just be my misunderstanding of the specification but will continue digging.

Gi60s commented 4 years ago

Thanks for the compliment on the library.

To help me troubleshoot this, will you share an example of what the ../schemas/response.yaml file contains?

Klaus-Nji-sp commented 4 years ago

Excellent library indeed.

Response.yaml could be a simple file with a couple of properties but that does not really matter. Here is how you can easily duplicate the problem.

In test-resources/v3-petstore.yml add the following snippet to the

/pets-> get -> responses -> 200 -> content -> application/json ->

portion of the schema:

           example: 
                $ref: "pets.yml"

That path will then look as follows:

/pets:
    get:
      summary: List all pets
      operationId: listPets
      tags:
        - pets
      parameters:
        - name: limit
          in: query
          description: How many items to return at one time (max 100)
          required: false
          schema:
            type: integer
            format: int32
      responses:
        '200':
          description: A paged array of pets
          headers:
            x-next:
              description: A link to the next page of responses
              schema:
                type: string
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Pets"
              example: 
                $ref: "pets.yml"

The content of pets.yml is as follows:

  - 
    id : 0
    name: "Bingo"
  - 
    id: 1
    name: Bullet

The run all the tests again and you will some some failing. I have been trying to debug this by running the test production and development instances have the same structure and what I am seeing is a second attempt to validate examples using an of two elements of nulls.

Think I am getting close but you'll probably be able to get there quicker.

Gi60s commented 4 years ago

I'm still having a hard time getting the environment configured properly to see the error. I've created a temporary repository at https://github.com/Gi60s/openai-enforcer-issue-70 which I've invited you to. If you can help me configure the YAML files correctly I'll work on the troubleshooting.

Thanks.

Klaus-Nji-sp commented 4 years ago

I quickly looked through the repo and that is about right in terms of the problematic spec. Not quite sure why you are not able to duplicate the problem.

Perhaps let me continue this investigation and see if I can clearly define where I see the problem. Think I am close. Is there a dramatic difference in behavior when the production flag is off?

Also, were you able to run the unit tests after making the above mods?

Gi60s commented 4 years ago

If you would like to point me to the repo that you're using I can try taking a look there too for you. Thanks for the help with troubleshooting.

Gi60s commented 4 years ago

Are you using the latest version of the enforcer?

Klaus-Nji-sp commented 4 years ago

Yea, I am using version 1.10.1.

In terms of repo, sorry I just checked out master and started debugging, so have not yet created a branch. But what I am seeing so far is that the examples array is deserialized correctly but when it is passed through validation the incoming array now becomes an array of nulls. This seems to only happen in production mode. I will try to narrow down the area of code.

Klaus-Nji-sp commented 4 years ago

Some more info:

The snippet below is from validateExamples in utils.js:

function validateExamples(context, exception) {
    if (context.hasOwnProperty('schema')) {
        if (context.hasOwnProperty('example')) {
            const child = exception.at('example');
            let value;
            let error;
            [ value, error ] = context.schema.deserialize(context.example);
            if (!error) error = context.schema.validate(value);

This code appears to be invoked when there are examples in the schema. When production mode is false, we come out of line

[ value, error ] = context.schema.deserialize(context.example);

with context.exampleactually containing the deserialized array in pets.yaml. In otherwords,

context.example = [{"id" : 0, "name": "Bingo"}, {"id":1, "name" : "Bullet"}]

However, when production mode is true, we come out of that line with context.example an array of two empty object literals.

context.example = [{}, {}]

Which causes the next line to result to an error. I am running the unit test named production and development instances have the same structure in index.tests.js.

If I change that test so that both Enforcers are being constructed with { production: false } test passes with the examples added to the schema.

Klaus-Nji-sp commented 4 years ago

I ran into another issue on how a collection of examples is also being handled. Should I open a new issue or should we track that in here as well? Looks like the validateExamples needs some more test cases.

Gi60s commented 4 years ago

So to make sure I'm understanding this correctly, the issue that was initially reported only occurs when setting the production option to true. Is that right?

For the second issue, is it also related to the production set to true option? If so I'd say we keep it in the same issue.

Klaus-Nji-sp commented 4 years ago

The first issue is only related to production being set to true.

For the second issue, the value of production does not matter.

Both issues are certainly related to the handling of examples so can fit here. But it may be better to create a new issue for the second one. Please let me know.

Gi60s commented 4 years ago

Let's create a second issue. Thank you.

Klaus-Nji-sp commented 4 years ago

Please ignore the comment on production mode. Cannot really recall how I ran into that conclusion! I guess it is save to say there is a general parsing problem when we have examples with arrays.

Gi60s commented 4 years ago

If you can provide me with a non-working example or get https://github.com/Gi60s/openai-enforcer-issue-70 to a state that it's demonstrating this then please let me know. I'd love to help out.

Klaus-Nji-sp commented 4 years ago

Please run the following spec through the Enforcer with default settings:

openapi: "3.0.0"
info:
  version: 1.0.0
  title: Swagger Petstore
  license:
    name: MIT
servers:
  - url: http://petstore.swagger.io/v1
paths: 
  /pets:
    get:
      summary: List all pets
      operationId: listPets
      tags:
        - pets
      parameters:
        - name: limit
          in: query
          description: How many items to return at one time (max 100)
          required: false
          schema:
            type: integer
            format: int32
      responses:
        '200':
          description: A paged array of pets
          headers:
            x-next:
              description: A link to the next page of responses
              schema:
                type: string
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Pets"
              examples: 
                dogs:
                  $ref: "dogs.yml"
                cats:
                  $ref: "cats.yml"

        default:
          description: unexpected error
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Error"

components:
  schemas:
    Pet:
      type: object
      required:
        - id
        - name
      properties:
        id:
          type: integer
          format: int64
        name:
          type: string
        tag:
          type: string
    Pets:
      type: array
      items:
        $ref: "#/components/schemas/Pet"
    Error:
      type: object
      required:
        - code
        - message
      properties:
        code:
          type: integer
          format: int32
        message:
          type: string

I have tried with the following content in dogs.yaml and cats.yaml based on what I discovered in the code:

dogs.yaml:

- 
  summary: 'Our first dog'
  value:
    id : 0
    name: "Bingo"
-
  summary : 'Our last dog'
  value:
    id: 1
    name: Bullet

cats.yaml: Could be similar to dog with slight modifications.

The error you get is as follows:

at: /pets > get > responses > 200 > content > application/json > examples at: dogs Unable to deserialize value Expected an array. Received: undefined Value must be a plain object at: cats Unable to deserialize value Expected an array. Received: undefined Value must be a plain object

There appears to be an issue dealing with examples containing collections.

Thanks again for looking into this.

Gi60s commented 4 years ago

If I understand this correctly, following the provided OpenAPI spec, after dereferencing occurs your response example structure will look like this:

examples: 
  dogs:
    - summary: 'Our first dog'
      value:
        id: 0
        name: "Bingo"
    - summary: 'Our last dog'
      value:
        id: 1
        name: Bullet

But to be valid it should look like this:

examples:
  dogs:
    summary: 'An example of a dogs'
    value:
      - id: 0
        name: "Bingo"
      - id: 1
        name: Bullet

Check out the example in the spec here: https://swagger.io/specification/#mediaTypeObject

The enforcer dereferences everything successfully, but when it goes to validate the example it can't find it because the structure is wrong.

Klaus-Nji-sp commented 4 years ago

And you are absolutely correct sir, thank you. Glad it is just a format with the file.

Robust library!!

Gi60s commented 4 years ago

LOL. Glad we figured it out. Thanks for the compliments.