cdimascio / express-openapi-validator

šŸ¦‹ Auto-validates api requests, responses, and securities using ExpressJS and an OpenAPI 3.x specification
MIT License
919 stars 209 forks source link

Introduction of multiple swagger docs with new endpoint now versioned returns 404 #876

Open asos-danielc opened 11 months ago

asos-danielc commented 11 months ago

Describe the bug A clear and concise description of what the bug is.

To Reproduce Steps to reproduce the behavior.

./swagger/v1.yaml

openapi: 3.0.3

info:
  title: Hello World Api
  description: API for greeting the world
  contact:
    name: Hello World Api
  version: 1.0.0

servers:
  - url: /

paths:
  /hello-world/{name}:
    get:
      summary: Say hi
      tags:
        - Hello World
      parameters:
        - name: name
          description: Name of who to greet
          example: "Dan"
          in: path
          required: true
          schema:
            type: string
            minLength: 1
      responses:
        200:
          description: (OK) Success.
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/get-hello-world-request-response-body"

components:
  securitySchemes:
    basicAuth:
      type: http
      scheme: basic
  schemas:
    get-hello-world-request-response-body:
      type: object
      properties:
        greet:
          type: string
          description: Greets the user.
  headers:
    authorization:
      description: Identifier (Basic Auth) used to authenticate with the proxied api.
      schema:
        type: string
      required: true

security:
  - basicAuth: []

./swagger/v2.yaml

openapi: 3.0.3

info:
  title: Hello World Api
  description: API for greeting the world
  contact:
    name: Hello World Api
  version: 2.0.0

servers:
  - url: /v2/

paths:
  /hello-world/{name}/{surname}:
    get:
      summary: Say hi
      tags:
        - Hello World
      parameters:
        - name: name
          description: Name of who to greet
          example: "Dan"
          in: path
          required: true
          schema:
            type: string
            minLength: 1
        - name: surname
          description: Surname of who to greet
          example: "Crouch"
          in: path
          required: true
          schema:
            type: string
            minLength: 1
      responses:
        200:
          description: (OK) Success.
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/get-hello-world-request-response-body"

components:
  securitySchemes:
    basicAuth:
      type: http
      scheme: basic
  schemas:
    get-hello-world-request-response-body:
      type: object
      properties:
        greet:
          type: string
          description: Greets the user.
  headers:
    authorization:
      description: Identifier (Basic Auth) used to authenticate with the proxied api.
      schema:
        type: string
      required: true

security:
  - basicAuth: []

useRoutes.js

import { Router } from "express";
import * as OpenApiValidator from "express-openapi-validator";
import path from "path";

import { defaultRoutes, v1Routes, v2Routes } from "./routes";

const versionRoutesMap = [
  { version: "v1", routes: v1Routes },
  { version: "v2", routes: v2Routes },
];
const useRoutes = ({ router, versionRoutes }) => {
  for (const route of versionRoutes) {
    route(router);
  }
};

// I can't figure out why we can't use a loop on versions to register the swagger versions in OpenApiValidator.middleware call and
// then simply call useRoutes to register all versioned routes together.
// If we do take that approach then for some reason it says "/changerequest" not found when we try to hit the route.
// I think it is because the OpenApiValidator.middleware needs to be called after each route has been registered as to avoid conflicts on similar endpoints.
// I have raised a question here on this.
// https://github.com/cdimascio/express-openapi-validator/issues/179#issuecomment-1816781096
export default ({ app }) => {
  const router = Router();

  for (const route of defaultRoutes) {
    route(router);
  }

  for (const versionRoutes of versionRoutesMap) {
    const apiSpec = path.join(
      "./docs/swagger",
      `${versionRoutes.version}.yaml`,
    );
    app.use(
      OpenApiValidator.middleware({
        apiSpec,
        ignorePaths: /.*\/api-docs\/.*/,
      }),
    );
    useRoutes({ router, versionRoutes: versionRoutes.routes });
  }
  app.use(router);
};

The routes are typical express route definitions:

  router.get(
    `/hello-world/:name`,
    (request, response, next) => {
        const { name} = { ...request.params };
        ....
        next();
    }
  );
...
  router.get(
    `/v2/hello-world/:name/:surname`,
    (request, response, next) => {
        const { name} = { ...request.params };
        ....
        next();
    }
  );

Actual behavior A clear and concise description of what happens.

GET http://localhost:3000/hello-world/Dan āœ… works as it did before

GET http://localhost:3000/v2/hello-world/Dan āŒ 404 response

{
    "status": 404,
    "message": "not found",
    "errors": [
        {
            "path": "/v2/hello-world/Dan",
            "message": "not found"
        }
    ]
}

If I don't register the unversioned route (/hello-world/:name), then the v2 route (/v2/hello-world/:name/:surname) does work.

Expected behavior A clear and concise description of what you expected to happen.

I am expecting the unversioned route and the versioned route to both work

Examples and context An example or relevant context e.g. an OpenAPI snippet, an Express handler function snippet

javier-sierra-sngular commented 8 months ago

I encountered the same issue. I can't validate against more than one specification.

Is this a potential bug, or are we using it incorrectly? I couldn't find any official documentation on using multiple specifications

Perhaps the 'apiSpec' could support an array of specifications, allowing the middleware function to be called just once?

Thanks.

Edit: Tested on v5.1.6

javier-sierra-sngular commented 8 months ago

In my case, the problem seems to be when not using servers/url in the spec.

Here are steps to reproduce:

Using the official example 2-standard-multiple-api-specs as is it works fine.

But if we remove servers from the specification

servers:
  - url: /v2

and modify the path from:

paths:
  /pets:

to:

paths:
  /v2/pets:

causes 404

{
  "message": "not found",
  "errors": [
    {
      "path": "/v2/pets",
      "message": "not found"
    }
  ]
}

GET http://localhost:3000/v2/pets?pet_type=kitty