cdimascio / express-openapi-validator

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

Broken ref when using different file for request & response #326

Closed AndrewIsh closed 4 years ago

AndrewIsh commented 4 years ago

Describe the bug

I have spent many hours trying to get to the bottom of this with no luck, so I'm hoping I've found a bug and it's not just my ineptness! :-)

I have a very minimal schema representing get & post methods on a /queries endpoint (see schema below). I'm getting some super weird errors when trying to use them. The problem appears to be related to the post portion of the schema, which has references to two different files, one for the request object and one for the response object (although for debugging purposes, the contents of both files is currently identical)

When I make a GET request, I receive:

{
    "error": {
        "message": "can't resolve reference #/paths/~1queries/post/responses/201/content/application~1json/schema from id #",
        "missingRef": "#/paths/~1queries/post/responses/201/content/application~1json/schema",
        "missingSchema": ""
    },
    "message": "can't resolve reference #/paths/~1queries/post/responses/201/content/application~1json/schema from id #"
}

I am confident the referenced file schemas/queryresponse.yaml is present:

~/ems-api/api_spec$ find
./ems.yaml
./schemas/queryrequest.yaml
./schemas/queryresponse.yaml
./schemas/queries.yaml

In fact, the queries.yaml schema for the get route references this file with no problems.

Bizarrely, in the spec, if I rename the the ref for the response object to schemas/queryrequest.yaml, it works, which is especially odd since both files are currently identical.

I'm surmising from all this that it's something to do with having two differently named files for request & response refs, though in the OpenAPI 3 spec I've not been able to find anything that would prohibit that.

I'm completely out of ideas. My spec seems valid (according to https://editor.swagger.io/ anyway). Might this be a bug?

Examples and context

ems.yaml:

openapi: 3.0.0
info:
  title: EMS
  description: Spec for EMS API
  version: 1.0.0
servers:
  - url: /api/v1.0
paths:
  /queries:
    get:
      summary: Returns all queries
      operationId: getQueries
      parameters:
        - name: offset
          in: query
          required: false
          description: The record index to start returning (e.g. 1 = second record)
          schema:
            type: integer
      responses:
        '200':
          description: An array of query objects
          content:
            'application/json':
              schema:
                $ref: 'schemas/queries.yaml'
        '500':
          description: Server error
    post:
      summary: Create a new query
      operationId: createQuery
      requestBody:
        description: A JSON object describing a query
        required: true
        content:
          'application/json':
            schema:
              $ref: 'schemas/queryrequest.yaml'
      responses:
        '201':
          description: The created query object
          content:
            'application/json':
              schema:
                $ref: 'schemas/queryresponse.yaml'
        '404':
          description: Query not found
        '500':
          description: Server error

schemas/queries.yaml:

type: array
items:
  $ref: 'queryresponse.yaml'

schemas/queryrequest.yaml & schemas/queryresponse.yaml:

type: object
properties:
  id:
    type: integer
    nullable: true
  title:
    type: string
  folder:
    type: string
    nullable: true
cdimascio commented 4 years ago

@AndrewIsh this is a bit cryptic to read. OpenAPI $ref paths can be a bit confusing to read due to special characters

See the 'Escape Characters' section here tl/dr

Character Escape With
~ ~0
/ ~1

For example, to refer to the path /blogs/{blog_id}/new~posts, you would use: $ref: '#/paths/~1blogs~1{blog_id}~1new~0posts'

so ultimately, #/paths/~1queries/post/requestBody/content/application~1json/schem refers to the following in ems.yaml

paths
   /queries
      post:
         requestBody:
            content:
               application/json:
                  schema:

It appears to have the correct location, but it's not resolving the schema.

this gets sent to AJV which isn't capable of resolving multi file yamls. there needs to be some additional logic to dereference that properly for this case. Note that we already do this using $refParser.mode bundle (default) ordereference

It's blowing up AJV when compiling the response schema here

