BugFix: #778 dont overwrite api doc when controller is shared (#780)

* hf: fixes #778

restores behavior of binding the operation, or (new behavior) by binding each middleware in the array

* handle node16 error message

* fix linting (why isnt prettier a precommit hook?)

* adds test with same controller supporting multiple operations in express

* fix linting

packages/express-openapi/test/sample-projects/with-multiple-operations-sharing-the-same-controller/api-doc.json

@@ -0,0 +1,31 @@
+{
+  "swagger": "2.0",
+  "info": {
+    "title": "sample api doc",
+    "version": "3"
+  },
+  "paths": {
+    "/foo": {
+      "get": {
+        "operationId": "getFoo",
+        "responses": {
+          "default": {
+            "description": "return foo",
+            "schema": {}
+          }
+        }
+      }
+    },
+    "/foo-two": {
+      "get": {
+        "operationId": "getFooTwo",
+        "responses": {
+          "default": {
+            "description": "same controller mounted on another path with different operationId",
+            "schema": {}
+          }
+        }
+      }
+    }
+  }
+}

packages/express-openapi/test/sample-projects/with-multiple-operations-sharing-the-same-controller/app.js

@@ -0,0 +1,35 @@
+var app = require('express')();
+var bodyParser = require('body-parser');
+// normally you'd just do require('express-openapi'), but this is for test purposes.
+var openapi = require('../../../');
+var path = require('path');
+var cors = require('cors');
+
+app.use(cors());
+app.use(bodyParser.json());
+
+//shared controller
+function controller(req, res, next) {
+  return res.json(req.operationDoc);
+}
+
+openapi.initialize({
+  apiDoc: require('./api-doc.json'),
+  app: app,
+  promiseMode: true,
+  operations: {
+    getFoo: controller,
+    getFooTwo: controller,
+  },
+});
+
+app.use(function (err, req, res, next) {
+  res.status(err.status).json(err.message);
+});
+
+module.exports = app;
+
+var port = parseInt(process.argv[2], 10);
+if (port) {
+  app.listen(port);
+}

packages/express-openapi/test/sample-projects/with-multiple-operations-sharing-the-same-controller/spec.js

@@ -0,0 +1,45 @@
+const expect = require('chai').expect;
+const request = require('supertest');
+let app;
+
+before(function () {
+  app = require('./app.js');
+});
+
+it('should return the correct operationDoc for getFoo', () => {
+  const expectedDoc = {
+    operationId: 'getFoo',
+    responses: {
+      default: {
+        description: 'return foo',
+        schema: {},
+      },
+    },
+  };
+
+  request(app)
+    .get('/foo')
+    .expect(200)
+    .end(function (err, res) {
+      expect(res.body).to.eql(expectedDoc);
+    });
+});
+
+it('should return the correct operation doc for getFooTwo', () => {
+  const expectedDoc = {
+    operationId: 'getFooTwo',
+    responses: {
+      default: {
+        description:
+          'same controller mounted on another path with different operationId',
+        schema: {},
+      },
+    },
+  };
+  request(app)
+    .get('/foo-two')
+    .expect(200)
+    .end(function (err, res) {
+      expect(res.body).to.eql(expectedDoc);
+    });
+});

packages/openapi-framework/index.ts

@@ -259,7 +259,19 @@ export default class OpenAPIFramework implements IOpenAPIFramework {
               if (operationId && operationId in this.operations) {
                 const operation = this.operations[operationId];
                 acc[METHOD_ALIASES[method]] = (() => {
-                  const innerFunction: any = operation;
+                  /**
+                   * We have two options:
+                   *
+                   * 1. the middleware gets bound + dependency injected, this may be breaking.
+                   * 2. we pick the last middleware as the operation handler. This means we cannot support
+                   *    _after_ middlewares (though not a common express pattern)
+                   */
+                  const innerFunction: any = Array.isArray(operation)
+                    ? operation.map((middleware) =>
+                        middleware.bind({ dependencies: this.dependencies })
+                      )
+                    : operation.bind({ dependencies: this.dependencies });
+
                   innerFunction.apiDoc = methodDoc;
                   return innerFunction;
                 })();

packages/openapi-framework/test/sample-projects/missing-operation-id/spec.ts

@@ -33,9 +33,15 @@ describe(path.basename(__dirname), () => {
   });
 
   it('should throw an error', () => {
+    /**
+     * Node16 (really, the v8 engine) changed the error message
+     * This regex is here so that tests pass on earlier versions and Node16
+     */
     expect(() => {
       framework.initialize({});
-    }).to.throw("Cannot read property 'undefined' of undefined");
+    }).to.throw(
+      /Cannot read property 'undefined' of undefined|Cannot read properties of undefined \(reading 'undefined'\)/
+    );
     expect(warnings).to.deep.equal([
       'some-framework: path /foo, operation get is missing an operationId',
     ]);

packages/openapi-framework/test/sample-projects/operations-dir-with-valid-methid-doc-after-dependency-injection/apiDoc.yml

@@ -10,4 +10,11 @@ paths:
         default:
           description: return foo
           schema: {}
+  /foo-two:
+    get:
+      operationId: getFooTwo
+      responses:
+        default:
+          description: same controller mounted on another path with different operationId
+          schema: {}
 

packages/openapi-framework/test/sample-projects/operations-dir-with-valid-methid-doc-after-dependency-injection/spec.ts

@@ -18,6 +18,7 @@ describe(path.basename(__dirname), () => {
       name: 'some-framework',
       operations: {
         getFoo: require('./operations/foo'),
+        getFooTwo: require('./operations/foo'),
       },
     });
   });
@@ -33,6 +34,11 @@ describe(path.basename(__dirname), () => {
       },
       visitApi(ctx) {
         const apiDoc = ctx.getApiDoc();
+
+        /**
+         * The two operations should have separate documentation,
+         * despite sharing the same controller
+         */
         expect(apiDoc.paths['/foo']).to.eql({
           parameters: [],
           get: {
@@ -45,6 +51,19 @@ describe(path.basename(__dirname), () => {
             },
           },
         });
+        expect(apiDoc.paths['/foo-two']).to.eql({
+          parameters: [],
+          get: {
+            operationId: 'getFooTwo',
+            responses: {
+              default: {
+                description:
+                  'same controller mounted on another path with different operationId',
+                schema: {},
+              },
+            },
+          },
+        });
       },
     });
   });