Skip to content

Commit

Permalink
fix: apply feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@jannyHou
jannyHou committed Jun 8, 2018
1 parent 05bb648 commit f59d45f
Show file tree
Hide file tree
Showing 8 changed files with 41 additions and 50 deletions.
5 changes: 1 addition & 4 deletions packages/rest/src/coercion/coerce-parameter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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);
Expand Down
16 changes: 9 additions & 7 deletions packages/rest/src/coercion/http-error-message.ts
View file
Original file line number Diff line number Diff line change
@@ -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<T>(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);
}
}
5 changes: 2 additions & 3 deletions packages/rest/src/coercion/validator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 '../';

/**
Expand Down Expand Up @@ -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;
}
Expand All @@ -68,6 +67,6 @@ export class Validator {
*/
// tslint:disable-next-line:no-any
isAbsent(value: any) {
return [''].includes(value);
return value === '';
}
}
6 changes: 3 additions & 3 deletions packages/rest/test/unit/coercion/paramStringToBoolean.unit.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const BOOLEAN_PARAM = {
};

describe('coerce param from string to boolean', () => {
test<boolean>(BOOLEAN_PARAM, 'false', false);
test<boolean>(BOOLEAN_PARAM, 'true', true);
test<undefined>(BOOLEAN_PARAM, undefined, undefined);
test(BOOLEAN_PARAM, 'false', false);
test(BOOLEAN_PARAM, 'true', true);
test(BOOLEAN_PARAM, undefined, undefined);
});
2 changes: 1 addition & 1 deletion packages/rest/test/unit/coercion/paramStringToBuffer.unit.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe('coerce param from string to buffer', () => {
base64: Buffer.from('Hello World').toString('base64'),
};

test<Buffer>(
test(
BUFFER_PARAM,
testValues.base64,
Buffer.from(testValues.base64, 'base64'),
Expand Down
2 changes: 1 addition & 1 deletion packages/rest/test/unit/coercion/paramStringToDate.unit.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@ const DATE_PARAM = {
};

describe('coerce param from string to date', () => {
test<Date>(DATE_PARAM, '2015-03-01', new Date('2015-03-01'));
test(DATE_PARAM, '2015-03-01', new Date('2015-03-01'));
});
4 changes: 2 additions & 2 deletions packages/rest/test/unit/coercion/paramStringToInteger.unit.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,6 @@ const INT64_PARAM = {
};

describe('coerce param from string to integer', () => {
test<number>(INT32_PARAM, '100', 100);
test<number>(INT64_PARAM, '9223372036854775807', 9223372036854775807);
test(INT32_PARAM, '100', 100);
test(INT64_PARAM, '9223372036854775807', 9223372036854775807);
});
51 changes: 22 additions & 29 deletions packages/rest/test/unit/coercion/paramStringToNumber.unit.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,71 +39,64 @@ const DOUBLE_PARAM = {
},
};

/*tslint:disable:max-line-length*/
describe('coerce param from string to number - required', () => {
context('valid values', () => {
test<number>(REQUIRED_NUMBER_PARAM, '0', 0);
test<number>(REQUIRED_NUMBER_PARAM, '1', 1);
test<number>(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<HttpErrors.HttpError>(
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>(NUMBER_PARAM, '0', 0);
test<number>(NUMBER_PARAM, '1', 1);
test<number>(NUMBER_PARAM, '-1', -1);
test<number>(NUMBER_PARAM, '1.2', 1.2);
test<number>(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>(NUMBER_PARAM, '2343546576878989879789', 2.34354657687899e21);
test<number>(NUMBER_PARAM, '-2343546576878989879789', -2.34354657687899e21);
test(NUMBER_PARAM, '2343546576878989879789', 2.34354657687899e21);
test(NUMBER_PARAM, '-2343546576878989879789', -2.34354657687899e21);
});

context('scientific notations', () => {
test<number>(NUMBER_PARAM, '1.234e+30', 1.234e30);
test<number>(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<undefined>(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<HttpErrors.HttpError>(
// '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<HttpErrors.HttpError>(
// {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<number>(FLOAT_PARAM, '3.333333', 3.333333);
test<number>(DOUBLE_PARAM, '3.3333333333', 3.3333333333);
test(FLOAT_PARAM, '3.333333', 3.333333);
test(DOUBLE_PARAM, '3.3333333333', 3.3333333333);
});

0 comments on commit f59d45f

Please sign in to comment.