Using the debugger, I inspected the state of the parsed and bundled apiDoc. Basically, your API spec files consolidated to a single JSON (here)[https://github.com/cdimascio/express-openapi-validator/blob/master/src/framework/index.ts#L26] I notice that queryrequest.yaml and queryresponse.yaml are dereferenced, but queries.yaml is not

So what might we do... Under the covers, this validator uses json-schema-ref-parser to bundle or dereference the API spec files.

A good first excercise may be to use json-schema-ref-parser to dereference you API spec properly. If you can get that to do the right thing, I will expose those options (assuming we need them) via this validator.

Currently, json-schema-ref-parser options are offered. For example, we support bundle (default) and dereference. These can be set via $refParser.mode. See the doc in the README for details. So far we use one of the two modes using all json-schema-ref-parser default options. If we need to configure additional options, we can certainly expose them.

Apologies, for not having a solution immediately, but hopefully, we'll be able to come to one after some investigation. It will be a great help if you can look into using json-ref-parser to bundle/dereference your spec.

cdimascio commented 4 years ago

Hmm.. I see these $ref resolved

 "/Users/carmine.dimascio/git/express-openapi-validator/examples/my-example/schemas/queries.yaml": {
    "type": "array",
    "items": {
      "$ref": "#/paths/~1queries/post/requestBody/content/application~1json/schema"
    }
  },
  "/Users/carmine.dimascio/git/express-openapi-validator/examples/my-example/schemas/queryrequest.yaml": {
    "type": "object",
    "properties": {
      "id": {
        "type": "integer",
        "nullable": true
      },
      "title": {
        "type": "string"
      },
      "folder": {
        "type": "string",
        "nullable": true
      }
    }
  },
  "/Users/carmine.dimascio/git/express-openapi-validator/examples/my-example/schemas/queryresponse.yaml": {
    "type": "object",
    "properties": {
      "id": {
        "type": "integer",
        "nullable": true
      },
      "title": {
        "type": "string"
      },
      "folder": {
        "type": "string",
        "nullable": true
      }
    }
  }
}

This may be an issue in the json-schema-ref-praser itself queries.yaml looks fishy. It's not resolving to queryrequest.yaml

cdimascio commented 4 years ago

@AndrewIsh ignore my note above. i'm a bit off with the diagnosis. i think i may have a fix that resolves the issue above will deliver shortly

cdimascio commented 4 years ago

please give v3.16.4 a try. this should solve it for responses. it possible that this also exists for requests (i haven't tested that yet). in any case, the example above should work please reopen if you continue to have issues

AndrewIsh commented 4 years ago

Hi @cdimascio

Thanks so much for responding to this so quickly. Your fix in v3.16.4 appears to work perfectly!

Thanks for creating such a useful library, it's made life much easier for me on my project :-)

AndrewIsh commented 4 years ago

Hi @cdimascio

Sorry to re-open this one. But I think I've just hit the thing you thought I might hit. When you created the fix for my previous problem, you said:

it possible that this also exists for requests (i haven't tested that yet) I've just hit a problem that looks to me like it is exactly this.

I have an endpoint with both a PUT and POST method on, the spec for these two methods is slightly different in that the body for them is different, hence the spec referenced is different:

[...]
    post:
      summary: Create a new query
      operationId: createQuery
      requestBody:
        $ref: '#/components/requestBodies/QueryBody'
      responses:
        '201':
          description: The created query object
          content:
            application/json:
              schema:
                $ref: 'schemas/queryresponse.yaml'
        '404':
          description: Query not found
        '500':
          description: Server error
    put:
      summary: Update queries in bulk
      operationId: updateQueryBulk
      requestBody:
        $ref: '#/components/requestBodies/QueriesBody'
      responses:
        '200':
          description: An array of updated query objects
          content:
            application/json:
              schema:
                $ref: 'schemas/queries.yaml'
        '500':
          description: Server error
[...]
components:
  requestBodies:
    QueryBody:
      description: A JSON object describing a query request
      required: true
      content:
        application/json:
          schema:
            $ref: 'schemas/queryrequest.yaml'
    QueriesBody:
      description: An array of JSON objects describing query requests
      required: true
      content:
        application/json:
          schema:
            $ref: 'schemas/queryrequests.yaml
[...]

Prior to me adding the spec & component for the put endpoint, everything was fine. However, as soon as I added it I received an error very similar to the one I had before:

{
    "error": {
        "message": "can't resolve reference #/components/requestBodies/QueryBody/content/application~1json/schema from id #",
        "missingRef": "#/components/requestBodies/QueryBody/content/application~1json/schema",
        "missingSchema": ""
    },
    "message": "can't resolve reference #/components/requestBodies/QueryBody/content/application~1json/schema from id #"
}

The error is weird because it's saying it can't resolve the QueryBody schema when it's definitely there, and the POST operation works perfectly. This is making me wonder if it's the problem that you foresaw.

I've looked at what you did to fix the previous issue, with a view to trying a similar fix for requests, but have not been able to determine where such a fix would be made.

Do you think this could be the problem you suggested?

cdimascio commented 4 years ago

yes. working on a fix

cdimascio commented 4 years ago

PR #334 should resolve this. it is included in v3.16.6 let me know how it goes. thanks again!

AndrewIsh commented 4 years ago

Hi @cdimascio

Thanks for the super speedy response. I just tried it and it works perfectly! Many thanks for fixing it, and for your continued work on this extremely useful package! :-)