Skip to content

Commit

Permalink
fix: parse to correct data type for nested body parameter (#122)
Browse files Browse the repository at this point in the history 
* fix: smapi command - fix parsing issue for number inside of body and second level body parameter mapping to the same parent
  • Loading branch information
@kakhaUrigashvili
kakhaUrigashvili authored Apr 15, 2020
1 parent 03439ad commit 6998837
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 29 deletions.
23 changes: 17 additions & 6 deletions lib/commands/smapi/cli-customization-processor.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class CliCustomizationProcessor {

_processNonBodyParameter(parameter, parentOperation, definitions) {
let customizedParameter = { ...parameter };
const isArray = parameter.type === 'array';
const { type } = parameter;
let enumOptions = parameter.enum;
let { description } = parameter;
if (parameter.items && parameter.items.$ref) {
Expand All @@ -71,7 +71,7 @@ class CliCustomizationProcessor {
enumOptions = enumValue.enumOptions;
description = enumValue.description;
}
customizedParameter = { ...customizedParameter, description, isArray, enum: enumOptions };
customizedParameter = { ...customizedParameter, description, type, enum: enumOptions };
this._addCustomizedParameter(parentOperation.customizationMetadata, customizedParameter);
}

Expand All @@ -82,6 +82,15 @@ class CliCustomizationProcessor {
}

_addCustomizedParameter(customizationMetadata, customizedParameter) {
const isArray = customizedParameter.type === 'array';
const isNumber = ['number', 'integer'].includes(customizedParameter.type);
const isBoolean = customizedParameter.type === 'boolean';
const flags = { isArray, isNumber, isBoolean };
Object.keys(flags).forEach(key => {
if (flags[key]) {
customizedParameter[key] = true;
}
});
customizationMetadata.flatParamsMap.set(camelCase(customizedParameter.name), customizedParameter);
}

Expand All @@ -101,6 +110,7 @@ class CliCustomizationProcessor {
Object.keys(secondLevelDefinition.properties || []).forEach(key => {
const property = secondLevelDefinition.properties[key];
let enumOptions = null;
const { type } = property;
let description = property.description || parentDescription;
if (property.$ref) {
const schema = this._getDefinitionSchema(property.$ref, definitions);
Expand All @@ -112,12 +122,13 @@ class CliCustomizationProcessor {
}
const json = this._shouldParseAsJson(property);
const bodyPath = `${parentName}${BODY_PATH_DELIMITER}${key}`;
customizedParameter = { name: `${parentName} ${key}`, description, rootName, required, bodyPath, enum: enumOptions, json };
customizedParameter = { name: `${parentName} ${key}`, description, rootName, required, bodyPath, enum: enumOptions, json, type };
this._addCustomizedParameter(customizationMetadata, customizedParameter);
});
if (secondLevelDefinition.enum) {
const { type } = secondLevelDefinition;
const { description, enumOptions } = this._mapEnum(parentDescription, secondLevelDefinition);
customizedParameter = { name: parentName, description, rootName, required, bodyPath: parentName, enum: enumOptions };
customizedParameter = { name: parentName, description, rootName, required, bodyPath: parentName, enum: enumOptions, type };
this._addCustomizedParameter(customizationMetadata, customizedParameter);
}
}
Expand All @@ -133,10 +144,10 @@ class CliCustomizationProcessor {
const isRequired = required.includes(key);
this._appendSecondLevelProperty(customizationMetadata, key, rootName, secondLevelDefinition, isRequired, definitions);
} else {
const { description } = property;
const { description, type } = property;
const json = this._shouldParseAsJson(property);
// required inherited from parent param
const customizedParameter = { name: key, description, rootName, required: param.required, bodyPath: key, json };
const customizedParameter = { name: key, description, rootName, required: param.required, bodyPath: key, json, type };
this._addCustomizedParameter(customizationMetadata, customizedParameter);
}
});
Expand Down
8 changes: 6 additions & 2 deletions lib/commands/smapi/smapi-command-handler.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const { StandardSmapiClientBuilder } = require('ask-smapi-sdk');
const os = require('os');
const path = require('path');
const R = require('ramda');

const AppConfig = require('@src/model/app-config');
const AuthorizationController = require('@src/controllers/authorization-controller');
Expand Down Expand Up @@ -36,15 +37,18 @@ const _mapToParams = (optionsValues, flatParamsMap, commanderToApiCustomizationM
const apiName = commanderToApiCustomizationMap.get(key) || key;
const param = flatParamsMap.get(apiName);
if (param) {
const value = param.isArray ? optionsValues[key].split(ARRAY_SPLIT_DELIMITER) : optionsValues[key];
let value = optionsValues[key];
value = param.isArray ? value.split(ARRAY_SPLIT_DELIMITER) : value;
value = param.isNumber ? Number(value) : value;
value = param.isBoolean ? Boolean(value) : value;
if (param.rootName) {
if (!res[param.rootName]) {
res[param.rootName] = {};
}
let mergeObject = {};
mergeObject[param.bodyPath] = value;
mergeObject = unflatten(mergeObject, BODY_PATH_DELIMITER);
Object.assign(res[param.rootName], mergeObject);
res[param.rootName] = R.mergeDeepRight(res[param.rootName], mergeObject);
} else if (param.json) {
res[param.name] = JSON.parse(value);
} else {
Expand Down
23 changes: 16 additions & 7 deletions test/unit/commands/smapi/cli-customization-processor-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ describe('Smapi test - CliCustomizationProcessor class', () => {
const bodyPropertyTwoName = 'propertyTwo';
const nestedPropertyName = 'nestedProperty';
const nestedEnumPropertyName = 'nestedEnumProperty';
const bodyProperty = {};
const bodyProperty = { type: 'number' };
const bodyPropertyWithArray = { type: 'array', items: { $ref: 'test' } };
const bodyPropertyWithDescription = { description: 'property description' };
const bodyPropertyWithDescription = { description: 'property description', type: 'boolean' };
const nestedProperty = { $ref: 'SomeObjectType' };
const nestedEnumProperty = { $ref: 'SomeEnumTypeWithDescription' };
let processor;
Expand Down Expand Up @@ -97,7 +97,7 @@ describe('Smapi test - CliCustomizationProcessor class', () => {
const key = camelCase(parameter.name);
const { flatParamsMap } = parentOperation.customizationMetadata;

const expected = { ...parameter, enum: undefined, isArray: false };
const expected = { ...parameter, enum: undefined };
expect(flatParamsMap.get(key)).eql(expected);
});

Expand Down Expand Up @@ -162,7 +162,8 @@ describe('Smapi test - CliCustomizationProcessor class', () => {
rootName,
bodyPath: bodyPropertyTwoName,
name: bodyPropertyTwoName,
json: false };
json: false,
isBoolean: true };
expect(flatParamsMap.get(bodyPropertyTwoName)).eql(expected);
});

Expand Down Expand Up @@ -206,14 +207,22 @@ describe('Smapi test - CliCustomizationProcessor class', () => {
const { flatParamsMap } = parentOperation.customizationMetadata;
let { name, bodyPath, key } = mapExpectedParams(nestedPropertyName, bodyPropertyOneName);

let expected = { description: parentDescription, name, rootName, bodyPath, required: true, enum: null, json: false };
let expected = { description: parentDescription,
name,
rootName,
bodyPath,
required: true,
enum: null,
json: false,
isNumber: true,
type: 'number' };
expect(flatParamsMap.get(key)).eql(expected);

const expectedParams = mapExpectedParams(nestedPropertyName, bodyPropertyTwoName);
name = expectedParams.name;
bodyPath = expectedParams.bodyPath;
key = expectedParams.key;
expected = { ...bodyPropertyWithDescription, name, rootName, bodyPath, required: true, enum: null, json: false };
expected = { ...bodyPropertyWithDescription, name, rootName, bodyPath, required: true, enum: null, json: false, isBoolean: true };

expect(flatParamsMap.get(key)).eql(expected);
});
Expand Down Expand Up @@ -244,7 +253,7 @@ describe('Smapi test - CliCustomizationProcessor class', () => {
const name = nestedEnumPropertyName;
const bodyPath = nestedEnumPropertyName;

const expected = { ...bodyProperty, name, rootName, bodyPath, required: false, enum: enumValues, description: enumDescription };
const expected = { ...bodyProperty, name, rootName, bodyPath, required: false, enum: enumValues, description: enumDescription, type: undefined };
expect(flatParamsMap.get(nestedEnumPropertyName)).eql(expected);
});
});
Expand Down
22 changes: 8 additions & 14 deletions test/unit/commands/smapi/smapi-command-handler-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,35 +14,28 @@ const smapiCommandHandler = require('@src/commands/smapi/smapi-command-handler')
describe('Smapi test - smapiCommandHandler function', () => {
const apiOperationName = 'someApiOperation';
const skillId = 'some skill id';
const inputContent = 'hello';
const deviceLocale = 'en-US';
const someNumber = '20';
const someBoolean = 'true';
const jsonValue = { test: 'test' };
const arrayValue = ['test', 'test1', 'test2'];
const arrayValueStr = arrayValue.join(ARRAY_SPLIT_DELIMITER);

const params = [
{ name: 'skillId', in: 'path' },
{ name: 'someNonPopulatedProperty', in: 'query' },
{ name: 'simulationsApiRequest', in: 'body' },
{ name: 'someJson', in: 'body' },
{ name: 'someArray', in: 'query' }
];
const flatParamsMap = new Map([
['skillId', { name: 'skillId' }],
['someJson', { name: 'someJson', json: true }],
['someArray', { name: 'someArray', isArray: true }],
['inputContent', { rootName: 'simulationsApiRequest', bodyPath: 'input>>>content' }],
['deviceLocale', { rootName: 'simulationsApiRequest', bodyPath: 'device>>>locale' }],
['someNumber', { rootName: 'simulationsApiRequest', bodyPath: 'input>>>someNumber', isNumber: true }],
['someBoolean', { rootName: 'simulationsApiRequest', bodyPath: 'input>>>someBoolean', isBoolean: true }],
['sessionMode', { rootName: 'simulationsApiRequest', bodyPath: 'session>>>mode' }]]);

const commanderToApiCustomizationMap = new Map();
const cmdObj = {
opts() {
return { skillId, inputContent, deviceLocale, someJson: JSON.stringify(jsonValue), someArray: arrayValueStr };
return { skillId, someNumber, someBoolean, someJson: JSON.stringify(jsonValue), someArray: arrayValueStr };
}
};

const clientStub = {};
const clientStub = { apiConfiguration: { apiEndpoint: null } };
beforeEach(() => {
sinon.stub(AuthorizationController.prototype, '_getAuthClientInstance').returns(
{ config: {} }
Expand All @@ -65,7 +58,8 @@ describe('Smapi test - smapiCommandHandler function', () => {
it('| should send smapi command with correct parameter mapping', async () => {
await smapiCommandHandler(apiOperationName, flatParamsMap, commanderToApiCustomizationMap, cmdObj);

const expectedParams = [jsonValue, skillId, null, arrayValue, { input: { content: inputContent }, device: { locale: deviceLocale } }];
const expectedParams = [jsonValue, skillId, null, arrayValue,
{ input: { someNumber: Number(someNumber), someBoolean: Boolean(someBoolean) } }];
const calledParams = clientStub[apiOperationName].args[0];

expect(calledParams).eql(expectedParams);
Expand Down

0 comments on commit 6998837

Please sign in to comment.