Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance improvement: preprocess will skip previously visited nodes #515

Merged
merged 14 commits into from
Jan 11, 2021
7 changes: 1 addition & 6 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 1 addition & 3 deletions package.json
Original file line number Diff line number Diff line change
@@ -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": {
Expand Down Expand Up @@ -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",
Expand Down
24 changes: 7 additions & 17 deletions src/framework/index.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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>()),
);
Expand All @@ -55,7 +53,7 @@ export class OpenAPIFramework {
}
}
const getApiDoc = () => {
return this.copy(apiDoc);
return apiDoc;
};

this.sortApiDocTags(apiDoc);
Expand Down Expand Up @@ -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);
}
Expand All @@ -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 => {
Expand All @@ -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]);
}
}
2 changes: 0 additions & 2 deletions src/framework/openapi.context.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import { OpenAPIV3 } from './types';
import { Spec, RouteMetadata } from './openapi.spec.loader';
import { Console } from 'console';


export interface RoutePair {
expressRoute: string;
Expand Down
14 changes: 3 additions & 11 deletions src/framework/openapi.schema.validator.ts
Original file line number Diff line number Diff line change
@@ -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(
Expand Down
1 change: 0 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down
1 change: 0 additions & 1 deletion src/middlewares/openapi.metadata.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
36 changes: 23 additions & 13 deletions src/middlewares/parsers/schema.preprocessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion src/openapi.validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
155 changes: 155 additions & 0 deletions test/511.spec.ts
Original file line number Diff line number Diff line change
@@ -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',
},
},
},
],
},
},
},
};
}