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

Fixes for 881 - multiple specs w/validateRequests fail #903

Merged
merged 3 commits into from Feb 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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