fix for bf orchestrator:add command (#1236)

* updated help text for routingName param

* orchestrator:add fixes

* Update datasourcehelper.ts

don't override routingname from user

* updated tests

* added add source test

Author: tsuwandy

packages/orchestrator/src/commands/orchestrator/add.ts

@@ -15,8 +15,8 @@ export default class OrchestratorAdd extends Command {
     $ bf orchestrator:add --in ./path/to/file/ --snapshot ./path/to/snapshot/	
     $ bf orchestrator:add --in ./path/to/file/ --snapshot ./path/to/snapshot/ --out ./path/to/output/	
     $ bf orchestrator:add --in ./path/to/file/ --out ./path/to/output/ --model ./path/to/model/directory
-    $ bf orchestrator:add -t luis --id LUIS_APP_ID --version LUIS_APP_VERSION --key LUIS_KEY --routingname l_Weather --endpoint 
-    $ bf orchestrator:add -t qna --id QNA_KB  --key QNA_KB_SERVICE_KEY --routingname q_kb
+    $ bf orchestrator:add -t luis --id LUIS_APP_ID --version LUIS_APP_VERSION --key LUIS_KEY --routingName l_Weather --endpoint 
+    $ bf orchestrator:add -t qna --id QNA_KB  --key QNA_KB_SERVICE_KEY --routingName q_kb
     `]
 
   static flags: flags.Input<any> = {
@@ -61,7 +61,7 @@ export default class OrchestratorAdd extends Command {
         if (type === 'luis' && Utility.isEmptyString(endpoint)) {
           throw new CLIError('LUIS endpoint required, ie --endpoint https://westus.api.cognitive.microsoft.com');
         }
-        OrchestratorSettings.init(cwd, baseModelPath, entityBaseModelPath, output, true);
+        OrchestratorSettings.init(cwd, baseModelPath, entityBaseModelPath, output, true, true);
         const dataSource: OrchestratorDataSource = new OrchestratorDataSource(id, key, version, endpoint, type, routingName, OrchestratorSettings.DataSources.path);
         await DataSourceHelper.ensureDataSourceAsync(dataSource, OrchestratorSettings.DataSources.path);
 

packages/orchestratorlib/src/datasourcehelper.ts

@@ -128,7 +128,7 @@ export class DataSourceHelper {
         }
         input.FilePath = path.join(filePath, input.RoutingName + '.lu');
       } else if (input.Type === 'qna') {
-        input.RoutingName = Utility.isEmptyString(input.RoutingName) ? `q_${input.Id}` : `q_${input.RoutingName}`;
+        input.RoutingName = Utility.isEmptyString(input.RoutingName) ? `q_${input.Id}` : `${input.RoutingName}`;
         input.FilePath = path.join(filePath, input.RoutingName + '.qna');
       } else {
         throw new Error(`Invalid content for type ${input.Type}`);

packages/orchestratorlib/src/orchestratorhelper.ts

@@ -249,6 +249,24 @@ export class OrchestratorHelper {
     return entitiesList;
   }
 
+  public static jsonStringify(obj: any): string {
+    return JSON.stringify(
+      obj,
+      (key: string, value: any) => {
+        if (value && typeof value === 'object' && !Array.isArray(value)) {
+          const replacement: any = {};
+          for (const k in value) {
+            if (Object.hasOwnProperty.call(value, k)) {
+              replacement[k && k.charAt(0).toLowerCase() + k.substring(1)] = value[k];
+            }
+          }
+          return replacement;
+        }
+        return value;
+      },
+      2);
+  }
+
   // eslint-disable-next-line max-params
   static async processFile(
     filePath: string,

packages/orchestratorlib/src/settings.ts

@@ -157,7 +157,8 @@ export class OrchestratorSettings {
     baseModelPath: string,
     entityBaseModelPath: string,
     snapshotPath: string,
-    hierarchical: boolean = false)  {
+    hierarchical: boolean = false,
+    hasDataSources: boolean = false)  {
     settingsDir = path.resolve(settingsDir);
     const settingsFile: string = path.join(settingsDir, OrchestratorSettings.OrchestratorSettingsFileName);
     OrchestratorSettings.SettingsPath = settingsFile;
@@ -171,8 +172,13 @@ export class OrchestratorSettings {
       let settings: any;
       if (settingsFileExists) {
         settings = JSON.parse(OrchestratorSettings.readFile(settingsFile));
+        if (settings.dataSources && settings.dataSources.inputs) {
+          hasDataSources = true;
+        }
+      }
+      if (hasDataSources) {
+        OrchestratorSettings.ensureDataSources(hierarchical, settings, settingsDir);
       }
-      OrchestratorSettings.ensureDataSources(hierarchical, settings, settingsDir);
       OrchestratorSettings.SnapshotPath = OrchestratorSettings.ensureSnapshotPath(snapshotPath, settingsDir, settings);
       OrchestratorSettings.ModelPath = OrchestratorSettings.ensureBaseModelPath(baseModelPath, settings);
       entityBaseModelPath = OrchestratorSettings.ensureEntityBaseModelPath(entityBaseModelPath, settings);
@@ -198,7 +204,7 @@ export class OrchestratorSettings {
         dataSources: OrchestratorSettings.DataSources,
       };
 
-      OrchestratorSettings.writeToFile(OrchestratorSettings.SettingsPath, Utility.jsonStringify(settings, null, 2));
+      OrchestratorSettings.writeToFile(OrchestratorSettings.SettingsPath, OrchestratorHelper.jsonStringify(settings));
     } catch (error) {
       throw new Error(error);
     }

packages/orchestratorlib/test/settings.test.ts

@@ -9,10 +9,9 @@ import assert = require('assert');
 import * as path from 'path';
 import * as fs from 'fs-extra';
 
-import {OrchestratorSettings} from '../src/settings';
+import {OrchestratorDataSource, OrchestratorSettings} from '../src/settings';
 
 import {Utility} from '../src/utility';
-import {UnitTestHelper} from './utility.test';
 
 describe('OrchestratorSettingsTests', () => {
   const SettingsDir: string = './test/fixtures/';
@@ -44,10 +43,15 @@ describe('OrchestratorSettingsTests', () => {
     assert.ok(OrchestratorSettings.SettingsPath === SettingsFile);
     assert.ok(OrchestratorSettings.ModelPath === BaseModelDir);
     assert.ok(OrchestratorSettings.EntityModelPath === '');
+    assert.ok(!OrchestratorSettings.DataSources);
+  });
 
-    Utility.resetFlagToPrintDebuggingLogToConsole(UnitTestHelper.getDefaultUnitTestDebuggingLogFlag());
-    Utility.debuggingLog(`OrchestratorSettings.DataSources.Path=${OrchestratorSettings.DataSources.path}`);
-
+  it('init settings with no settings file + data source', () => {
+    OrchestratorSettings.init(SettingsDir, BaseModelDir, '', SettingsDir, false, true);
+    assert.ok(OrchestratorSettings.SettingsPath === SettingsFile);
+    assert.ok(OrchestratorSettings.ModelPath === BaseModelDir);
+    assert.ok(OrchestratorSettings.EntityModelPath === '');
+    assert.ok(OrchestratorSettings.DataSources);
     assert.ok(OrchestratorSettings.DataSources.path === DataSourcesPath);
     assert.ok(OrchestratorSettings.DataSources.inputs.length === 0);
     assert.ok(OrchestratorSettings.DataSources.hierarchical === false);
@@ -114,6 +118,27 @@ describe('OrchestratorSettingsTests', () => {
     assert.ok(OrchestratorSettings.DataSources.inputs[0].Type === 'qna');
     assert.ok(OrchestratorSettings.DataSources.inputs[1].Type === 'luis');
     assert.ok(OrchestratorSettings.DataSources.hierarchical);
+
+    const dataSource: OrchestratorDataSource = new OrchestratorDataSource(
+      'a5ee4d79-28e0-4757-a9f8-45ab64ee1f7e',
+      'LUISKEY',
+      'version',
+      'https://westus.api.cognitive.microsoft.com',
+      'luis',
+      'l_HomeAutomation',
+      OrchestratorSettings.DataSources.path);
+
+    OrchestratorSettings.addUpdateDataSource(dataSource);
+    OrchestratorSettings.persist();
+    OrchestratorSettings.init(SettingsDir, BaseModelDir, '', SettingsDir);
+    assert.ok(OrchestratorSettings.DataSources.hierarchical);
+    assert.ok(OrchestratorSettings.DataSources.inputs.length === 3);
+    assert.ok(OrchestratorSettings.DataSources.inputs[0].Type === 'qna');
+    assert.ok(OrchestratorSettings.DataSources.inputs[0].Id === '213a48d3-855d-4083-af6d-339c03d497dd');
+    assert.ok(OrchestratorSettings.DataSources.inputs[1].Type === 'luis');
+    assert.ok(OrchestratorSettings.DataSources.inputs[1].Id === 'd06d7acf-a9ec-43e0-94c6-3b37ee313a21');
+    assert.ok(OrchestratorSettings.DataSources.inputs[2].Type === 'luis');
+    assert.ok(OrchestratorSettings.DataSources.inputs[2].Id === 'a5ee4d79-28e0-4757-a9f8-45ab64ee1f7e');
   });
 
   it('init settings with settings file', () => {