From f59d45f9c914b5a86400e5806e1554b1dec36394 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 | 5 +- .../rest/src/coercion/http-error-message.ts | 16 +++--- packages/rest/src/coercion/validator.ts | 5 +- .../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 | 51 ++++++++----------- 8 files changed, 41 insertions(+), 50 deletions(-) diff --git a/packages/rest/src/coercion/coerce-parameter.ts b/packages/rest/src/coercion/coerce-parameter.ts index ae6e489d880e..a59dffeabfb4 100644 --- a/packages/rest/src/coercion/coerce-parameter.ts +++ b/packages/rest/src/coercion/coerce-parameter.ts @@ -6,7 +6,6 @@ import {ParameterObject, isReferenceObject} from '@loopback/openapi-v3-types'; import {Validator} from './validator'; import * as debugModule from 'debug'; -import {HttpErrors} from '..'; import {HttpErrorMessage} from '../'; const debug = debugModule('loopback:rest:coercion'); @@ -50,9 +49,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..fe092143e982 100644 --- a/packages/rest/src/coercion/validator.ts +++ b/packages/rest/src/coercion/validator.ts @@ -4,7 +4,6 @@ // License text available at https://opensource.org/licenses/MIT import {ParameterObject} from '@loopback/openapi-v3-types'; -import * as HttpErrors from 'http-errors'; import {HttpErrorMessage} from '../'; /** @@ -43,7 +42,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 +67,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..ddc52ac9c77b 100644 --- a/packages/rest/test/unit/coercion/paramStringToNumber.unit.ts +++ b/packages/rest/test/unit/coercion/paramStringToNumber.unit.ts @@ -39,71 +39,64 @@ 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( + // 'false', false, 'true', true, 'text' sent from request are converted to a string + test( NUMBER_PARAM, 'text', - new HttpErrors['400'](errorMsg), + HttpErrorMessage.invalidData('text', NUMBER_PARAM.name), ); - - errorMsg = HttpErrorMessage.INVALID_DATA({a: true}, NUMBER_PARAM.name); - // {a: true}, [1,2] are convert to object - test( + // {a: true}, [1,2] are converted to object + test( NUMBER_PARAM, {a: true}, - new HttpErrors['400'](errorMsg), + 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); });