Skip to content

Commit

Permalink
Fixes for 881 - multiple specs w/validateRequests fail (#903)
Browse files Browse the repository at this point in the history
  • Loading branch information
mzbik committed Feb 10, 2024
1 parent 708f2f5 commit 766806b
Show file tree
Hide file tree
Showing 10 changed files with 154 additions and 13 deletions.
6 changes: 3 additions & 3 deletions examples/2-standard-multiple-api-specs/api.v2.yaml
Expand Up @@ -117,9 +117,9 @@ components:
schemas:
Pet:
required:
- id
- name
- type
- pet_id
- pet_name
- pet_type
properties:
pet_id:
readOnly: true
Expand Down
2 changes: 2 additions & 0 deletions src/framework/openapi.context.ts
Expand Up @@ -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;

Expand All @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions src/framework/openapi.spec.loader.ts
Expand Up @@ -9,6 +9,7 @@ export interface Spec {
apiDoc: OpenAPIV3.Document;
basePaths: string[];
routes: RouteMetadata[];
serial: number;
}

export interface RouteMetadata {
Expand All @@ -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}
Expand All @@ -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) {
Expand Down Expand Up @@ -91,10 +96,12 @@ export class OpenApiSpecLoader {

routes.sort(sortRoutes);

serial = serial + 1;
return {
apiDoc,
basePaths,
routes,
serial
};
}

Expand Down
1 change: 1 addition & 0 deletions src/framework/types.ts
Expand Up @@ -497,6 +497,7 @@ export interface OpenApiRequestMetadata {
openApiRoute: string;
pathParams: { [index: string]: string };
schema: OpenAPIV3.OperationObject;
serial: number;
}

export interface OpenApiRequest extends Request {
Expand Down
2 changes: 2 additions & 0 deletions src/middlewares/openapi.metadata.ts
Expand Up @@ -48,6 +48,7 @@ export function applyOpenApiMetadata(
openApiRoute: openApiRoute,
pathParams: pathParams,
schema: schema,
serial: openApiContext.serial,
};
req.params = pathParams;
if (responseApiDoc) {
Expand Down Expand Up @@ -101,6 +102,7 @@ export function applyOpenApiMetadata(
expressRoute,
openApiRoute,
pathParams,
serial: -1,
};
(<any>r)._responseSchema = _schema;
return r;
Expand Down
5 changes: 4 additions & 1 deletion src/middlewares/openapi.response.validator.ts
Expand Up @@ -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`
Expand All @@ -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 = <OpenApiRequestMetadata>req.openapi;
// instead of openapi.schema, use openapi._responseSchema to get the response copy
const responses: OpenAPIV3.ResponsesObject = (<any>openapi)
Expand Down
17 changes: 12 additions & 5 deletions src/openapi.validator.ts
Expand Up @@ -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;
Expand Down Expand Up @@ -92,7 +98,7 @@ export class OpenApiValidator {

installMiddleware(spec: Promise<Spec>): OpenApiRequestHandler[] {
const middlewares: OpenApiRequestHandler[] = [];
const pContext = spec
const pContext: Promise<MiddlewareContext> = spec
.then((spec) => {
const apiDoc = spec.apiDoc;
const ajvOpts = this.ajvOpts.preprocessor;
Expand Down Expand Up @@ -183,7 +189,7 @@ export class OpenApiValidator {
});
}

// request middlweare
// request middleware
if (this.options.validateRequests) {
let reqmw;
middlewares.push(function requestMiddleware(req, res, next) {
Expand All @@ -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);
Expand Down Expand Up @@ -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();
}

Expand Down
2 changes: 1 addition & 1 deletion 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';
Expand Down
119 changes: 119 additions & 0 deletions 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;
}
6 changes: 3 additions & 3 deletions test/api.v2.yaml
Expand Up @@ -117,9 +117,9 @@ components:
schemas:
Pet:
required:
- id
- name
- type
- pet_id
- pet_name
- pet_type
properties:
pet_id:
readOnly: true
Expand Down

0 comments on commit 766806b

Please sign in to comment.