Skip to content

feat: Impossible to use UUID as param for object construction #706 #708

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
14 changes: 9 additions & 5 deletions src/ActionParameterHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,14 @@ export class ActionParameterHandler<T extends BaseDriver> {
protected async normalizeParamValue(value: any, param: ParamMetadata): Promise<any> {
if (value === null || value === undefined) return value;

// if param value is an object and param type match, normalize its string properties
if (
const isNormalisationNeeded =
typeof value === 'object' &&
['queries', 'headers', 'params', 'cookies'].some(paramType => paramType === param.type)
) {
['queries', 'headers', 'params', 'cookies'].some(paramType => paramType === param.type);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

includes() would be better in this case

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const isTargetPrimitive = ['number', 'string', 'boolean'].indexOf(param.targetName) > -1;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also use includes() here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const isTransformationNeeded = (param.parse || param.isTargetObject) && param.type !== 'param';

// if param value is an object and param type match, normalize its string properties
if (isNormalisationNeeded) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (isNormalisationNeeded) {
if (isNormalizationNeeded) {

await Promise.all(
Object.keys(value).map(async key => {
const keyValue = value[key];
Expand All @@ -123,6 +126,7 @@ export class ActionParameterHandler<T extends BaseDriver> {
})
);
}

// if value is a string, normalize it to demanded type
else if (typeof value === 'string') {
switch (param.targetName) {
Expand All @@ -135,7 +139,7 @@ export class ActionParameterHandler<T extends BaseDriver> {
}

// if target type is not primitive, transform and validate it
if (['number', 'string', 'boolean'].indexOf(param.targetName) === -1 && (param.parse || param.isTargetObject)) {
if (!isTargetPrimitive && isTransformationNeeded) {
value = this.parseValue(value, param);
value = this.transformValue(value, param);
value = await this.validateValue(value, param);
Expand Down
63 changes: 63 additions & 0 deletions test/ActionParameterHandler.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { ActionParameterHandler } from '../src/ActionParameterHandler';
import { ActionMetadata, ControllerMetadata, ExpressDriver, ParamMetadata } from '../src';
import { ActionMetadataArgs } from '../src/metadata/args/ActionMetadataArgs';
import { ControllerMetadataArgs } from '../src/metadata/args/ControllerMetadataArgs';

const expect = require('chakram').expect;

describe('ActionParameterHandler', () => {
it('handle', async () => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should add negative and positive cases

Copy link
Contributor Author

@ivanproskuryakov ivanproskuryakov Apr 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Broader test coverage was added including two negative tests:


PASS test/ActionParameterHandler.spec.ts
ActionParameterHandler
✓ handle - should process string parameters (154 ms)
✓ handle - should process string parameters, returns empty if a given string is empty
✓ handle - should process number parameters (1 ms)
✓ handle - undefined on empty object provided
✓ handle - throws error if the parameter is required
✓ handle - throws error if the parameter is required, type file provided (10 ms)

const driver = new ExpressDriver();
const actionParameterHandler = new ActionParameterHandler(driver);

const action = {
request: {
params: {
id: '0b5ec98f-e26d-4414-b798-dcd35a5ef859',
},
},
response: {},
};
const controllerMetadataArgs: ControllerMetadataArgs = {
target: function () {},
route: '',
type: 'json',
options: {},
};
const controllerMetadata = new ControllerMetadata(controllerMetadataArgs);
const args: ActionMetadataArgs = {
route: '',
method: 'getProduct',
options: {},
target: function () {},
type: 'get',
appendParams: undefined,
};
const actionMetadata = new ActionMetadata(controllerMetadata, args, {});

const param: ParamMetadata = {
targetName: 'product',
isTargetObject: true,
actionMetadata,
target: () => {},
method: 'getProduct',
object: 'getProduct',
extraOptions: undefined,
index: 0,
type: 'param',
name: 'id',
parse: undefined,
required: false,
transform: function (action, value) {
return value;
},
classTransform: undefined,
validate: undefined,
targetType: function () {},
};

const processedValue = await actionParameterHandler.handle(action, param);

expect(processedValue).to.be.eq(action.request.params.id);
});
});