From 1578ecff3be3e1635705f8763d7601e6b81a013b Mon Sep 17 00:00:00 2001 From: jannyHou Date: Fri, 8 Jun 2018 11:40:13 -0400 Subject: [PATCH] fix: apply feedback --- .../rest/src/coercion/coerce-parameter.ts | 4 +- .../rest/src/coercion/http-error-message.ts | 16 +++--- packages/rest/src/coercion/validator.ts | 4 +- .../coercion/paramStringToBoolean.unit.ts | 6 +- .../unit/coercion/paramStringToBuffer.unit.ts | 2 +- .../unit/coercion/paramStringToDate.unit.ts | 2 +- .../coercion/paramStringToInteger.unit.ts | 4 +- .../unit/coercion/paramStringToNumber.unit.ts | 55 +++++++------------ 8 files changed, 39 insertions(+), 54 deletions(-) diff --git a/packages/rest/src/coercion/coerce-parameter.ts b/packages/rest/src/coercion/coerce-parameter.ts index ae6e489d880e..cb2d93f83b18 100644 --- a/packages/rest/src/coercion/coerce-parameter.ts +++ b/packages/rest/src/coercion/coerce-parameter.ts @@ -50,9 +50,7 @@ export function coerceParameter(data: string, spec: ParameterObject) { coercedResult = data ? Number(data) : undefined; if (coercedResult === undefined) break; if (isNaN(coercedResult)) - throw new HttpErrors['400']( - HttpErrorMessage.INVALID_DATA(data, spec.name), - ); + throw HttpErrorMessage.invalidData(data, spec.name); break; case 'long': coercedResult = Number(data); diff --git a/packages/rest/src/coercion/http-error-message.ts b/packages/rest/src/coercion/http-error-message.ts index 6d3faeb2bba6..a1d1c43fa7d0 100644 --- a/packages/rest/src/coercion/http-error-message.ts +++ b/packages/rest/src/coercion/http-error-message.ts @@ -1,9 +1,11 @@ -// tslint:disable:no-any +import * as HttpErrors from 'http-errors'; export namespace HttpErrorMessage { - export const INVALID_DATA = (data: any, name: any) => { - return `Invalid data ${data} of parameter ${name}!`; - }; - export const MISSING_REQUIRED = (name: any) => { - return `Required parameter ${name} is missing!`; - }; + export function invalidData(data: T, name: string) { + const msg = `Invalid data ${JSON.stringify(data)} for parameter ${name}!`; + return new HttpErrors.BadRequest(msg); + } + export function missingRequired(name: string): HttpErrors.HttpError { + const msg = `Required parameter ${name} is missing!`; + return new HttpErrors.BadRequest(msg); + } } diff --git a/packages/rest/src/coercion/validator.ts b/packages/rest/src/coercion/validator.ts index be1a68128cd0..e4f498a370fc 100644 --- a/packages/rest/src/coercion/validator.ts +++ b/packages/rest/src/coercion/validator.ts @@ -43,7 +43,7 @@ export class Validator { if (this.isAbsent(value)) { if (this.isRequired(opts)) { const name = this.ctx.parameterSpec.name; - throw new HttpErrors['400'](HttpErrorMessage.MISSING_REQUIRED(name)); + throw HttpErrorMessage.missingRequired(name); } else { return; } @@ -68,6 +68,6 @@ export class Validator { */ // tslint:disable-next-line:no-any isAbsent(value: any) { - return [''].includes(value); + return value === ''; } } diff --git a/packages/rest/test/unit/coercion/paramStringToBoolean.unit.ts b/packages/rest/test/unit/coercion/paramStringToBoolean.unit.ts index 23ba1aa1db49..2121e197be2c 100644 --- a/packages/rest/test/unit/coercion/paramStringToBoolean.unit.ts +++ b/packages/rest/test/unit/coercion/paramStringToBoolean.unit.ts @@ -13,7 +13,7 @@ const BOOLEAN_PARAM = { }; describe('coerce param from string to boolean', () => { - test(BOOLEAN_PARAM, 'false', false); - test(BOOLEAN_PARAM, 'true', true); - test(BOOLEAN_PARAM, undefined, undefined); + test(BOOLEAN_PARAM, 'false', false); + test(BOOLEAN_PARAM, 'true', true); + test(BOOLEAN_PARAM, undefined, undefined); }); diff --git a/packages/rest/test/unit/coercion/paramStringToBuffer.unit.ts b/packages/rest/test/unit/coercion/paramStringToBuffer.unit.ts index d98e25c3eefd..967b02176f0c 100644 --- a/packages/rest/test/unit/coercion/paramStringToBuffer.unit.ts +++ b/packages/rest/test/unit/coercion/paramStringToBuffer.unit.ts @@ -17,7 +17,7 @@ describe('coerce param from string to buffer', () => { base64: Buffer.from('Hello World').toString('base64'), }; - test( + test( BUFFER_PARAM, testValues.base64, Buffer.from(testValues.base64, 'base64'), diff --git a/packages/rest/test/unit/coercion/paramStringToDate.unit.ts b/packages/rest/test/unit/coercion/paramStringToDate.unit.ts index d185c4cd7930..c787510c4818 100644 --- a/packages/rest/test/unit/coercion/paramStringToDate.unit.ts +++ b/packages/rest/test/unit/coercion/paramStringToDate.unit.ts @@ -13,5 +13,5 @@ const DATE_PARAM = { }; describe('coerce param from string to date', () => { - test(DATE_PARAM, '2015-03-01', new Date('2015-03-01')); + test(DATE_PARAM, '2015-03-01', new Date('2015-03-01')); }); diff --git a/packages/rest/test/unit/coercion/paramStringToInteger.unit.ts b/packages/rest/test/unit/coercion/paramStringToInteger.unit.ts index d279b101081f..1eb692f824f7 100644 --- a/packages/rest/test/unit/coercion/paramStringToInteger.unit.ts +++ b/packages/rest/test/unit/coercion/paramStringToInteger.unit.ts @@ -19,6 +19,6 @@ const INT64_PARAM = { }; describe('coerce param from string to integer', () => { - test(INT32_PARAM, '100', 100); - test(INT64_PARAM, '9223372036854775807', 9223372036854775807); + test(INT32_PARAM, '100', 100); + test(INT64_PARAM, '9223372036854775807', 9223372036854775807); }); diff --git a/packages/rest/test/unit/coercion/paramStringToNumber.unit.ts b/packages/rest/test/unit/coercion/paramStringToNumber.unit.ts index f6b7935a9812..77bdd109f7f5 100644 --- a/packages/rest/test/unit/coercion/paramStringToNumber.unit.ts +++ b/packages/rest/test/unit/coercion/paramStringToNumber.unit.ts @@ -39,71 +39,56 @@ const DOUBLE_PARAM = { }, }; -/*tslint:disable:max-line-length*/ describe('coerce param from string to number - required', () => { context('valid values', () => { - test(REQUIRED_NUMBER_PARAM, '0', 0); - test(REQUIRED_NUMBER_PARAM, '1', 1); - test(REQUIRED_NUMBER_PARAM, '-1', -1); + test(REQUIRED_NUMBER_PARAM, '0', 0); + test(REQUIRED_NUMBER_PARAM, '1', 1); + test(REQUIRED_NUMBER_PARAM, '-1', -1); }); context('empty values trigger ERROR_BAD_REQUEST', () => { // null, '' sent from request are converted to raw value '' - const errorMsg = HttpErrorMessage.MISSING_REQUIRED( - REQUIRED_NUMBER_PARAM.name, - ); test( REQUIRED_NUMBER_PARAM, '', - new HttpErrors['400'](errorMsg), + HttpErrorMessage.missingRequired(REQUIRED_NUMBER_PARAM.name), ); }); }); describe('coerce param from string to number - optional', () => { context('valid values', async () => { - test(NUMBER_PARAM, '0', 0); - test(NUMBER_PARAM, '1', 1); - test(NUMBER_PARAM, '-1', -1); - test(NUMBER_PARAM, '1.2', 1.2); - test(NUMBER_PARAM, '-1.2', -1.2); + test(NUMBER_PARAM, '0', 0); + test(NUMBER_PARAM, '1', 1); + test(NUMBER_PARAM, '-1', -1); + test(NUMBER_PARAM, '1.2', 1.2); + test(NUMBER_PARAM, '-1.2', -1.2); }); context('numbers larger than MAX_SAFE_INTEGER get trimmed', () => { - test(NUMBER_PARAM, '2343546576878989879789', 2.34354657687899e21); - test(NUMBER_PARAM, '-2343546576878989879789', -2.34354657687899e21); + test(NUMBER_PARAM, '2343546576878989879789', 2.34354657687899e21); + test(NUMBER_PARAM, '-2343546576878989879789', -2.34354657687899e21); }); context('scientific notations', () => { - test(NUMBER_PARAM, '1.234e+30', 1.234e30); - test(NUMBER_PARAM, '-1.234e+30', -1.234e30); + test(NUMBER_PARAM, '1.234e+30', 1.234e30); + test(NUMBER_PARAM, '-1.234e+30', -1.234e30); }); context('empty value converts to undefined', () => { // [], {} sent from request are converted to raw value undefined - test(NUMBER_PARAM, undefined, undefined); + test(NUMBER_PARAM, undefined, undefined); }); context('All other non-number values trigger ERROR_BAD_REQUEST', () => { - let errorMsg = HttpErrorMessage.INVALID_DATA('text', NUMBER_PARAM.name); - // 'false', false, 'true', true, 'text' sent from request are convert to a string - test( - NUMBER_PARAM, - 'text', - new HttpErrors['400'](errorMsg), - ); - - errorMsg = HttpErrorMessage.INVALID_DATA({a: true}, NUMBER_PARAM.name); - // {a: true}, [1,2] are convert to object - test( - NUMBER_PARAM, - {a: true}, - new HttpErrors['400'](errorMsg), - ); + // 'false', false, 'true', true, 'text' sent from request are converted to a string + test(NUMBER_PARAM, 'text', HttpErrorMessage.invalidData('text', NUMBER_PARAM.name)); + // {a: true}, [1,2] are converted to object + test(NUMBER_PARAM, {a: true}, HttpErrorMessage.invalidData({a: true}, NUMBER_PARAM.name)); }); }); describe('OAI3 primitive types', () => { - test(FLOAT_PARAM, '3.333333', 3.333333); - test(DOUBLE_PARAM, '3.3333333333', 3.3333333333); + test(FLOAT_PARAM, '3.333333', 3.333333); + test(DOUBLE_PARAM, '3.3333333333', 3.3333333333); });