From c770417ff23d9f01ffbc54beb45840352be0ffb7 Mon Sep 17 00:00:00 2001 From: jannyHou Date: Mon, 28 May 2018 23:43:11 -0400 Subject: [PATCH] feat: add type coercion --- .../todo/src/controllers/todo.controller.ts | 12 -- .../rest/src/coercion/coerce-parameter.ts | 115 ++++++++++++++++++ packages/rest/src/coercion/rest-http-error.ts | 16 +++ packages/rest/src/coercion/validator.ts | 68 +++++++++++ packages/rest/src/index.ts | 1 + packages/rest/src/parser.ts | 47 ++++--- .../coercion/coercion.acceptance.ts | 58 +++++++++ .../unit/coercion/invalid-spec.unit.test.ts | 18 +++ .../coercion/paramStringToBoolean.unit.ts | 19 +++ .../unit/coercion/paramStringToBuffer.unit.ts | 25 ++++ .../unit/coercion/paramStringToDate.unit.ts | 17 +++ .../coercion/paramStringToInteger.unit.ts | 24 ++++ .../unit/coercion/paramStringToNumber.unit.ts | 101 +++++++++++++++ packages/rest/test/unit/coercion/utils.ts | 96 +++++++++++++++ 14 files changed, 588 insertions(+), 29 deletions(-) create mode 100644 packages/rest/src/coercion/coerce-parameter.ts create mode 100644 packages/rest/src/coercion/rest-http-error.ts create mode 100644 packages/rest/src/coercion/validator.ts create mode 100644 packages/rest/test/acceptance/coercion/coercion.acceptance.ts create mode 100644 packages/rest/test/unit/coercion/invalid-spec.unit.test.ts create mode 100644 packages/rest/test/unit/coercion/paramStringToBoolean.unit.ts create mode 100644 packages/rest/test/unit/coercion/paramStringToBuffer.unit.ts create mode 100644 packages/rest/test/unit/coercion/paramStringToDate.unit.ts create mode 100644 packages/rest/test/unit/coercion/paramStringToInteger.unit.ts create mode 100644 packages/rest/test/unit/coercion/paramStringToNumber.unit.ts create mode 100644 packages/rest/test/unit/coercion/utils.ts diff --git a/examples/todo/src/controllers/todo.controller.ts b/examples/todo/src/controllers/todo.controller.ts index f5d3ef83b37c..2e620136513d 100644 --- a/examples/todo/src/controllers/todo.controller.ts +++ b/examples/todo/src/controllers/todo.controller.ts @@ -48,12 +48,6 @@ export class TodoController { @param.path.number('id') id: number, @requestBody() todo: Todo, ): Promise { - // REST adapter does not coerce parameter values coming from string sources - // like path & query. As a workaround, we have to cast the value to a number - // ourselves. - // See https://github.com/strongloop/loopback-next/issues/750 - id = +id; - return await this.todoRepo.replaceById(id, todo); } @@ -62,12 +56,6 @@ export class TodoController { @param.path.number('id') id: number, @requestBody() todo: Todo, ): Promise { - // REST adapter does not coerce parameter values coming from string sources - // like path & query. As a workaround, we have to cast the value to a number - // ourselves. - // See https://github.com/strongloop/loopback-next/issues/750 - id = +id; - return await this.todoRepo.updateById(id, todo); } diff --git a/packages/rest/src/coercion/coerce-parameter.ts b/packages/rest/src/coercion/coerce-parameter.ts new file mode 100644 index 000000000000..c2e0ec90d747 --- /dev/null +++ b/packages/rest/src/coercion/coerce-parameter.ts @@ -0,0 +1,115 @@ +// Copyright IBM Corp. 2018. All Rights Reserved. +// Node module: @loopback/rest +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + +import {ParameterObject, isReferenceObject} from '@loopback/openapi-v3-types'; +import {Validator} from './validator'; +import * as debugModule from 'debug'; +import {RestHttpErrors} from '../'; + +const debug = debugModule('loopback:rest:coercion'); + +/** + * Coerce the http raw data to a JavaScript type data of a parameter + * according to its OpenAPI schema specification. + * + * @param data The raw data get from http request + * @param schema The parameter's schema defined in OpenAPI specification + */ +export function coerceParameter(data: string, spec: ParameterObject) { + const schema = spec.schema; + if (!schema || isReferenceObject(schema)) { + debug( + 'The parameter with schema %s is not coerced since schema' + + 'dereference is not supported yet.', + schema, + ); + return data; + } + const OAIType = getOAIPrimitiveType(schema.type, schema.format); + const validator = new Validator({parameterSpec: spec}); + + validator.validateParamBeforeCoercion(data); + + switch (OAIType) { + case 'byte': + return Buffer.from(data, 'base64'); + case 'date': + return new Date(data); + case 'float': + case 'double': + return parseFloat(data); + case 'number': + const coercedData = data ? Number(data) : undefined; + if (coercedData === undefined) return; + if (isNaN(coercedData)) throw RestHttpErrors.invalidData(data, spec.name); + return coercedData; + case 'long': + return Number(data); + case 'integer': + return parseInt(data); + case 'boolean': + return isTrue(data) ? true : isFalse(data) ? false : undefined; + case 'string': + case 'password': + // serialize will be supported in next PR + case 'serialize': + default: + return data; + } +} + +/** + * A set of truthy values. A data in this set will be coerced to `true`. + * + * @param data The raw data get from http request + * @returns The corresponding coerced boolean type + */ +function isTrue(data: string): boolean { + return ['true', '1'].includes(data); +} + +/** + * A set of falsy values. A data in this set will be coerced to `false`. + * @param data The raw data get from http request + * @returns The corresponding coerced boolean type + */ +function isFalse(data: string): boolean { + return ['false', '0'].includes(data); +} + +/** + * Return the corresponding OpenAPI data type given an OpenAPI schema + * + * @param type The type in an OpenAPI schema specification + * @param format The format in an OpenAPI schema specification + */ +function getOAIPrimitiveType(type?: string, format?: string) { + // serizlize will be supported in next PR + if (type === 'object' || type === 'array') return 'serialize'; + if (type === 'string') { + switch (format) { + case 'byte': + return 'byte'; + case 'binary': + return 'binary'; + case 'date': + return 'date'; + case 'date-time': + return 'date-time'; + case 'password': + return 'password'; + default: + return 'string'; + } + } + if (type === 'boolean') return 'boolean'; + if (type === 'number') + return format === 'float' + ? 'float' + : format === 'double' + ? 'double' + : 'number'; + if (type === 'integer') return format === 'int64' ? 'long' : 'integer'; +} diff --git a/packages/rest/src/coercion/rest-http-error.ts b/packages/rest/src/coercion/rest-http-error.ts new file mode 100644 index 000000000000..ba15067c2437 --- /dev/null +++ b/packages/rest/src/coercion/rest-http-error.ts @@ -0,0 +1,16 @@ +import * as HttpErrors from 'http-errors'; +export namespace RestHttpErrors { + 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); + } + export function invalidParamLocation(location: string): HttpErrors.HttpError { + return new HttpErrors.NotImplemented( + 'Parameters with "in: ' + location + '" are not supported yet.', + ); + } +} diff --git a/packages/rest/src/coercion/validator.ts b/packages/rest/src/coercion/validator.ts new file mode 100644 index 000000000000..1e72497c2bd9 --- /dev/null +++ b/packages/rest/src/coercion/validator.ts @@ -0,0 +1,68 @@ +// Copyright IBM Corp. 2018. All Rights Reserved. +// Node module: @loopback/rest +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + +import {ParameterObject} from '@loopback/openapi-v3-types'; +import {RestHttpErrors} from '../'; + +/** + * A set of options to pass into the validator functions + */ +export type ValidationOptions = { + required?: boolean; +}; + +/** + * The context information that a validator needs + */ +export type ValidationContext = { + parameterSpec: ParameterObject; +}; + +/** + * Validator class provides a bunch of functions that perform + * validations on the request parameters and request body. + */ +export class Validator { + constructor(public ctx: ValidationContext) {} + + /** + * The validation executed before type coercion. Like + * checking absence. + * + * @param type A parameter's type. + * @param value A parameter's raw value from http request. + * @param opts options + */ + validateParamBeforeCoercion( + value: string | object | undefined, + opts?: ValidationOptions, + ) { + if (this.isAbsent(value) && this.isRequired(opts)) { + const name = this.ctx.parameterSpec.name; + throw RestHttpErrors.missingRequired(name); + } + } + + /** + * Check is a parameter required or not. + * + * @param opts + */ + isRequired(opts?: ValidationOptions) { + if (this.ctx.parameterSpec.required) return true; + if (opts && opts.required) return true; + return false; + } + + /** + * Return `true` if the value is empty, return `false` otherwise. + * + * @param value + */ + // tslint:disable-next-line:no-any + isAbsent(value: any) { + return value === '' || value === undefined; + } +} diff --git a/packages/rest/src/index.ts b/packages/rest/src/index.ts index e99c016069e1..7137560ed2d6 100644 --- a/packages/rest/src/index.ts +++ b/packages/rest/src/index.ts @@ -39,3 +39,4 @@ export * from './rest.component'; export * from './rest.server'; export * from './sequence'; export * from '@loopback/openapi-v3'; +export * from './coercion/rest-http-error'; diff --git a/packages/rest/src/parser.ts b/packages/rest/src/parser.ts index bb7da42c8f93..b83653534fde 100644 --- a/packages/rest/src/parser.ts +++ b/packages/rest/src/parser.ts @@ -14,6 +14,8 @@ import {REQUEST_BODY_INDEX} from '@loopback/openapi-v3'; import {promisify} from 'util'; import {OperationArgs, Request, PathParameterValues} from './types'; import {ResolvedRoute} from './router/routing-table'; +import {coerceParameter} from './coercion/coerce-parameter'; +import {RestHttpErrors} from './index'; type HttpError = HttpErrors.HttpError; // tslint:disable-next-line:no-any @@ -101,24 +103,35 @@ function buildOperationArguments( throw new Error('$ref parameters are not supported yet.'); } const spec = paramSpec as ParameterObject; - switch (spec.in) { - case 'query': - paramArgs.push(request.query[spec.name]); - break; - case 'path': - paramArgs.push(pathParams[spec.name]); - break; - case 'header': - paramArgs.push(request.headers[spec.name.toLowerCase()]); - break; - // TODO(jannyhou) to support `cookie`, - // see issue https://github.com/strongloop/loopback-next/issues/997 - default: - throw new HttpErrors.NotImplemented( - 'Parameters with "in: ' + spec.in + '" are not supported yet.', - ); - } + const rawValue = getParamFromRequest(spec, request, pathParams); + const coercedValue = coerceParameter(rawValue, spec); + paramArgs.push(coercedValue); } if (requestBodyIndex > -1) paramArgs.splice(requestBodyIndex, 0, body); return paramArgs; } + +function getParamFromRequest( + spec: ParameterObject, + request: Request, + pathParams: PathParameterValues, +) { + let result; + switch (spec.in) { + case 'query': + result = request.query[spec.name]; + break; + case 'path': + result = pathParams[spec.name]; + break; + case 'header': + // @jannyhou TBD: check edge cases + result = request.headers[spec.name.toLowerCase()]; + break; + // TODO(jannyhou) to support `cookie`, + // see issue https://github.com/strongloop/loopback-next/issues/997 + default: + throw RestHttpErrors.invalidParamLocation(spec.in); + } + return result; +} diff --git a/packages/rest/test/acceptance/coercion/coercion.acceptance.ts b/packages/rest/test/acceptance/coercion/coercion.acceptance.ts new file mode 100644 index 000000000000..2478ece9bf7e --- /dev/null +++ b/packages/rest/test/acceptance/coercion/coercion.acceptance.ts @@ -0,0 +1,58 @@ +import {supertest, createClientForHandler, sinon} from '@loopback/testlab'; +import {RestApplication, get, param} from '../../..'; + +describe('Coercion', () => { + let app: RestApplication; + let client: supertest.SuperTest; + + before(givenAClient); + + after(async () => { + await app.stop(); + }); + + class MyController { + @get('/create-number-from-path/{num}') + createNumberFromPath(@param.path.number('num') num: number) { + return num; + } + + @get('/create-number-from-query') + createNumberFromQuery(@param.query.number('num') num: number) { + return num; + } + + @get('/create-number-from-header') + createNumberFromHeader(@param.header.number('num') num: number) { + return num; + } + } + + it('coerces parameter in path from string to number', async () => { + const spy = sinon.spy(MyController.prototype, 'createNumberFromPath'); + await client.get('/create-number-from-path/100').expect(200); + sinon.assert.calledWithExactly(spy, 100); + }); + + it('coerces parameter in header from string to number', async () => { + const spy = sinon.spy(MyController.prototype, 'createNumberFromHeader'); + await client.get('/create-number-from-header').set({num: 100}); + sinon.assert.calledWithExactly(spy, 100); + }); + + it('coerces parameter in query from string to number', async () => { + const spy = sinon.spy(MyController.prototype, 'createNumberFromQuery'); + await client + .get('/create-number-from-query') + .query({num: 100}) + .expect(200); + sinon.assert.calledWithExactly(spy, 100); + }); + + async function givenAClient() { + app = new RestApplication(); + app.controller(MyController); + await app.start(); + client = await createClientForHandler(app.requestHandler); + } +}); diff --git a/packages/rest/test/unit/coercion/invalid-spec.unit.test.ts b/packages/rest/test/unit/coercion/invalid-spec.unit.test.ts new file mode 100644 index 000000000000..2df8db53cd08 --- /dev/null +++ b/packages/rest/test/unit/coercion/invalid-spec.unit.test.ts @@ -0,0 +1,18 @@ +// Copyright IBM Corp. 2018. All Rights Reserved. +// Node module: @loopback/rest +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + +import {test} from './utils'; +import {RestHttpErrors} from '../../../'; +import {ParameterLocation} from '@loopback/openapi-v3-types'; + +const INVALID_PARAM = { + in: 'unknown', + name: 'aparameter', + schema: {type: 'unknown'}, +}; + +describe('throws error for invalid parameter spec', () => { + test(INVALID_PARAM, '', RestHttpErrors.invalidParamLocation('unknown')); +}); diff --git a/packages/rest/test/unit/coercion/paramStringToBoolean.unit.ts b/packages/rest/test/unit/coercion/paramStringToBoolean.unit.ts new file mode 100644 index 000000000000..2121e197be2c --- /dev/null +++ b/packages/rest/test/unit/coercion/paramStringToBoolean.unit.ts @@ -0,0 +1,19 @@ +// Copyright IBM Corp. 2018. All Rights Reserved. +// Node module: @loopback/rest +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + +import {test} from './utils'; +import {ParameterLocation} from '@loopback/openapi-v3-types'; + +const BOOLEAN_PARAM = { + in: 'path', + name: 'aparameter', + schema: {type: 'boolean'}, +}; + +describe('coerce param from string to boolean', () => { + 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 new file mode 100644 index 000000000000..967b02176f0c --- /dev/null +++ b/packages/rest/test/unit/coercion/paramStringToBuffer.unit.ts @@ -0,0 +1,25 @@ +// Copyright IBM Corp. 2018. All Rights Reserved. +// Node module: @loopback/rest +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + +import {test} from './utils'; +import {ParameterLocation} from '@loopback/openapi-v3-types'; + +const BUFFER_PARAM = { + in: 'path', + name: 'aparameter', + schema: {type: 'string', format: 'byte'}, +}; + +describe('coerce param from string to buffer', () => { + const testValues = { + base64: Buffer.from('Hello World').toString('base64'), + }; + + 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 new file mode 100644 index 000000000000..c787510c4818 --- /dev/null +++ b/packages/rest/test/unit/coercion/paramStringToDate.unit.ts @@ -0,0 +1,17 @@ +// Copyright IBM Corp. 2018. All Rights Reserved. +// Node module: @loopback/rest +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + +import {test} from './utils'; +import {ParameterLocation} from '@loopback/openapi-v3-types'; + +const DATE_PARAM = { + in: 'path', + name: 'aparameter', + schema: {type: 'string', format: 'date'}, +}; + +describe('coerce param from string to date', () => { + 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 new file mode 100644 index 000000000000..11be18b16ef6 --- /dev/null +++ b/packages/rest/test/unit/coercion/paramStringToInteger.unit.ts @@ -0,0 +1,24 @@ +// Copyright IBM Corp. 2018. All Rights Reserved. +// Node module: @loopback/rest +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + +import {test} from './utils'; +import {ParameterLocation} from '@loopback/openapi-v3-types'; + +const INT32_PARAM = { + in: 'path', + name: 'aparameter', + schema: {type: 'integer', format: 'int32'}, +}; + +const INT64_PARAM = { + in: 'path', + name: 'aparameter', + schema: {type: 'integer', format: 'int64'}, +}; + +describe('coerce param from string to integer', () => { + 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 new file mode 100644 index 000000000000..5c32a76c4563 --- /dev/null +++ b/packages/rest/test/unit/coercion/paramStringToNumber.unit.ts @@ -0,0 +1,101 @@ +// Copyright IBM Corp. 2018. All Rights Reserved. +// Node module: @loopback/rest +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + +import {test} from './utils'; +import {ParameterLocation} from '@loopback/openapi-v3-types'; +import {RestHttpErrors} from './../../../'; + +const NUMBER_PARAM = { + in: 'path', + name: 'aparameter', + schema: {type: 'number'}, +}; + +const REQUIRED_NUMBER_PARAM = { + in: 'path', + name: 'aparameter', + schema: {type: 'number'}, + required: true, +}; + +const FLOAT_PARAM = { + in: 'path', + name: 'aparameter', + schema: { + type: 'number', + format: 'float', + }, +}; + +const DOUBLE_PARAM = { + in: 'path', + name: 'aparameter', + schema: { + type: 'number', + format: 'double', + }, +}; + +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); + }); + + context('empty values trigger ERROR_BAD_REQUEST', () => { + // null, '' sent from request are converted to raw value '' + test( + REQUIRED_NUMBER_PARAM, + '', + RestHttpErrors.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); + }); + + context('numbers larger than MAX_SAFE_INTEGER get trimmed', () => { + 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); + }); + + context('empty value converts to undefined', () => { + // [], {} sent from request are converted to raw value undefined + test(NUMBER_PARAM, undefined, undefined); + }); + + context('All other non-number values trigger ERROR_BAD_REQUEST', () => { + // 'false', false, 'true', true, 'text' sent from request are converted to a string + test( + NUMBER_PARAM, + 'text', + RestHttpErrors.invalidData('text', NUMBER_PARAM.name), + ); + // {a: true}, [1,2] are converted to object + test( + NUMBER_PARAM, + {a: true}, + RestHttpErrors.invalidData({a: true}, NUMBER_PARAM.name), + ); + }); +}); + +describe('OAI3 primitive types', () => { + test(FLOAT_PARAM, '3.333333', 3.333333); + test(DOUBLE_PARAM, '3.3333333333', 3.3333333333); +}); diff --git a/packages/rest/test/unit/coercion/utils.ts b/packages/rest/test/unit/coercion/utils.ts new file mode 100644 index 000000000000..cdcb8981b494 --- /dev/null +++ b/packages/rest/test/unit/coercion/utils.ts @@ -0,0 +1,96 @@ +import {OperationObject, ParameterObject} from '@loopback/openapi-v3-types'; + +import { + ShotRequestOptions, + expect, + stubExpressContext, +} from '@loopback/testlab'; + +import { + PathParameterValues, + Request, + Route, + createResolvedRoute, + parseOperationArgs, + ResolvedRoute, +} from '../../..'; +import * as HttpErrors from 'http-errors'; + +function givenOperationWithParameters(params?: ParameterObject[]) { + return { + 'x-operation-name': 'testOp', + parameters: params, + responses: {}, + }; +} + +function givenRequest(options?: ShotRequestOptions): Request { + return stubExpressContext(options).request; +} + +function givenResolvedRoute( + spec: OperationObject, + pathParams: PathParameterValues = {}, +): ResolvedRoute { + const route = new Route('get', '/', spec, () => {}); + return createResolvedRoute(route, pathParams); +} + +export interface TestArgs { + paramSpec: ParameterObject; + rawValue: string | undefined | object; + expectedResult: T; + caller: string; + expectError: boolean; + opts: TestOptions; +} + +export type TestOptions = { + testName?: string; +}; + +export async function testCoercion(config: TestArgs) { + /* istanbul ignore next */ + try { + const req = givenRequest(); + const spec = givenOperationWithParameters([config.paramSpec]); + const route = givenResolvedRoute(spec, {aparameter: config.rawValue}); + + if (config.expectError) { + try { + await parseOperationArgs(req, route); + throw new Error("'parseOperationArgs' should throw error!"); + } catch (err) { + expect(err).to.eql(config.expectedResult); + } + } else { + const args = await parseOperationArgs(req, route); + expect(args).to.eql([config.expectedResult]); + } + } catch (err) { + err.stack += config.caller; + throw err; + } +} + +export function test( + paramSpec: ParameterObject, + rawValue: string | undefined | object, + expectedResult: T, + opts?: TestOptions, +) { + const caller: string = new Error().stack!; + const DEFAULT_TEST_NAME = `convert request raw value ${rawValue} to ${expectedResult}`; + const testName = (opts && opts.testName) || DEFAULT_TEST_NAME; + + it(testName, async () => { + await testCoercion({ + paramSpec, + rawValue, + expectedResult, + caller, + expectError: expectedResult instanceof HttpErrors.HttpError, + opts: opts || {}, + }); + }); +}