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 5, 2018
1 parent 61ece0f commit 729fbdc
Show file tree
Hide file tree
Showing 9 changed files with 55 additions and 57 deletions.
16 changes: 12 additions & 4 deletions packages/rest/src/coercion/coerce-parameter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ import {
ReferenceObject,
isReferenceObject,
} from '@loopback/openapi-v3-types';

import * as HttpErrors from 'http-errors';
import * as debugModule from 'debug';

const debug = debugModule('loopback:rest:coercion');

/**
* Coerce the http raw data to a JavaScript type data of a parameter
Expand All @@ -22,8 +24,14 @@ export function coerceParameter(
data: string,
schema?: SchemaObject | ReferenceObject,
) {
// ignore reference schema
if (!schema || isReferenceObject(schema)) return data;
if (!schema || isReferenceObject(schema)) {
debug(
'The parameter with schema %s is not coerced since schema' +
'dereferrence is not supported yet.',
schema,
);
return data;
}
let coercedResult;
coercedResult = data;
const OAIType = getOAIPrimitiveType(schema.type, schema.format);
Expand Down Expand Up @@ -51,7 +59,7 @@ export function coerceParameter(
coercedResult = isTrue(data) ? true : false;
case 'string':
case 'password':
// serizlize will be supported in next PR
// serialize will be supported in next PR
case 'serialize':
break;
case 'unknownType':
Expand Down
52 changes: 28 additions & 24 deletions packages/rest/src/parser.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,33 +102,37 @@ function buildOperationArguments(
throw new Error('$ref parameters are not supported yet.');
}
const spec = paramSpec as ParameterObject;
const rawValue = getParamFromRequest();
const rawValue = getParamFromRequest(spec, request, pathParams);
const coercedValue = coerceParameter(rawValue, spec.schema);
paramArgs.push(coercedValue);

function getParamFromRequest() {
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 new HttpErrors.NotImplemented(
'Parameters with "in: ' + spec.in + '" are not supported yet.',
);
}
return result;
}
}
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 new HttpErrors.NotImplemented(
'Parameters with "in: ' + spec.in + '" are not supported yet.',
);
}
return result;
}
24 changes: 5 additions & 19 deletions packages/rest/test/acceptance/coercion/coercion.acceptance.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ describe('Coercion', () => {
let server: RestServer;
let client: supertest.SuperTest<supertest.Test>;

before(givenAnApplication);
before(givenAServer);
before(givenAClient);

after(async () => {
Expand All @@ -39,13 +37,13 @@ describe('Coercion', () => {
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);
assertCoercion(spy);
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});
assertCoercion(spy);
sinon.assert.calledWithExactly(spy, 100);
});

it('coerces parameter in query from string to number', async () => {
Expand All @@ -54,25 +52,13 @@ describe('Coercion', () => {
.get('/create-number-from-query')
.query({num: 100})
.expect(200);
assertCoercion(spy);
});

function assertCoercion(spy: SinonSpy) {
sinon.assert.calledWithExactly(spy, 100);
sinon.assert.neverCalledWith(spy, '100');
}
});

async function givenAnApplication() {
async function givenAClient() {
app = new RestApplication();
app.controller(MyController);
await app.start();
}

async function givenAServer() {
server = await app.getServer(RestServer);
}

async function givenAClient() {
client = await createClientForHandler(server.requestHandler);
client = await createClientForHandler(app.requestHandler);
}
});
4 changes: 2 additions & 2 deletions packages/rest/test/unit/coercion/paramStringToBoolean.unit.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import {runTests} from './utils';
describe('coerce param from string to boolean', () => {
/*tslint:disable:max-line-length*/
const testCases = [
['false', {type: 'boolean'}, 'false', false, new Error().stack!],
['true', {type: 'boolean'}, 'true', true, new Error().stack!],
['false', {type: 'boolean'}, 'false', false, new Error().stack],
['true', {type: 'boolean'}, 'true', true, new Error().stack],
];

runTests(testCases);
Expand Down
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 @@ -11,7 +11,7 @@ describe('coerce param from string to buffer', () => {
}
/*tslint:disable:max-line-length*/
const testCases = [
['base64', {type: 'string', format: 'byte'}, testValues.base64, Buffer.from(testValues.base64, 'base64'), new Error().stack!],
['base64', {type: 'string', format: 'byte'}, testValues.base64, Buffer.from(testValues.base64, 'base64'), new Error().stack],
];

runTests(testCases);
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 @@ -8,7 +8,7 @@ import {runTests} from './utils';
describe('coerce param from string to date', () => {
/*tslint:disable:max-line-length*/
const testCases = [
['date', {type: 'string', format: 'date'}, '2015-03-01', new Date('2015-03-01'), new Error().stack!],
['date', {type: 'string', format: 'date'}, '2015-03-01', new Date('2015-03-01'), new Error().stack],
];

runTests(testCases);
Expand Down
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 @@ -8,8 +8,8 @@ import {runTests} from './utils';
describe('coerce param from string to integer', () => {
/*tslint:disable:max-line-length*/
const testCases = [
['integer', {type: 'integer', format: 'int32'}, '100', 100, new Error().stack!],
['long', {type: 'integer', format: 'int64'}, '9223372036854775807', 9223372036854775807, new Error().stack!],
['integer', {type: 'integer', format: 'int32'}, '100', 100, new Error().stack],
['long', {type: 'integer', format: 'int64'}, '9223372036854775807', 9223372036854775807, new Error().stack],
]
runTests(testCases);
});
4 changes: 2 additions & 2 deletions packages/rest/test/unit/coercion/paramStringToNumber.unit.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import {runTests} from './utils';
describe('coerce param from string to number', () => {
/*tslint:disable:max-line-length*/
const testCases = [
['float', {type: 'number', format: 'float'}, '3.333333', 3.333333, new Error().stack!],
['double', {type: 'number', format: 'double'}, '3.3333333333', 3.3333333333, new Error().stack!],
['float', {type: 'number', format: 'float'}, '3.333333', 3.333333, new Error().stack],
['double', {type: 'number', format: 'double'}, '3.3333333333', 3.3333333333, new Error().stack],
];

runTests(testCases);
Expand Down
4 changes: 2 additions & 2 deletions packages/rest/test/unit/coercion/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,13 @@ export async function testCoercion<T>(config: TestArgs<T>) {
const route = givenResolvedRoute(spec, {aparameter: config.valueFromReq});
const args = await parseOperationArgs(req, route);
expect(args).to.eql([config.expectedResult]);
} catch(err) {
} catch (err) {
throw new Error(`${err} \n Failed ${config.caller.split(/\n/)[1]}`);
}
}

// tslint:disable-next-line:no-any
export function runTests (tests: any[][]) {
export function runTests(tests: any[][]) {
for (let t of tests) {
it(t[0] as string, async () => {
await testCoercion({
Expand Down

0 comments on commit 729fbdc

Please sign in to comment.