Skip to content

Commit

Permalink
fix: address comment to avoid checking fs.existsSync every time
Browse files Browse the repository at this point in the history
  • Loading branch information
@RonWang
RonWang committed Nov 9, 2020
1 parent eae53b5 commit 2655742
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 30 deletions.
43 changes: 25 additions & 18 deletions lib/commands/skill/add-locales/helper.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,33 +43,35 @@ function initiateModels(profile) {
* @param {Function} callback
*/
function addLocales(selectedLocales, profile, doDebug, callback) {
// analyze all the selected locales, whether copy from local existing files or get from remote templates
_getNewLocaleModelUri(selectedLocales, profile, doDebug, (resolveErr, iModelSourceByLocales) => {
if (resolveErr) {
return callback(resolveErr);
}
const uniqueTemplateSet = new Set(iModelSourceByLocales.values());
const iModelFolderPath = path.join(process.cwd(), ResourcesConfig.getInstance().getSkillMetaSrc(profile),
CONSTANTS.FILE_PATH.SKILL_PACKAGE.INTERACTION_MODEL, 'custom');
// copy iModel JSONs that contains same language
iModelSourceByLocales.forEach((v, k) => {
if (fs.existsSync(v)) {
const sourceLocale = path.basename(v, path.extname(v));
const targetFilePath = path.join(iModelFolderPath, `${k}.json`);
fs.copySync(v, targetFilePath); // do not fail if the target locale exists already
uniqueTemplateSet.delete(v);
const sourceManifestLocale = Manifest.getInstance().getPublishingLocale(sourceLocale);
Manifest.getInstance().setPublishingLocale(k, sourceManifestLocale);
}
});
const remoteTemplateSet = new Set(
[...iModelSourceByLocales.values()].reduce((acc, cur) => {
if (!cur.canCopy) { acc.push(cur.uri); }
return acc;
}, [])
);
// retrieve remote templates when no existing JSON shares the same language
_retrieveTemplatesByLanguage(uniqueTemplateSet, doDebug, (templateErr, templateByLanguage) => {
_retrieveTemplatesByLanguage(remoteTemplateSet, doDebug, (templateErr, templateByLanguage) => {
if (templateErr) {
return callback(templateErr);
}
// copy iModel JSONs that contains same language
iModelSourceByLocales.forEach((v, k) => {
if (!fs.existsSync(v)) {
if (v.canCopy) {
const sourceLocale = path.basename(v.uri, path.extname(v.uri));
const targetFilePath = path.join(iModelFolderPath, `${k}.json`);
fs.copySync(v.uri, targetFilePath); // do not fail if the target locale exists already
const sourceManifestLocale = Manifest.getInstance().getPublishingLocale(sourceLocale);
Manifest.getInstance().setPublishingLocale(k, sourceManifestLocale);
} else {
const targetFilePath = path.join(iModelFolderPath, `${k}.json`);
fs.writeFileSync(targetFilePath, templateByLanguage.get(v), { encoding: 'utf-8' });
fs.writeFileSync(targetFilePath, templateByLanguage.get(v.uri), { encoding: 'utf-8' });
Manifest.getInstance().setPublishingLocale(k, { name: 'please change' });
}
});
Expand All @@ -86,7 +88,7 @@ function addLocales(selectedLocales, profile, doDebug, callback) {
* @param {Array} selectedLocales
* @param {String} profile
* @param {Boolean} doDebug
* @returns {Function} callback Map { locale: filePath/remoteURI }
* @returns {Function} callback Map { locale: { uri, canCopy} } where the uri is filePath or remoteURI
*/
function _getNewLocaleModelUri(selectedLocales, profile, doDebug, callback) {
const skillMetaController = new SkillMetadataController({ profile, doDebug });
Expand Down Expand Up @@ -114,9 +116,9 @@ ${JSON.stringify(templateMapResponse, null, 2)}`);
const language = languageExtractor(locale);
const reusableLocale = R.find((k) => languageExtractor(k) === language)(R.keys(localIModelByLocale));
if (reusableLocale) {
result.set(locale, localIModelByLocale[reusableLocale]);
result.set(locale, { uri: localIModelByLocale[reusableLocale], canCopy: true });
} else {
result.set(locale, templateIndexMap.INTERACTION_MODEL_BY_LANGUAGE[language]);
result.set(locale, { uri: templateIndexMap.INTERACTION_MODEL_BY_LANGUAGE[language], canCopy: false });
}
}
callback(undefined, result);
Expand All @@ -131,6 +133,11 @@ ${JSON.stringify(templateMapResponse, null, 2)}`);
*/
function _retrieveTemplatesByLanguage(templateSet, doDebug, callback) {
const result = new Map();
if (templateSet.size === 0) {
return process.nextTick(() => {
callback(undefined, result);
});
}
async.forEach(templateSet.values(), (templateUrl, loopCallback) => {
httpClient.request({
url: templateUrl,
Expand Down
7 changes: 3 additions & 4 deletions lib/commands/skill/add-locales/ui.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
const fs = require('fs');
const inquirer = require('inquirer');
const path = require('path');

Expand Down Expand Up @@ -30,7 +29,7 @@ function selectLocales(localeChoices, callback) {
/**
* Display the result of locales addition
* @param {Array} selectedLocales selected locales
* @param {Map} iModelSourceByLocales Map { locale: filePath/remoteURI }
* @param {Map} iModelSourceByLocales Map { locale: { uri, canCopy} }
*/
function displayAddLocalesResult(selectedLocales, iModelSourceByLocales) {
for (const locale of selectedLocales) {
Expand All @@ -45,8 +44,8 @@ function displayAddLocalesResult(selectedLocales, iModelSourceByLocales) {

Messenger.getInstance().info('The following skill locale(s) have been added according to your local project:');
iModelSourceByLocales.forEach((v, k) => {
if (fs.existsSync(v)) {
const sourceLocale = path.basename(v, path.extname(v));
if (v.canCopy) {
const sourceLocale = path.basename(v.uri, path.extname(v.uri));
Messenger.getInstance().info(` Added locale ${k}.json from ${sourceLocale}'s interactionModel`);
} else {
Messenger.getInstance().info(` Added locale ${k}.json from the template of interactionModel`);
Expand Down
4 changes: 2 additions & 2 deletions test/unit/commands/skill/add-locales/helper-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ ${JSON.stringify({ statusCode: 401 }, null, 2)}`);
expect(fs.copySync.args[0][0]).equal(TEST_MODEL_PATH);
expect(err).equal(undefined);
expect(result.size).equal(1);
expect(result.get('en-US')).equal(TEST_MODEL_PATH);
expect(result.get('en-US')).deep.equal({ uri: TEST_MODEL_PATH, canCopy: true });
done();
});
});
Expand Down Expand Up @@ -179,7 +179,7 @@ ${JSON.stringify({ statusCode: 401 }, null, 2)}`);
helper.addLocales(['en-US'], TEST_PROFILE, TEST_DEBUG, (err, result) => {
expect(fs.writeFileSync.args[0][1]).equal(TEST_TEMPLATE_BODY.body);
expect(result.size).equal(1);
expect(result.get('en-US')).equal('TEST_EN_URL');
expect(result.get('en-US')).deep.equal({ uri: 'TEST_EN_URL', canCopy: false });
expect(err).equal(undefined);
done();
});
Expand Down
8 changes: 2 additions & 6 deletions test/unit/commands/skill/add-locales/ui-test.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
const { expect } = require('chai');
const inquirer = require('inquirer');
const sinon = require('sinon');
const fs = require('fs');

const Messenger = require('@src/view/messenger');

Expand Down Expand Up @@ -90,13 +89,10 @@ describe('Commands add-locales - UI test', () => {
it('| display info message when all selected locales are added', () => {
// setup
const TEST_MAP = new Map([
['1', 'file1.json'],
['2', 'file2.json']
['1', { uri: 'file1.json', canCopy: true }],
['2', { uri: 'file2.json', canCopy: false }]
]);
const TEST_LIST = ['1', '2', '3'];
sinon.stub(fs, 'existsSync');
fs.existsSync.withArgs('file1.json').returns(true);
fs.existsSync.withArgs('file2.json').returns(false);
// call
ui.displayAddLocalesResult(TEST_LIST, TEST_MAP);
// verify
Expand Down

0 comments on commit 2655742

Please sign in to comment.