improve preprocess logic and skip unneeded clones (#515)

* feat: skip visited nodes

* fix: remove uneeded clones

* fix: cleanup

* fix: remove yaml parse

* fix: remove merge

* fix: remove uneeded deps

* chore: beta

* fix: skip schemas

* fix: update path to $refs

* fix: visit ref and update deref path

* chore: comment

* test: add polymorphism test for #511

package.json

@@ -1,6 +1,6 @@
 {
   "name": "express-openapi-validator",
-  "version": "4.10.4",
+  "version": "4.11.0-beta.2",
   "description": "Automatically validate API requests and responses with OpenAPI 3 and Express.",
   "main": "dist/index.js",
   "scripts": {
@@ -34,11 +34,9 @@
   "dependencies": {
     "ajv": "^6.12.6",
     "content-type": "^1.0.4",
-    "js-yaml": "^3.14.0",
     "json-schema-ref-parser": "^9.0.6",
     "lodash.clonedeep": "^4.5.0",
     "lodash.get": "^4.4.2",
-    "lodash.merge": "^4.6.2",
     "lodash.uniq": "^4.5.0",
     "lodash.zipobject": "^4.1.3",
     "media-typer": "^1.1.0",

src/framework/index.ts

@@ -1,5 +1,4 @@
 import * as fs from 'fs';
-import * as jsYaml from 'js-yaml';
 import * as path from 'path';
 import * as $RefParser from 'json-schema-ref-parser';
 import { OpenAPISchemaValidator } from './openapi.schema.validator';
@@ -23,13 +22,12 @@ export class OpenAPIFramework {
     visitor: OpenAPIFrameworkVisitor,
   ): Promise<OpenAPIFrameworkInit> {
     const args = this.args;
-    const apiDoc = await this.copy(
-      await this.loadSpec(args.apiDoc, args.$refParser),
-    );
+    const apiDoc = await this.loadSpec(args.apiDoc, args.$refParser);
+
     const basePathObs = this.getBasePathsFromServers(apiDoc.servers);
     const basePaths = Array.from(
       basePathObs.reduce((acc, bp) => {
-        bp.all().forEach(path => acc.add(path));
+        bp.all().forEach((path) => acc.add(path));
         return acc;
       }, new Set<string>()),
     );
@@ -55,7 +53,7 @@ export class OpenAPIFramework {
       }
     }
     const getApiDoc = () => {
-      return this.copy(apiDoc);
+      return apiDoc;
     };
 
     this.sortApiDocTags(apiDoc);
@@ -87,13 +85,9 @@ export class OpenAPIFramework {
         // Get document, or throw exception on error
         try {
           process.chdir(specDir);
-          const docWithRefs = jsYaml.safeLoad(
-            fs.readFileSync(absolutePath, 'utf8'),
-            { json: true },
-          );
           return $refParser.mode === 'dereference'
-            ? $RefParser.dereference(docWithRefs)
-            : $RefParser.bundle(docWithRefs);
+            ? $RefParser.dereference(absolutePath)
+            : $RefParser.bundle(absolutePath);
         } finally {
           process.chdir(origCwd);
         }
@@ -108,10 +102,6 @@ export class OpenAPIFramework {
       : $RefParser.bundle(filePath);
   }
 
-  private copy<T>(obj: T): T {
-    return JSON.parse(JSON.stringify(obj));
-  }
-
   private sortApiDocTags(apiDoc: OpenAPIV3.Document): void {
     if (apiDoc && Array.isArray(apiDoc.tags)) {
       apiDoc.tags.sort((a, b): number => {
@@ -131,6 +121,6 @@ export class OpenAPIFramework {
       const basePath = new BasePath(server);
       basePathsMap[basePath.expressPath] = basePath;
     }
-    return Object.keys(basePathsMap).map(key => basePathsMap[key]);
+    return Object.keys(basePathsMap).map((key) => basePathsMap[key]);
   }
 }

src/framework/openapi.context.ts

@@ -1,7 +1,5 @@
 import { OpenAPIV3 } from './types';
 import { Spec, RouteMetadata } from './openapi.spec.loader';
-import { Console } from 'console';
-
 
 export interface RoutePair {
   expressRoute: string;

src/framework/openapi.schema.validator.ts

@@ -1,29 +1,21 @@
 import * as Ajv from 'ajv';
-import * as merge from 'lodash.merge';
 import * as draftSchema from 'ajv/lib/refs/json-schema-draft-04.json';
 // https://github.com/OAI/OpenAPI-Specification/blob/master/schemas/v3.0/schema.json
 import * as openapi3Schema from './openapi.v3.schema.json';
 import { OpenAPIV3 } from './types.js';
 
 export class OpenAPISchemaValidator {
   private validator: Ajv.ValidateFunction;
-  constructor({
-    version,
-    extensions,
-  }: {
-    version: string;
-    extensions?: object;
-  }) {
+  constructor({ version }: { version: string; extensions?: object }) {
     const v = new Ajv({ schemaId: 'auto', allErrors: true });
     v.addMetaSchema(draftSchema);
 
     const ver = version && parseInt(String(version), 10);
     if (!ver) throw Error('version missing from OpenAPI specification');
     if (ver != 3) throw Error('OpenAPI v3 specification version is required');
 
-    const schema = merge({}, openapi3Schema, extensions ?? {});
-    v.addSchema(schema);
-    this.validator = v.compile(schema);
+    v.addSchema(openapi3Schema);
+    this.validator = v.compile(openapi3Schema);
   }
 
   public validate(

src/index.ts

@@ -1,4 +1,3 @@
-import * as _uniq from 'lodash.uniq';
 import * as res from './resolvers';
 import { OpenApiValidator, OpenApiValidatorOpts } from './openapi.validator';
 import { OpenApiSpecLoader } from './framework/openapi.spec.loader';

src/middlewares/openapi.metadata.ts

@@ -1,6 +1,5 @@
 import * as _zipObject from 'lodash.zipobject';
 import { pathToRegexp } from 'path-to-regexp';
-import * as deepCopy from 'lodash.clonedeep';
 import { Response, NextFunction } from 'express';
 import { OpenApiContext } from '../framework/openapi.context';
 import {

src/middlewares/parsers/schema.preprocessor.ts

@@ -102,7 +102,7 @@ export class SchemaPreprocessor {
     const componentSchemas = this.gatherComponentSchemaNodes();
     const r = this.gatherSchemaNodesFromPaths();
 
-    // Now that we've processed paths, clonse the spec
+    // Now that we've processed paths, clone a response spec if we are validating responses
     this.apiDocRes = !!this.responseOpts ? cloneDeep(this.apiDoc) : null;
 
     const schemaNodes = {
@@ -168,20 +168,30 @@ export class SchemaPreprocessor {
    * @param visit a function to invoke per node
    */
   private traverseSchemas(nodes: TopLevelSchemaNodes, visit) {
+    const seen = new Set();
     const recurse = (parent, node, opts: TraversalStates) => {
-      const schema = this.resolveSchema<SchemaObject>(node.schema);
-      if (!schema) {
-        // if we can't dereference a path within the schema, skip preprocessing
-        // TODO handle refs like below during preprocessing
-        // #/paths/~1subscription/get/requestBody/content/application~1json/schema/properties/subscription
+      const schema = node.schema;
+
+      if (!schema || seen.has(schema)) return;
+
+      seen.add(schema);
+
+      if (schema.$ref) {
+        const resolvedSchema = this.resolveSchema<SchemaObject>(schema);
+        const path = schema.$ref.split('/').slice(1);
+
+        (<any>opts).req.originalSchema = schema;
+        (<any>opts).res.originalSchema = schema;
+
+        visit(parent, node, opts);
+        recurse(node, new Node(schema, resolvedSchema, path), opts);
         return;
       }
+
       // Save the original schema so we can check if it was a $ref
-      (<any>opts).req.originalSchema = node.schema;
-      (<any>opts).res.originalSchema = node.schema;
+      (<any>opts).req.originalSchema = schema;
+      (<any>opts).res.originalSchema = schema;
 
-      // TODO mark visited, and skip visited
-      // TODO Visit api docs
       visit(parent, node, opts);
 
       if (schema.allOf) {
@@ -199,8 +209,8 @@ export class SchemaPreprocessor {
           const child = new Node(node, s, [...node.path, 'anyOf', i + '']);
           recurse(node, child, opts);
         });
-      } else if (node.schema.properties) {
-        Object.entries(node.schema.properties).forEach(([id, cschema]) => {
+      } else if (schema.properties) {
+        Object.entries(schema.properties).forEach(([id, cschema]) => {
           const path = [...node.path, 'properties', id];
           const child = new Node(node, cschema, path);
           recurse(node, child, opts);
@@ -249,7 +259,7 @@ export class SchemaPreprocessor {
       const options = opts[kind];
       options.path = node.path;
 
-      if (nschema) {
+      if (nschema) { // This null check should no longer be necessary
         this.handleSerDes(pschema, nschema, options);
         this.handleReadonly(pschema, nschema, options);
         this.processDiscriminator(pschema, nschema, options);

src/openapi.validator.ts

@@ -2,7 +2,6 @@ import ono from 'ono';
 import ajv = require('ajv');
 import * as express from 'express';
 import * as _uniq from 'lodash.uniq';
-import * as cloneDeep from 'lodash.clonedeep';
 import * as middlewares from './middlewares';
 import { Application, Response, NextFunction, Router } from 'express';
 import { OpenApiContext } from './framework/openapi.context';

test/511.spec.ts

@@ -0,0 +1,155 @@
+import * as request from 'supertest';
+import { createApp } from './common/app';
+
+describe('511 schema.preprocessor inheritance', () => {
+  let app = null;
+
+  before(async () => {
+    // set up express app
+    app = await createApp(
+      {
+        apiSpec: apiSpec(),
+        validateResponses: true,
+      },
+      3001,
+      (app) => {
+        app.post('/example', (req, res) => {
+          res.status(201).json({
+            object_type: 'PolyObject1',
+            shared_prop1: 'sp1',
+            shared_prop2: 'sp2',
+            polyObject1SpecificProp1: 'poly1',
+          });
+        });
+      },
+      false,
+    );
+    return app;
+  });
+
+  after(() => {
+    app.server.close();
+  });
+
+  it('should return 201', async () =>
+    request(app)
+      .post(`/example`)
+      .send({
+        object_type: 'PolyObject1',
+        shared_prop1: 'sp1',
+        shared_prop2: 'sp2',
+        polyObject1SpecificProp1: 'poly1',
+      })
+      .expect(201));
+});
+
+function apiSpec(): any {
+  return {
+    openapi: '3.0.0',
+    info: {
+      title: 'Example API',
+      version: '0.1.0',
+    },
+    servers: [
+      {
+        url: 'https://localhost/',
+      },
+    ],
+    paths: {
+      '/example': {
+        post: {
+          description: 'Request',
+          requestBody: {
+            content: {
+              'application/json': {
+                schema: {
+                  $ref: '#/components/schemas/PolyObject',
+                },
+              },
+            },
+          },
+          responses: {
+            '201': {
+              description: 'Response',
+              content: {
+                'application/json': {
+                  schema: {
+                    type: 'object',
+                  },
+                },
+              },
+            },
+          },
+        },
+      },
+    },
+    components: {
+      schemas: {
+        PolyObject: {
+          type: 'object',
+          discriminator: {
+            propertyName: 'object_type',
+            mapping: {
+              PolyObject1: '#/components/schemas/PolyObject1',
+              PolyObject2: '#/components/schemas/PolyObject2',
+            },
+          },
+          oneOf: [
+            {
+              $ref: '#/components/schemas/PolyObject1',
+            },
+            {
+              $ref: '#/components/schemas/PolyObject2',
+            },
+          ],
+        },
+        PolyObjectBase: {
+          type: 'object',
+          required: ['object_type'],
+          properties: {
+            object_type: {
+              type: 'string',
+              enum: ['PolyObject1', 'PolyObject2'],
+            },
+            shared_prop1: {
+              type: 'string',
+            },
+            shared_prop2: {
+              type: 'string',
+            },
+          },
+        },
+        PolyObject1: {
+          allOf: [
+            {
+              $ref: '#/components/schemas/PolyObjectBase',
+            },
+            {
+              type: 'object',
+              properties: {
+                polyObject1SpecificProp1: {
+                  type: 'string',
+                },
+              },
+            },
+          ],
+        },
+        PolyObject2: {
+          allOf: [
+            {
+              $ref: '#/components/schemas/PolyObjectBase',
+            },
+            {
+              type: 'object',
+              properties: {
+                polyObject2SpecificProp1: {
+                  type: 'string',
+                },
+              },
+            },
+          ],
+        },
+      },
+    },
+  };
+}