From b6aedf0e9258b62a63744b441670f9b99ab5c955 Mon Sep 17 00:00:00 2001 From: Vincenzo Chianese Date: Tue, 19 Nov 2019 13:04:29 +0100 Subject: [PATCH 1/3] feat: handle allowedParams in ajv --- .../__tests__/body-params-validation.spec.ts | 4 +- .../callback/__tests__/callbacks.spec.ts | 100 ++++++++++++------ .../__snapshots__/utils.spec.ts.snap | 2 +- .../validators/__tests__/utils.spec.ts | 11 +- .../http/src/validator/validators/utils.ts | 19 ++-- 5 files changed, 88 insertions(+), 48 deletions(-) diff --git a/packages/http-server/src/__tests__/body-params-validation.spec.ts b/packages/http-server/src/__tests__/body-params-validation.spec.ts index cfe662b73..0eda47a51 100644 --- a/packages/http-server/src/__tests__/body-params-validation.spec.ts +++ b/packages/http-server/src/__tests__/body-params-validation.spec.ts @@ -368,7 +368,7 @@ describe('body params validation', () => { { code: 'enum', location: ['body', 'status'], - message: 'should be equal to one of the allowed values', + message: 'should be equal to one of the allowed values: placed, approved, delivered', severity: 'Error', }, ], @@ -495,7 +495,7 @@ describe('body params validation', () => { location: ['body', 'status'], severity: 'Error', code: 'enum', - message: 'should be equal to one of the allowed values', + message: 'should be equal to one of the allowed values: open, close', }, ], }); diff --git a/packages/http/src/mocker/callback/__tests__/callbacks.spec.ts b/packages/http/src/mocker/callback/__tests__/callbacks.spec.ts index 39c5dab1d..4b1514fa7 100644 --- a/packages/http/src/mocker/callback/__tests__/callbacks.spec.ts +++ b/packages/http/src/mocker/callback/__tests__/callbacks.spec.ts @@ -12,7 +12,6 @@ describe('runCallback()', () => { info: jest.fn(), }; - beforeEach(() => { jest.clearAllMocks(); }); @@ -32,15 +31,17 @@ describe('runCallback()', () => { method: 'get', path: 'http://example.com/{$method}/{$statusCode}/{$response.body#/id}/{$request.header.content-type}', id: '1', - responses: [{ code: '200', contents: [ { mediaType: 'application/json' } ] }], + responses: [{ code: '200', contents: [{ mediaType: 'application/json' }] }], request: { body: { - contents: [{ - mediaType: 'application/json', - examples: [{ key: 'e1', value: { about: 'something' }}] - }], + contents: [ + { + mediaType: 'application/json', + examples: [{ key: 'e1', value: { about: 'something' } }], + }, + ], }, - } + }, }, request: { body: '', @@ -52,12 +53,20 @@ describe('runCallback()', () => { }, response: { statusCode: 200, - body: { id: 5 } + body: { id: 5 }, }, })(logger)(); - expect(fetch).toHaveBeenCalledWith('http://example.com/get/200/5/weird/content', { method: 'get', body: '{"about":"something"}', headers: { 'content-type': 'application/json' } }); - expect(logger.info).toHaveBeenNthCalledWith(1, { name: 'CALLBACK' }, 'test callback: Making request to http://example.com/get/200/5/weird/content...'); + expect(fetch).toHaveBeenCalledWith('http://example.com/get/200/5/weird/content', { + method: 'get', + body: '{"about":"something"}', + headers: { 'content-type': 'application/json' }, + }); + expect(logger.info).toHaveBeenNthCalledWith( + 1, + { name: 'CALLBACK' }, + 'test callback: Making request to http://example.com/get/200/5/weird/content...' + ); expect(logger.info).toHaveBeenNthCalledWith(2, { name: 'CALLBACK' }, 'test callback: Request finished'); expect(logger.error).not.toHaveBeenCalled(); expect(logger.warn).not.toHaveBeenCalled(); @@ -66,7 +75,7 @@ describe('runCallback()', () => { describe('callback response is incorrect', () => { it('logs violations', async () => { - const headers = { 'content-type': 'application/json', 'test': 'test' }; + const headers = { 'content-type': 'application/json', test: 'test' }; ((fetch as unknown) as jest.Mock).mockResolvedValue({ status: 200, headers: { get: (n: string) => headers[n], raw: () => mapValues(headers, (h: string) => h.split(' ')) }, @@ -79,24 +88,35 @@ describe('runCallback()', () => { method: 'get', path: 'http://example.com/{$method}/{$statusCode}/{$response.body#/id}/{$request.header.content-type}', id: '1', - responses: [{ - code: '200', - headers: [ - { name: 'test', style: HttpParamStyles.Simple, deprecated: true, schema: { type: 'string', enum: ['a'] } }, - ], - contents: [{ - mediaType: 'application/json', - schema: { type: 'object', properties: { test: { type: 'string', maxLength: 3, } } }, - }], - }], + responses: [ + { + code: '200', + headers: [ + { + name: 'test', + style: HttpParamStyles.Simple, + deprecated: true, + schema: { type: 'string', enum: ['a'] }, + }, + ], + contents: [ + { + mediaType: 'application/json', + schema: { type: 'object', properties: { test: { type: 'string', maxLength: 3 } } }, + }, + ], + }, + ], request: { body: { - contents: [{ - mediaType: 'application/json', - examples: [{ key: 'e1', value: { about: 'something' }}], - }], + contents: [ + { + mediaType: 'application/json', + examples: [{ key: 'e1', value: { about: 'something' } }], + }, + ], }, - } + }, }, request: { body: '', @@ -108,14 +128,30 @@ describe('runCallback()', () => { }, response: { statusCode: 200, - body: { id: 5 } + body: { id: 5 }, }, })(logger)(); - expect(fetch).toHaveBeenCalledWith('http://example.com/get/200/5/weird/content', { method: 'get', body: '{"about":"something"}', headers: { 'content-type': 'application/json' } }); - expect(logger.warn).toHaveBeenNthCalledWith(1, { name: 'VALIDATOR' }, 'Violation: header.test Header param test is deprecated'); - expect(logger.error).toHaveBeenNthCalledWith(1, { name: 'VALIDATOR' }, 'Violation: body.test should NOT be longer than 3 characters'); - expect(logger.error).toHaveBeenNthCalledWith(2, { name: 'VALIDATOR' }, 'Violation: header.test should be equal to one of the allowed values'); + expect(fetch).toHaveBeenCalledWith('http://example.com/get/200/5/weird/content', { + method: 'get', + body: '{"about":"something"}', + headers: { 'content-type': 'application/json' }, + }); + expect(logger.warn).toHaveBeenNthCalledWith( + 1, + { name: 'VALIDATOR' }, + 'Violation: header.test Header param test is deprecated' + ); + expect(logger.error).toHaveBeenNthCalledWith( + 1, + { name: 'VALIDATOR' }, + 'Violation: body.test should NOT be longer than 3 characters' + ); + expect(logger.error).toHaveBeenNthCalledWith( + 2, + { name: 'VALIDATOR' }, + 'Violation: header.test should be equal to one of the allowed values: a' + ); }); }); @@ -146,7 +182,7 @@ describe('runCallback()', () => { }, response: { statusCode: 200, - body: { id: 5 } + body: { id: 5 }, }, })(logger)(); diff --git a/packages/http/src/validator/validators/__tests__/__snapshots__/utils.spec.ts.snap b/packages/http/src/validator/validators/__tests__/__snapshots__/utils.spec.ts.snap index 7edf60019..6ce2ceef9 100644 --- a/packages/http/src/validator/validators/__tests__/__snapshots__/utils.spec.ts.snap +++ b/packages/http/src/validator/validators/__tests__/__snapshots__/utils.spec.ts.snap @@ -32,7 +32,7 @@ exports[`convertAjvErrors() message field is missing converts properly 1`] = ` Array [ Object { "code": "required", - "message": "", + "message": "undefined", "path": Array [ "b", ], diff --git a/packages/http/src/validator/validators/__tests__/utils.spec.ts b/packages/http/src/validator/validators/__tests__/utils.spec.ts index 19eda6fe9..89c344871 100644 --- a/packages/http/src/validator/validators/__tests__/utils.spec.ts +++ b/packages/http/src/validator/validators/__tests__/utils.spec.ts @@ -1,14 +1,15 @@ import { DiagnosticSeverity } from '@stoplight/types'; import * as convertAjvErrorsModule from '../utils'; import { convertAjvErrors, validateAgainstSchema } from '../utils'; +import { ErrorObject } from 'ajv'; describe('convertAjvErrors()', () => { - const errorObjectFixture = { + const errorObjectFixture: ErrorObject = { dataPath: 'a.b', keyword: 'required', message: 'c is required', schemaPath: '..', - params: '', + params: {}, }; describe('all fields defined', () => { @@ -20,7 +21,7 @@ describe('convertAjvErrors()', () => { describe('keyword field is missing', () => { it('converts properly', () => { expect( - convertAjvErrors([Object.assign({}, errorObjectFixture, { keyword: undefined })], DiagnosticSeverity.Error), + convertAjvErrors([Object.assign({}, errorObjectFixture, { keyword: undefined })], DiagnosticSeverity.Error) ).toMatchSnapshot(); }); }); @@ -28,7 +29,7 @@ describe('convertAjvErrors()', () => { describe('message field is missing', () => { it('converts properly', () => { expect( - convertAjvErrors([Object.assign({}, errorObjectFixture, { message: undefined })], DiagnosticSeverity.Error), + convertAjvErrors([Object.assign({}, errorObjectFixture, { message: undefined })], DiagnosticSeverity.Error) ).toMatchSnapshot(); }); }); @@ -75,7 +76,7 @@ describe('validateAgainstSchema()', () => { schemaPath: '#/type', }, ], - DiagnosticSeverity.Error, + DiagnosticSeverity.Error ); }); }); diff --git a/packages/http/src/validator/validators/utils.ts b/packages/http/src/validator/validators/utils.ts index c69b75b9f..37e7c21fe 100644 --- a/packages/http/src/validator/validators/utils.ts +++ b/packages/http/src/validator/validators/utils.ts @@ -6,22 +6,25 @@ import { option } from 'fp-ts/lib/Option'; import { sequenceT } from 'fp-ts/lib/Apply'; import * as Ajv from 'ajv'; import { JSONSchema } from '../../'; -// @ts-ignore import * as AjvOAI from 'ajv-oai'; -const ajv = new AjvOAI({ allErrors: true, messages: true, schemaId: 'auto' }) as Ajv.Ajv; +const ajv = new AjvOAI({ allErrors: true, messages: true, schemaId: 'auto' }); export const convertAjvErrors = (errors: Ajv.ErrorObject[] | undefined | null, severity: DiagnosticSeverity) => { if (!errors) { return []; } - return errors.map(error => ({ - path: error.dataPath.split('.').slice(1), - code: error.keyword || '', - message: error.message || '', - severity, - })); + return errors.map(error => { + const allowedParameters = 'allowedValues' in error.params ? `: ${error.params.allowedValues.join(', ')}` : ''; + + return { + path: error.dataPath.split('.').slice(1), + code: error.keyword || '', + message: `${error.message}${allowedParameters}` || '', + severity, + }; + }); }; export const validateAgainstSchema = (value: any, schema: JSONSchema, prefix?: string): IPrismDiagnostic[] => { From 2ba9889f64c11be5eb21a73236cced33e8036621 Mon Sep 17 00:00:00 2001 From: Vincenzo Chianese Date: Tue, 19 Nov 2019 13:09:51 +0100 Subject: [PATCH 2/3] test: fix --- CHANGELOG.md | 1 + .../validators/__tests__/__snapshots__/utils.spec.ts.snap | 2 +- packages/http/src/validator/validators/utils.ts | 2 +- .../validate-body-params/form-data-invalid-request.oas2.txt | 2 +- .../validate-body-params/form-data-invalid-request.oas3.txt | 2 +- .../specs/validate-body-params/json-invalid-request-2.oas2.txt | 2 +- .../specs/validate-body-params/json-invalid-request-2.oas3.txt | 2 +- .../specs/violations/value-not-in-enum.httpResponse.txt | 2 +- 8 files changed, 8 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e25e7c87e..e48b2ee51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Prism has values for path/query params bolded and in color [#743](https://github.com/stoplightio/prism/pull/743) - The CLI now displays a timestamp for all the logged operations [#779](https://github.com/stoplightio/prism/pull/779) - Prism has now support for OpenAPI 3.0 callbacks [#716](https://github.com/stoplightio/prism/pull/716) +- Prism body validator will now show allowed enum parameters in error messages [#828](https://github.com/stoplightio/prism/pull/828) ## Fixed diff --git a/packages/http/src/validator/validators/__tests__/__snapshots__/utils.spec.ts.snap b/packages/http/src/validator/validators/__tests__/__snapshots__/utils.spec.ts.snap index 6ce2ceef9..7edf60019 100644 --- a/packages/http/src/validator/validators/__tests__/__snapshots__/utils.spec.ts.snap +++ b/packages/http/src/validator/validators/__tests__/__snapshots__/utils.spec.ts.snap @@ -32,7 +32,7 @@ exports[`convertAjvErrors() message field is missing converts properly 1`] = ` Array [ Object { "code": "required", - "message": "undefined", + "message": "", "path": Array [ "b", ], diff --git a/packages/http/src/validator/validators/utils.ts b/packages/http/src/validator/validators/utils.ts index 37e7c21fe..4f6cce87e 100644 --- a/packages/http/src/validator/validators/utils.ts +++ b/packages/http/src/validator/validators/utils.ts @@ -21,7 +21,7 @@ export const convertAjvErrors = (errors: Ajv.ErrorObject[] | undefined | null, s return { path: error.dataPath.split('.').slice(1), code: error.keyword || '', - message: `${error.message}${allowedParameters}` || '', + message: `${error.message || ''}${allowedParameters || ''}`, severity, }; }); diff --git a/test-harness/specs/validate-body-params/form-data-invalid-request.oas2.txt b/test-harness/specs/validate-body-params/form-data-invalid-request.oas2.txt index 8a1b26620..c5b1a29e7 100644 --- a/test-harness/specs/validate-body-params/form-data-invalid-request.oas2.txt +++ b/test-harness/specs/validate-body-params/form-data-invalid-request.oas2.txt @@ -33,4 +33,4 @@ HTTP/1.1 422 Unprocessable Entity content-type: application/problem+json Connection: keep-alive -{"type":"https://stoplight.io/prism/errors#UNPROCESSABLE_ENTITY","title":"Invalid request body payload","status":422,"detail":"Your request is not valid and no HTTP validation response was found in the spec, so Prism is generating this error for you.","validation":[{"location":["body"],"severity":"Error","code":"required","message":"should have required property 'id'"},{"location":["body","status"],"severity":"Error","code":"enum","message":"should be equal to one of the allowed values"}]} +{"type":"https://stoplight.io/prism/errors#UNPROCESSABLE_ENTITY","title":"Invalid request body payload","status":422,"detail":"Your request is not valid and no HTTP validation response was found in the spec, so Prism is generating this error for you.","validation":[{"location":["body"],"severity":"Error","code":"required","message":"should have required property 'id'"},{"location":["body","status"],"severity":"Error","code":"enum","message":"should be equal to one of the allowed values: open, close"}]} diff --git a/test-harness/specs/validate-body-params/form-data-invalid-request.oas3.txt b/test-harness/specs/validate-body-params/form-data-invalid-request.oas3.txt index 1fdbc59fa..14e9c8f1a 100644 --- a/test-harness/specs/validate-body-params/form-data-invalid-request.oas3.txt +++ b/test-harness/specs/validate-body-params/form-data-invalid-request.oas3.txt @@ -36,4 +36,4 @@ HTTP/1.1 422 Unprocessable Entity content-type: application/problem+json Connection: keep-alive -{"type":"https://stoplight.io/prism/errors#UNPROCESSABLE_ENTITY","title":"Invalid request body payload","status":422,"detail":"Your request is not valid and no HTTP validation response was found in the spec, so Prism is generating this error for you.","validation":[{"location":["body"],"severity":"Error","code":"required","message":"should have required property 'id'"},{"location":["body","status"],"severity":"Error","code":"enum","message":"should be equal to one of the allowed values"}]} +{"type":"https://stoplight.io/prism/errors#UNPROCESSABLE_ENTITY","title":"Invalid request body payload","status":422,"detail":"Your request is not valid and no HTTP validation response was found in the spec, so Prism is generating this error for you.","validation":[{"location":["body"],"severity":"Error","code":"required","message":"should have required property 'id'"},{"location":["body","status"],"severity":"Error","code":"enum","message":"should be equal to one of the allowed values: open, close"}]} diff --git a/test-harness/specs/validate-body-params/json-invalid-request-2.oas2.txt b/test-harness/specs/validate-body-params/json-invalid-request-2.oas2.txt index 9ef36025b..389b55253 100644 --- a/test-harness/specs/validate-body-params/json-invalid-request-2.oas2.txt +++ b/test-harness/specs/validate-body-params/json-invalid-request-2.oas2.txt @@ -35,4 +35,4 @@ HTTP/1.1 422 Unprocessable Entity content-type: application/problem+json Connection: keep-alive -{"type":"https://stoplight.io/prism/errors#UNPROCESSABLE_ENTITY","title":"Invalid request body payload","status":422,"detail":"Your request is not valid and no HTTP validation response was found in the spec, so Prism is generating this error for you.","validation":[{"location":["body"],"severity":"Error","code":"required","message":"should have required property 'id'"},{"location":["body","status"],"severity":"Error","code":"enum","message":"should be equal to one of the allowed values"}]} +{"type":"https://stoplight.io/prism/errors#UNPROCESSABLE_ENTITY","title":"Invalid request body payload","status":422,"detail":"Your request is not valid and no HTTP validation response was found in the spec, so Prism is generating this error for you.","validation":[{"location":["body"],"severity":"Error","code":"required","message":"should have required property 'id'"},{"location":["body","status"],"severity":"Error","code":"enum","message":"should be equal to one of the allowed values: open, close"}]} diff --git a/test-harness/specs/validate-body-params/json-invalid-request-2.oas3.txt b/test-harness/specs/validate-body-params/json-invalid-request-2.oas3.txt index 9b4d3a89a..25b7e1aed 100644 --- a/test-harness/specs/validate-body-params/json-invalid-request-2.oas3.txt +++ b/test-harness/specs/validate-body-params/json-invalid-request-2.oas3.txt @@ -34,4 +34,4 @@ HTTP/1.1 422 Unprocessable Entity content-type: application/problem+json Connection: keep-alive -{"type":"https://stoplight.io/prism/errors#UNPROCESSABLE_ENTITY","title":"Invalid request body payload","status":422,"detail":"Your request is not valid and no HTTP validation response was found in the spec, so Prism is generating this error for you.","validation":[{"location":["body"],"severity":"Error","code":"required","message":"should have required property 'id'"},{"location":["body","status"],"severity":"Error","code":"enum","message":"should be equal to one of the allowed values"}]} +{"type":"https://stoplight.io/prism/errors#UNPROCESSABLE_ENTITY","title":"Invalid request body payload","status":422,"detail":"Your request is not valid and no HTTP validation response was found in the spec, so Prism is generating this error for you.","validation":[{"location":["body"],"severity":"Error","code":"required","message":"should have required property 'id'"},{"location":["body","status"],"severity":"Error","code":"enum","message":"should be equal to one of the allowed values: open, close"}]} diff --git a/test-harness/specs/violations/value-not-in-enum.httpResponse.txt b/test-harness/specs/violations/value-not-in-enum.httpResponse.txt index b026f10ac..df4e73319 100644 --- a/test-harness/specs/violations/value-not-in-enum.httpResponse.txt +++ b/test-harness/specs/violations/value-not-in-enum.httpResponse.txt @@ -30,4 +30,4 @@ curl -i http://localhost:4010/path ====expect==== HTTP/1.1 500 Internal Server Error -{"type":"https://stoplight.io/prism/errors#VIOLATIONS","title":"Request/Response not valid","status":500,"detail":"Your request/response is not valid and the --errors flag is set, so Prism is generating this error for you.","validation":[{"location":["response","body"],"severity":"Error","code":"enum","message":"should be equal to one of the allowed values"}]} +{"type":"https://stoplight.io/prism/errors#VIOLATIONS","title":"Request/Response not valid","status":500,"detail":"Your request/response is not valid and the --errors flag is set, so Prism is generating this error for you.","validation":[{"location":["response","body"],"severity":"Error","code":"enum","message":"should be equal to one of the allowed values: placed, approved, delivered"}]} From 99bb04263197f4ec2a57092a01a1cddf06056303 Mon Sep 17 00:00:00 2001 From: Vincenzo Chianese Date: Tue, 19 Nov 2019 13:14:17 +0100 Subject: [PATCH 3/3] test: simplify --- .../__tests__/__snapshots__/utils.spec.ts.snap | 13 ------------- .../validator/validators/__tests__/utils.spec.ts | 4 ++-- packages/http/src/validator/validators/utils.ts | 2 +- 3 files changed, 3 insertions(+), 16 deletions(-) diff --git a/packages/http/src/validator/validators/__tests__/__snapshots__/utils.spec.ts.snap b/packages/http/src/validator/validators/__tests__/__snapshots__/utils.spec.ts.snap index 7edf60019..c60314c98 100644 --- a/packages/http/src/validator/validators/__tests__/__snapshots__/utils.spec.ts.snap +++ b/packages/http/src/validator/validators/__tests__/__snapshots__/utils.spec.ts.snap @@ -28,19 +28,6 @@ Array [ ] `; -exports[`convertAjvErrors() message field is missing converts properly 1`] = ` -Array [ - Object { - "code": "required", - "message": "", - "path": Array [ - "b", - ], - "severity": 0, - }, -] -`; - exports[`validateAgainstSchema() has validation errors returns validation errors 1`] = ` Array [ Object { diff --git a/packages/http/src/validator/validators/__tests__/utils.spec.ts b/packages/http/src/validator/validators/__tests__/utils.spec.ts index 89c344871..45fe628cc 100644 --- a/packages/http/src/validator/validators/__tests__/utils.spec.ts +++ b/packages/http/src/validator/validators/__tests__/utils.spec.ts @@ -29,8 +29,8 @@ describe('convertAjvErrors()', () => { describe('message field is missing', () => { it('converts properly', () => { expect( - convertAjvErrors([Object.assign({}, errorObjectFixture, { message: undefined })], DiagnosticSeverity.Error) - ).toMatchSnapshot(); + convertAjvErrors([Object.assign({}, errorObjectFixture, { message: undefined })], DiagnosticSeverity.Error)[0] + ).toHaveProperty('message', ''); }); }); diff --git a/packages/http/src/validator/validators/utils.ts b/packages/http/src/validator/validators/utils.ts index 4f6cce87e..00a0b2495 100644 --- a/packages/http/src/validator/validators/utils.ts +++ b/packages/http/src/validator/validators/utils.ts @@ -21,7 +21,7 @@ export const convertAjvErrors = (errors: Ajv.ErrorObject[] | undefined | null, s return { path: error.dataPath.split('.').slice(1), code: error.keyword || '', - message: `${error.message || ''}${allowedParameters || ''}`, + message: `${error.message || ''}${allowedParameters}`, severity, }; });