diff --git a/README.md b/README.md index 3f9e8369..e1f7eee4 100644 --- a/README.md +++ b/README.md @@ -105,7 +105,7 @@ app.use('/spec', express.static(spec)); // 3. Install the OpenApiValidator onto your express app new OpenApiValidator({ - apiSpecPath: './openapi.yaml', + apiSpec: './openapi.yaml', }).install(app); // 4. Define routes using Express diff --git a/src/index.ts b/src/index.ts index 6424469b..111bf9e1 100644 --- a/src/index.ts +++ b/src/index.ts @@ -6,28 +6,26 @@ import ono from 'ono'; import { OpenAPIV3, OpenApiRequest } from './framework/types'; export interface OpenApiValidatorOpts { - apiSpecPath?: string; - apiSpec?: OpenAPIV3.Document | string; + apiSpec: OpenAPIV3.Document | string; coerceTypes?: boolean; + validateResponses?: boolean; + validateRequests?: boolean; multerOpts?: {}; } export class OpenApiValidator { private context: OpenApiContext; - private coerceTypes: boolean; - private multerOpts: {}; + private options: OpenApiValidatorOpts; constructor(options: OpenApiValidatorOpts) { - if (!options.apiSpecPath && !options.apiSpec) - throw ono('apiSpecPath or apiSpec required'); - if (options.apiSpecPath && options.apiSpec) - throw ono('apiSpecPath or apiSpec required. not both.'); + if (!options.apiSpec) throw ono('apiSpec required.'); + if (options.coerceTypes == null) options.coerceTypes = true; + if (options.validateRequests == null) options.validateRequests = true; - this.coerceTypes = options.coerceTypes == null ? true : options.coerceTypes; - this.multerOpts = options.multerOpts; + this.options = options; const openApiContext = new OpenApiContext({ - apiDoc: options.apiSpecPath || options.apiSpec, + apiDoc: options.apiSpec, }); this.context = openApiContext; @@ -52,9 +50,10 @@ export class OpenApiValidator { }); } + const coerceTypes = this.options.coerceTypes; const aoav = new middlewares.RequestValidator(this.context.apiDoc, { nullable: true, - coerceTypes: this.coerceTypes, + coerceTypes, removeAdditional: false, useDefaults: true, }); @@ -64,14 +63,16 @@ export class OpenApiValidator { }; const resOav = new middlewares.ResponseValidator(this.context.apiDoc, { - coerceTypes: this.coerceTypes, + coerceTypes, }); - app.use( + const use = [ middlewares.applyOpenApiMetadata(this.context), - middlewares.multipart(this.context, this.multerOpts), - validateMiddleware, - resOav.validate(), - ); + middlewares.multipart(this.context, this.options.multerOpts), + ]; + if (this.options.validateRequests) use.push(validateMiddleware); + if (this.options.validateResponses) use.push(resOav.validate()); + + app.use(use); } } diff --git a/src/middlewares/openapi.response.validator.ts b/src/middlewares/openapi.response.validator.ts index 2ed62d1d..e3691c56 100644 --- a/src/middlewares/openapi.response.validator.ts +++ b/src/middlewares/openapi.response.validator.ts @@ -11,10 +11,14 @@ export class ResponseValidator { private ajv; private spec; private validatorsCache = {}; - private useCache: boolean; + constructor(openApiSpec, options: any = {}) { this.spec = openApiSpec; this.ajv = createResponseAjv(openApiSpec, options); + (mung).onError = function(err, req, res) { + // monkey patch mung to rethrow exception + throw err; + }; } validate() { @@ -47,10 +51,7 @@ export class ResponseValidator { } _validate({ validators, body, statusCode }) { - // TODO build validators should be cached per endpoint - // const validators: any = this.buildValidators(responses); - - // find a response by status code or 'default' + // find the validator for the 'status code' e.g 200, 2XX or 'default' let validator; const status = statusCode; if (status) { @@ -65,7 +66,7 @@ export class ResponseValidator { } if (!validator) { - console.log('no validator found'); + console.warn('no validator found'); // assume valid return; } @@ -75,9 +76,8 @@ export class ResponseValidator { if (!valid) { const errors = validator.errors; - console.log(errors); const message = this.ajv.errorsText(errors, { - dataVar: 'request', + dataVar: '', // responses }); throw ono(ajvErrorsToValidatorError(500, errors), message); } diff --git a/test/common/app.ts b/test/common/app.ts index bef7e33e..0b1eb3a2 100644 --- a/test/common/app.ts +++ b/test/common/app.ts @@ -23,15 +23,15 @@ export async function createApp( new OpenApiValidator(opts).install(app); - if (useRoutes) routes(app); - - // Register error handler - app.use((err, req, res, next) => { - console.error(err); - res.status(err.status || 500).json({ - errors: err.errors, + if (useRoutes) { + routes(app); + // Register error handler + app.use((err, req, res, next) => { + res.status(err.status || 500).json({ + errors: err.errors, + }); }); - }); + } const server = await startServer(app, port); const shutDown = () => { diff --git a/test/middleware.spec.ts b/test/middleware.spec.ts index a20f364e..1dfc05b7 100644 --- a/test/middleware.spec.ts +++ b/test/middleware.spec.ts @@ -16,7 +16,7 @@ describe(packageJson.name, () => { it('should throw when spec is missing', async () => { const createMiddleware = () => new OpenApiValidator({ - apiSpecPath: './not-found.yaml', + apiSpec: './not-found.yaml', }); expect(createMiddleware).to.throw( diff --git a/test/query.params.spec.ts b/test/query.params.spec.ts index 97ca4290..89cd3af2 100644 --- a/test/query.params.spec.ts +++ b/test/query.params.spec.ts @@ -49,7 +49,6 @@ describe(packageJson.name, () => { }) .expect(400) .then(r => { - console.log(r.body); expect(r.body.errors).to.be.an('array'); })); }); diff --git a/test/resources/response.validation.yaml b/test/resources/response.validation.yaml index 176aae3e..7437c443 100644 --- a/test/resources/response.validation.yaml +++ b/test/resources/response.validation.yaml @@ -20,6 +20,11 @@ paths: get: description: find pets operationId: findPets + parameters: + - name: mode + in: query + schema: + type: string responses: '200': description: pet response diff --git a/test/response.validation.spec.ts b/test/response.validation.spec.ts index b952b4fd..80746715 100644 --- a/test/response.validation.spec.ts +++ b/test/response.validation.spec.ts @@ -1,15 +1,11 @@ import * as path from 'path'; -import * as fs from 'fs'; import * as express from 'express'; -import * as jsyaml from 'js-yaml'; import { expect } from 'chai'; import * as request from 'supertest'; -import { ResponseValidator } from '../src/middlewares/openapi.response.validator'; import { createApp } from './common/app'; const packageJson = require('../package.json'); const apiSpecPath = path.join('test', 'resources', 'response.validation.yaml'); -const apiSpec = jsyaml.safeLoad(fs.readFileSync(apiSpecPath, 'utf8')); describe(packageJson.name, () => { let app = null; @@ -17,151 +13,40 @@ describe(packageJson.name, () => { before(async () => { // Set up the express app - app = await createApp({ apiSpec, coerceTypes: false }, 3005, false); - basePath = app.basePath; - app.use( - `${basePath}`, - express.Router().get(`/pets/rejectadditionalProps`, (req, res) => - res.json([ - { - id: 1, - type: 'test', - tag: 'woah', - }, - ]), - ), + app = await createApp( + { apiSpec: apiSpecPath, validateResponses: true }, + 3005, + false, ); + basePath = app.basePath; + app.get(`${basePath}/pets`, (req, res) => { + let json = {}; + if ((req.query.mode = 'bad_type')) { + json = [{ id: 'bad_id', name: 'name', tag: 'tag' }]; + } + return res.json(json); + }); + // Register error handler + app.use((err, req, res, next) => { + res.status(err.status || 500).json({ + message: err.message, + code: err.status, + }); + }); }); after(() => { app.server.close(); }); - it('should fail if response does not match', async () => + it('should fail if response field has a value of incorrect type', async () => request(app) - .get(`${basePath}/rejectadditionalProps`) + .get(`${basePath}/pets?mode=bad_type`) .expect(500) .then((r: any) => { - expect(r.body.error).to.be.not.null; + expect(r.body.message).to.contain('should be integer'); + expect(r.body) + .to.have.property('code') + .that.equals(500); })); - - // TODO - it.only('should always return valid for non-JSON responses', async () => { - const v = new ResponseValidator(apiSpec); - const responses = petsResponseSchema(); - const statusCode = 200; - - try { - const validators = v._getOrBuildValidator(null, responses); - v._validate({ - validators, - body: { id: 1, name: 'test', tag: 'tag' }, - statusCode, - }); - } catch (e) { - expect(e).to.be.not.null; - expect(e.message).to.contain('should be array'); - } - }); - - // TODO - it.only('should validate the error object', async () => {}); - - it.only('should return an error if field type is invalid', async () => { - const v = new ResponseValidator(apiSpec); - const responses = petsResponseSchema(); - const statusCode = 200; - - const validators = v._getOrBuildValidator(null, responses); - try { - v._validate({ - validators, - body: [{ id: 'bad-id', name: 'test', tag: 'tag' }], - statusCode, - }); - } catch (e) { - expect(e).to.be.not.null; - expect(e.message).to.contain('should be integer'); - expect(e.message).to.not.contain('additional properties'); - } - - try { - v._validate({ - validators, - body: { id: 1, name: 'test', tag: 'tag' }, - statusCode, - }); - } catch (e) { - expect(e).to.be.not.null; - expect(e.message).to.contain('should be array'); - } - - try { - v._validate({ - validators, - body: [{ id: 1, name: [], tag: 'tag' }], - statusCode, - }); - } catch (e) { - expect(e).to.be.not.null; - expect(e.message).to.contain('should be string'); - } - }); - - // TODO may not be possible to fix - // https://github.com/epoberezkin/ajv/issues/837 - it.skip('should if additional properties are provided when set false', async () => { - const v = new ResponseValidator(apiSpec); - const responses = petsResponseSchema(); - const statusCode = 200; - const body = [ - { - id: 10, - name: 'test', - tag: 'tag', - additionalProp: 'test', - }, - ]; - try { - const validators = v._getOrBuildValidator(null, responses); - v._validate({ validators, body, statusCode }); - expect('here').to.be.null; - } catch (e) { - // TODO include params.additionalProperty: value in error message - // TODO maybe params should be in the response - expect(e.message).to.contain('should NOT have additional properties'); - expect(e.status).to.equal(500); - expect(e.errors[0].message).to.contain( - 'should NOT have additional properties', - ); - } - }); }); - -function petsResponseSchema() { - return { - '200': { - description: 'pet response', - content: { - 'application/json': { - schema: { - type: 'array', - items: { - $ref: '#/components/schemas/Pet', - }, - }, - }, - }, - }, - default: { - description: 'unexpected error', - content: { - 'application/json': { - schema: { - $ref: '#/components/schemas/Error', - }, - }, - }, - }, - }; -} diff --git a/test/response.validator.spec.ts b/test/response.validator.spec.ts new file mode 100644 index 00000000..e97bc804 --- /dev/null +++ b/test/response.validator.spec.ts @@ -0,0 +1,151 @@ +import * as path from 'path'; +import * as fs from 'fs'; +import * as jsyaml from 'js-yaml'; +import { expect } from 'chai'; +import { ResponseValidator } from '../src/middlewares/openapi.response.validator'; + +const packageJson = require('../package.json'); +const apiSpecPath = path.join('test', 'resources', 'response.validation.yaml'); +const apiSpec = jsyaml.safeLoad(fs.readFileSync(apiSpecPath, 'utf8')); + +describe(packageJson.name, () => { + // TODO + it.skip('should always return valid for non-JSON responses', async () => {}); + + it('should validate the using default (in this case the error object)', async () => { + const v = new ResponseValidator(apiSpec); + const responses = petsResponseSchema(); + const validators = v._getOrBuildValidator(null, responses); + + try { + expect( + v._validate({ + validators, + body: { message: 'some error message', code: 400 }, + statusCode: 400, + }), + ).to.not.exist; + } catch (e) { + expect(e).to.not.exist; + } + }); + + it('should throw error when default response is invalid', async () => { + const v = new ResponseValidator(apiSpec); + const responses = petsResponseSchema(); + const validators = v._getOrBuildValidator(null, responses); + + try { + const message = { note: 'bad message type' }; + const code = 400; + expect( + v._validate({ + validators, + body: { message, code }, + statusCode: code, + }), + ).to.not.exist; + } catch (e) { + expect(e.status).to.equal(500); + expect(e.errors).to.be.an('array'); + expect(e.errors[0].message).to.equal('should be string'); + } + }); + + it('should return an error if field type is invalid', async () => { + const v = new ResponseValidator(apiSpec); + const responses = petsResponseSchema(); + const validators = v._getOrBuildValidator(null, responses); + + try { + v._validate({ + validators, + body: [{ id: 'bad-id', name: 'test', tag: 'tag' }], + statusCode: 200, + }); + } catch (e) { + expect(e).to.be.not.null; + expect(e.message).to.contain('should be integer'); + expect(e.message).to.not.contain('additional properties'); + } + + try { + v._validate({ + validators, + body: { id: 1, name: 'test', tag: 'tag' }, + statusCode: 200, + }); + } catch (e) { + expect(e).to.be.not.null; + expect(e.message).to.contain('should be array'); + } + + try { + v._validate({ + validators, + body: [{ id: 1, name: [], tag: 'tag' }], + statusCode: 200, + }); + } catch (e) { + expect(e).to.be.not.null; + expect(e.message).to.contain('should be string'); + } + }); + + // TODO may not be possible to fix + // https://github.com/epoberezkin/ajv/issues/837 + it.skip('should if additional properties are provided when set false', async () => { + const v = new ResponseValidator(apiSpec); + const responses = petsResponseSchema(); + const validators = v._getOrBuildValidator(null, responses); + const statusCode = 200; + const body = [ + { + id: 10, + name: 'test', + tag: 'tag', + additionalProp: 'test', + }, + ]; + try { + expect(v._validate({ validators, body, statusCode })).to.not.exist; + expect('here').to.be.null; + } catch (e) { + // TODO include params.additionalProperty: value in error message + // TODO maybe params should be in the response + expect(e.message).to.contain('should NOT have additional properties'); + expect(e.status).to.equal(500); + expect(e.errors[0].message).to.contain( + 'should NOT have additional properties', + ); + } + }); +}); + +function petsResponseSchema() { + return { + '200': { + description: 'pet response', + content: { + 'application/json': { + schema: { + type: 'array', + items: { + $ref: '#/components/schemas/Pet', + }, + }, + }, + }, + }, + '400': { + description: 'unexpected error', + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Error', + }, + }, + }, + }, + }; +}