From 766806b7609d373a6abea6448085687d3344ca7d Mon Sep 17 00:00:00 2001 From: mzbik Date: Sat, 10 Feb 2024 09:04:15 -0700 Subject: [PATCH] Fixes for 881 - multiple specs w/validateRequests fail (#903) --- .../2-standard-multiple-api-specs/api.v2.yaml | 6 +- src/framework/openapi.context.ts | 2 + src/framework/openapi.spec.loader.ts | 7 ++ src/framework/types.ts | 1 + src/middlewares/openapi.metadata.ts | 2 + src/middlewares/openapi.response.validator.ts | 5 +- src/openapi.validator.ts | 17 ++- test/356.campaign.spec.ts | 2 +- test/881.spec.ts | 119 ++++++++++++++++++ test/api.v2.yaml | 6 +- 10 files changed, 154 insertions(+), 13 deletions(-) create mode 100644 test/881.spec.ts diff --git a/examples/2-standard-multiple-api-specs/api.v2.yaml b/examples/2-standard-multiple-api-specs/api.v2.yaml index b5b7f2ea..91a58f85 100644 --- a/examples/2-standard-multiple-api-specs/api.v2.yaml +++ b/examples/2-standard-multiple-api-specs/api.v2.yaml @@ -117,9 +117,9 @@ components: schemas: Pet: required: - - id - - name - - type + - pet_id + - pet_name + - pet_type properties: pet_id: readOnly: true diff --git a/src/framework/openapi.context.ts b/src/framework/openapi.context.ts index 2df421f0..8d6fc23f 100644 --- a/src/framework/openapi.context.ts +++ b/src/framework/openapi.context.ts @@ -12,6 +12,7 @@ export class OpenApiContext { public readonly routes: RouteMetadata[] = []; public readonly ignoreUndocumented: boolean; public readonly useRequestUrl: boolean; + public readonly serial: number; private readonly basePaths: string[]; private readonly ignorePaths: RegExp | Function; @@ -28,6 +29,7 @@ export class OpenApiContext { this.ignoreUndocumented = ignoreUndocumented; this.buildRouteMaps(spec.routes); this.useRequestUrl = useRequestUrl; + this.serial = spec.serial; } public isManagedRoute(path: string): boolean { diff --git a/src/framework/openapi.spec.loader.ts b/src/framework/openapi.spec.loader.ts index 44b6a8a3..3ccb1486 100644 --- a/src/framework/openapi.spec.loader.ts +++ b/src/framework/openapi.spec.loader.ts @@ -9,6 +9,7 @@ export interface Spec { apiDoc: OpenAPIV3.Document; basePaths: string[]; routes: RouteMetadata[]; + serial: number; } export interface RouteMetadata { @@ -23,6 +24,7 @@ interface DiscoveredRoutes { apiDoc: OpenAPIV3.Document; basePaths: string[]; routes: RouteMetadata[]; + serial: number; } // Sort routes by most specific to least specific i.e. static routes before dynamic // e.g. /users/my_route before /users/{id} @@ -33,6 +35,9 @@ export const sortRoutes = (r1, r2) => { return e1 > e2 ? 1 : -1; }; +// Uniquely identify the Spec that is emitted +let serial = 0; + export class OpenApiSpecLoader { private readonly framework: OpenAPIFramework; constructor(opts: OpenAPIFrameworkArgs) { @@ -91,10 +96,12 @@ export class OpenApiSpecLoader { routes.sort(sortRoutes); + serial = serial + 1; return { apiDoc, basePaths, routes, + serial }; } diff --git a/src/framework/types.ts b/src/framework/types.ts index eb4f4af0..7cc4167a 100644 --- a/src/framework/types.ts +++ b/src/framework/types.ts @@ -497,6 +497,7 @@ export interface OpenApiRequestMetadata { openApiRoute: string; pathParams: { [index: string]: string }; schema: OpenAPIV3.OperationObject; + serial: number; } export interface OpenApiRequest extends Request { diff --git a/src/middlewares/openapi.metadata.ts b/src/middlewares/openapi.metadata.ts index a1370736..12813f05 100644 --- a/src/middlewares/openapi.metadata.ts +++ b/src/middlewares/openapi.metadata.ts @@ -48,6 +48,7 @@ export function applyOpenApiMetadata( openApiRoute: openApiRoute, pathParams: pathParams, schema: schema, + serial: openApiContext.serial, }; req.params = pathParams; if (responseApiDoc) { @@ -101,6 +102,7 @@ export function applyOpenApiMetadata( expressRoute, openApiRoute, pathParams, + serial: -1, }; (r)._responseSchema = _schema; return r; diff --git a/src/middlewares/openapi.response.validator.ts b/src/middlewares/openapi.response.validator.ts index 6166ca5a..a187ad0a 100644 --- a/src/middlewares/openapi.response.validator.ts +++ b/src/middlewares/openapi.response.validator.ts @@ -32,15 +32,18 @@ export class ResponseValidator { [key: string]: { [key: string]: ValidateFunction }; } = {}; private eovOptions: ValidateResponseOpts; + private serial: number; constructor( openApiSpec: OpenAPIV3.Document, options: Options = {}, eovOptions: ValidateResponseOpts = {}, + serial: number = -1, ) { this.spec = openApiSpec; this.ajvBody = createResponseAjv(openApiSpec, options); this.eovOptions = eovOptions; + this.serial = serial; // This is a pseudo-middleware function. It doesn't get registered with // express via `use` @@ -51,7 +54,7 @@ export class ResponseValidator { public validate(): RequestHandler { return mung.json((body, req, res) => { - if (req.openapi) { + if (req.openapi && this.serial == req.openapi.serial) { const openapi = req.openapi; // instead of openapi.schema, use openapi._responseSchema to get the response copy const responses: OpenAPIV3.ResponsesObject = (openapi) diff --git a/src/openapi.validator.ts b/src/openapi.validator.ts index c4b639e7..43073be0 100644 --- a/src/openapi.validator.ts +++ b/src/openapi.validator.ts @@ -35,6 +35,12 @@ export { Forbidden, } from './framework/types'; +interface MiddlewareContext { + context: OpenApiContext, + responseApiDoc: OpenAPIV3.Document, + error: any, +} + export class OpenApiValidator { readonly options: NormalizedOpenApiValidatorOpts; readonly ajvOpts: AjvOptions; @@ -92,7 +98,7 @@ export class OpenApiValidator { installMiddleware(spec: Promise): OpenApiRequestHandler[] { const middlewares: OpenApiRequestHandler[] = []; - const pContext = spec + const pContext: Promise = spec .then((spec) => { const apiDoc = spec.apiDoc; const ajvOpts = this.ajvOpts.preprocessor; @@ -183,7 +189,7 @@ export class OpenApiValidator { }); } - // request middlweare + // request middleware if (this.options.validateRequests) { let reqmw; middlewares.push(function requestMiddleware(req, res, next) { @@ -201,8 +207,8 @@ export class OpenApiValidator { let resmw; middlewares.push(function responseMiddleware(req, res, next) { return pContext - .then(({ responseApiDoc }) => { - resmw = resmw || self.responseValidationMiddleware(responseApiDoc); + .then(({ responseApiDoc, context: { serial } }) => { + resmw = resmw || self.responseValidationMiddleware(responseApiDoc, serial); return resmw(req, res, next); }) .catch(next); @@ -288,12 +294,13 @@ export class OpenApiValidator { return (req, res, next) => requestValidator.validate(req, res, next); } - private responseValidationMiddleware(apiDoc: OpenAPIV3.Document) { + private responseValidationMiddleware(apiDoc: OpenAPIV3.Document, serial: number) { return new middlewares.ResponseValidator( apiDoc, this.ajvOpts.response, // This has already been converted from boolean if required this.options.validateResponses as ValidateResponseOpts, + serial ).validate(); } diff --git a/test/356.campaign.spec.ts b/test/356.campaign.spec.ts index 57004cd5..fbe834db 100644 --- a/test/356.campaign.spec.ts +++ b/test/356.campaign.spec.ts @@ -1,4 +1,4 @@ -import * as path from 'path'; + import * as path from 'path'; import * as express from 'express'; import * as request from 'supertest'; import { createApp } from './common/app'; diff --git a/test/881.spec.ts b/test/881.spec.ts new file mode 100644 index 00000000..33bd3d19 --- /dev/null +++ b/test/881.spec.ts @@ -0,0 +1,119 @@ +import { expect } from 'chai'; +import * as request from 'supertest'; + +describe('multi-spec', () => { + let app = null; + + before(async () => { + // Set up the express app + app = createServer(); + }); + + after(() => { + app.server.close(); + }); + + it('create campaign should return 200', async () => + request(app) + .get(`/v1/pets`) + .expect(400) + .then((r) => { + expect(r.body.message).include('limit'); + expect(r.body.message).include(`'type'`); + })); + + it('create campaign should return 200', async () => + request(app) + .get(`/v2/pets`) + .expect(400) + .then((r) => { + expect(r.body.message).include(`'pet_type'`); + })); +}); + +function createServer() { + const express = require('express'); + const path = require('path'); + const bodyParser = require('body-parser'); + const http = require('http'); + const OpenApiValidator = require('../src'); + + const app = express(); + app.use(bodyParser.urlencoded({ extended: false })); + app.use(bodyParser.text()); + app.use(bodyParser.json()); + + const versions = [1, 2]; + + for (const v of versions) { + const apiSpec = path.join(__dirname, `api.v${v}.yaml`); + app.use( + OpenApiValidator.middleware({ + apiSpec, + validateResponses: true, + }), + ); + + routes(app, v); + } + + const server = http.createServer(app); + server.listen(3000); + console.log('Listening on port 3000'); + + function routes(app, v) { + if (v === 1) routesV1(app); + if (v === 2) routesV2(app); + } + + function routesV1(app) { + const v = '/v1'; + app.post(`${v}/pets`, (req, res, next) => { + res.json({ ...req.body }); + }); + app.get(`${v}/pets`, (req, res, next) => { + res.json([ + { + id: 1, + name: 'happy', + type: 'cat', + }, + ]); + }); + + app.use((err, req, res, next) => { + // format error + res.status(err.status || 500).json({ + message: err.message, + errors: err.errors, + }); + }); + } + + function routesV2(app) { + const v = '/v2'; + app.get(`${v}/pets`, (req, res, next) => { + res.json([ + { + pet_id: 1, + pet_name: 'happy', + pet_type: 'kitty', + }, + ]); + }); + app.post(`${v}/pets`, (req, res, next) => { + res.json({ ...req.body }); + }); + + app.use((err, req, res, next) => { + // format error + res.status(err.status || 500).json({ + message: err.message, + errors: err.errors, + }); + }); + } + + app.server = server; + return app; +} diff --git a/test/api.v2.yaml b/test/api.v2.yaml index b5b7f2ea..91a58f85 100644 --- a/test/api.v2.yaml +++ b/test/api.v2.yaml @@ -117,9 +117,9 @@ components: schemas: Pet: required: - - id - - name - - type + - pet_id + - pet_name + - pet_type properties: pet_id: readOnly: true