Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: parse order of parameters from function source code #112

Merged
merged 10 commits into from
Apr 10, 2020
Merged

fix: parse order of parameters from function source code #112

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
aaf3057
fix: parse order of parameters from function source code
Apr 9, 2020
76c7a01
chore: add auto register for hook when vendorId parameter
Apr 9, 2020
f74209a
chore: add TestersRequest to auto register hooks
Apr 9, 2020
614da6a
chore: change case of testersRequest parameter
Apr 9, 2020
e01a75d
chore: update test
Apr 9, 2020
5e039fe
chore: match when there is case difference between swagger and smapi …
Apr 9, 2020
fa25313
chore: freezing ask-smapi-model version
Apr 10, 2020
e4399f6
fix: add JSON flag for complex parameter inside of the body
Apr 10, 2020
cd6c6a6
test: improve test coverage
Apr 10, 2020
1782c15
chore: remove ask-smapi-model
Apr 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/commands/smapi/customizations/hook-functions/map-testers-emails.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/
const mapTestersEmails = (requestParameters) => {
const { testersEmails } = requestParameters;
requestParameters.TestersRequest = {
requestParameters.testersRequest = {
testers: testersEmails.map(email => ({ emailId: email }))
};

Expand Down
35 changes: 24 additions & 11 deletions lib/commands/smapi/customizations/smapi-hooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const { ModelIntrospector } = require('ask-smapi-sdk');
const appendVendorId = require('@src/commands/smapi/customizations/hook-functions/append-vendor-id');
const mapTestersEmails = require('@src/commands/smapi/customizations/hook-functions/map-testers-emails');

Expand All @@ -7,19 +8,31 @@ const events = {

const operationHooks = new Map();

operationHooks.set('createSkillForVendorV1', new Map([[events.BEFORE_SEND, appendVendorId]]));
operationHooks.set('createIspForVendorV1', new Map([[events.BEFORE_SEND, appendVendorId]]));
operationHooks.set('getIspListForVendorV1', new Map([[events.BEFORE_SEND, appendVendorId]]));
operationHooks.set('getAlexaHostedSkillUserPermissionsV1', new Map([[events.BEFORE_SEND, appendVendorId]]));
operationHooks.set('listInteractionModelCatalogsV1', new Map([[events.BEFORE_SEND, appendVendorId]]));
operationHooks.set('listInteractionModelSlotTypesV1', new Map([[events.BEFORE_SEND, appendVendorId]]));
operationHooks.set('listSkillsForVendorV1', new Map([[events.BEFORE_SEND, appendVendorId]]));
operationHooks.set('addTestersToBetaTestV1', new Map([[events.BEFORE_SEND, mapTestersEmails]]));
operationHooks.set('removeTestersFromBetaTestV1', new Map([[events.BEFORE_SEND, mapTestersEmails]]));
operationHooks.set('sendReminderToTestersV1', new Map([[events.BEFORE_SEND, mapTestersEmails]]));
operationHooks.set('requestFeedbackFromTestersV1', new Map([[events.BEFORE_SEND, mapTestersEmails]]));
const _autoRegisterHooks = () => {
RonWang marked this conversation as resolved.
Show resolved Hide resolved
const modelIntrospector = new ModelIntrospector();
const operations = modelIntrospector.getOperations();
operations.forEach(operation => {
operation.params.forEach(param => {
switch (param.name) {
case 'vendorId':
operationHooks.set(operation.apiOperationName, new Map([[events.BEFORE_SEND, appendVendorId]]));
break;
case 'TestersRequest':
operationHooks.set(operation.apiOperationName, new Map([[events.BEFORE_SEND, mapTestersEmails]]));
break;
default:
}
});
});
};

const _manualRegisterHooks = () => {
operationHooks.set('createIspForVendorV1', new Map([[events.BEFORE_SEND, appendVendorId]]));
operationHooks.set('createSkillForVendorV1', new Map([[events.BEFORE_SEND, appendVendorId]]));
};

_autoRegisterHooks();
_manualRegisterHooks();
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick, add a new line after this line

class SmapiHooks {
/**
* Returns object with available hook events.
Expand Down
19 changes: 11 additions & 8 deletions lib/commands/smapi/smapi-command-handler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const jsonView = require('@src/view/json-view');
const Messenger = require('@src/view/messenger');
const profileHelper = require('@src/utils/profile-helper');
const unflatten = require('@src/utils/unflatten');
const { getParamNames } = require('@src/utils/string-utils');

const { BODY_PATH_DELIMITER, ARRAY_SPLIT_DELIMITER } = require('./cli-customization-processor');
const SmapiHooks = require('./customizations/smapi-hooks');
Expand All @@ -18,12 +19,13 @@ const configFilePath = path.join(os.homedir(), CONSTANTS.FILE_PATH.ASK.HIDDEN_FO
const _mapToArgs = (params, paramsObject) => {
const res = [];
params.forEach(param => {
const { name } = param;
if (paramsObject[name]) {
res.push(paramsObject[name]);
} else {
res.push(null);
}
let value = null;
Object.keys(paramsObject).forEach(k => {
if (k.toLowerCase() === param.toLowerCase()) {
RonWang marked this conversation as resolved.
Show resolved Hide resolved
value = paramsObject[k];
}
});
res.push(value);
});
return res;
};
Expand Down Expand Up @@ -62,7 +64,7 @@ const _mapToParams = (optionsValues, flatParamsMap, commanderToApiCustomizationM
* for api properties.
* @param {Object} cmdObj Commander object with passed values.
*/
const smapiCommandHandler = (swaggerApiOperationName, swaggerParams, flatParamsMap, commanderToApiCustomizationMap, inputCmdObj) => {
const smapiCommandHandler = (swaggerApiOperationName, flatParamsMap, commanderToApiCustomizationMap, inputCmdObj) => {
new AppConfig(configFilePath);
const authorizationController = new AuthorizationController({
auth_client_type: 'LWA'
Expand Down Expand Up @@ -90,7 +92,8 @@ const smapiCommandHandler = (swaggerApiOperationName, swaggerParams, flatParamsM
Messenger.getInstance().info('Payload:');
Messenger.getInstance().info(jsonView.toString(paramsObject));
}
const args = _mapToArgs(swaggerParams, paramsObject);
const params = getParamNames(client[swaggerApiOperationName]);
const args = _mapToArgs(params, paramsObject);
return client[swaggerApiOperationName](...args);
};

Expand Down
1 change: 0 additions & 1 deletion lib/commands/smapi/smapi-commander.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ const makeSmapiCommander = () => {
});
commanderInstance.action((inputCmdObj) => smapiCommandHandler(
apiOperationName,
operation.params,
flatParamsMap,
commanderToApiCustomizationMap,
inputCmdObj
Expand Down
10 changes: 10 additions & 0 deletions lib/utils/string-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ const R = require('ramda');
const CONSTANTS = require('@src/utils/constants');

module.exports = {
getParamNames,
camelCase,
kebabCase,
isNonEmptyString,
Expand All @@ -12,6 +13,15 @@ module.exports = {
validateSyntax
};

function getParamNames(func) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this function need unit test

const STRIP_COMMENTS = /((\/\/.*$)|(\/\*[\s\S]*?\*\/))/mg;
const ARGUMENT_NAMES = /([^\s,]+)/g;
Comment on lines +17 to +18
Copy link
Contributor

Choose a reason for hiding this comment

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

We have a REGEX_VALIDATIONS in constants.js, should we try to put regex in one place?

const fnStr = func.toString().replace(STRIP_COMMENTS, '');
let result = fnStr.slice(fnStr.indexOf('(') + 1, fnStr.indexOf(')')).match(ARGUMENT_NAMES);
if (result === null) result = [];
return result;
}

function camelCase(str) {
return str
.replace(/(?:^\w|[A-Z]|\b\w)/g, (word, index) => (index === 0 ? word.toLowerCase() : word.toUpperCase()))
Expand Down
4 changes: 2 additions & 2 deletions test/unit/commands/smapi/hook-functions/map-testers-emails-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ const mapTestersEmails = require('@src/commands/smapi/customizations/hook-functi


describe('Smapi hook functions test - mapTestersEmails tests', () => {
it('| should map testers emails to TestersRequest parameter', () => {
it('| should map testers emails to testersRequest parameter', () => {
const requestParameters = { testersEmails: ['tester1', 'tester2'] };

mapTestersEmails(requestParameters);

const expected = { testers: [{ emailId: 'tester1' }, { emailId: 'tester2' }] };

expect(requestParameters.TestersRequest).eql(expected);
expect(requestParameters.testersRequest).eql(expected);
});
});
10 changes: 6 additions & 4 deletions test/unit/commands/smapi/smapi-command-handler-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,16 @@ describe('Smapi test - smapiCommandHandler function', () => {
}
});
clientStub[apiOperationName] = sinon.stub().resolves();
clientStub[apiOperationName].toString = () => 'function (someJson, skillId, '
+ 'someNonPopulatedProperty, someArray, simulationsApiRequest) { return 0};';
sinon.stub(StandardSmapiClientBuilder.prototype, 'client').returns(clientStub);
});


it('| should send smapi command with correct parameter mapping', async () => {
await smapiCommandHandler(apiOperationName, params, flatParamsMap, commanderToApiCustomizationMap, cmdObj);
await smapiCommandHandler(apiOperationName, flatParamsMap, commanderToApiCustomizationMap, cmdObj);

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

expect(calledParams).eql(expectedParams);
Expand All @@ -73,7 +75,7 @@ describe('Smapi test - smapiCommandHandler function', () => {
const stubHook = sinon.stub();
sinon.stub(SmapiHooks, 'getFunction').returns(stubHook);

await smapiCommandHandler(apiOperationName, params, flatParamsMap, commanderToApiCustomizationMap, cmdObj);
await smapiCommandHandler(apiOperationName, flatParamsMap, commanderToApiCustomizationMap, cmdObj);

expect(stubHook.calledOnce).eql(true);
});
Expand All @@ -87,7 +89,7 @@ describe('Smapi test - smapiCommandHandler function', () => {

const messengerStub = sinon.stub(Messenger, 'displayMessage');

await smapiCommandHandler(apiOperationName, params, flatParamsMap, commanderToApiCustomizationMap, cmdObjDebug);
await smapiCommandHandler(apiOperationName, flatParamsMap, commanderToApiCustomizationMap, cmdObjDebug);

expect(messengerStub.args[0]).eql(['INFO', 'Operation: someApiOperation']);
expect(messengerStub.args[1]).eql(['INFO', 'Payload:']);
Expand Down