add --force and --log arg for cross-train cli

packages/lu/src/parser/cross-train/cross-train.js

@@ -7,8 +7,6 @@ const fs = require('fs-extra')
 const path = require('path')
 const file = require('../../utils/filehelper')
 const fileExtEnum = require('../utils/helpers').FileExtTypeEnum
-const exception = require('../utils/exception')
-const retCode = require('../utils/enums/CLI-errors')
 const crossTrainer = require('./crossTrainer')
 const confighelper = require('./confighelper')
 
@@ -30,54 +28,19 @@ module.exports = {
    * @param {string} input full path of input lu and qna files folder.
    * @param {string} intentName interruption intent name. Default value is _Interruption.
    * @param {string} config path to config of mapping rules or mapping rules json content itself. If undefined, it will read config.json from input folder.
+   * @param {boolean} verbose verbose to indicate whether log warnings and errors or not when parsing cross-train files.
    * @returns {luResult: any, qnaResult: any} trainedResult of luResult and qnaResult or undefined if no results.
    */
-  train: async function (input, intentName, config) {
+  train: async function (input, intentName, config, verbose) {
     // Get all related file content.
     const luContents = await file.getFilesContent(input, fileExtEnum.LUFile)
     const qnaContents = await file.getFilesContent(input, fileExtEnum.QnAFile)
     const configContent = config && !fs.existsSync(config) ? {id: path.join(input, 'config.json'), content: config} : await file.getConfigContent(config)
 
-    const configObject = file.getConfigObject(configContent, intentName)
+    const configObject = file.getConfigObject(configContent, intentName, verbose)
 
     const trainedResult = await crossTrainer.crossTrain(luContents, qnaContents, configObject)
 
     return trainedResult
-  },
-
-  /**
-   * Write lu and qna files
-   * @param {Map<string, any>} fileIdToLuResourceMap lu or qna file id to lu resource map.
-   * @param {string} out output folder name. If not specified, source lu and qna files will be updated.
-   * @throws {exception} Throws on errors.
-   */
-  writeFiles: async function (fileIdToLuResourceMap, out) {
-    if (fileIdToLuResourceMap) {
-      let newFolder
-      if (out) {
-        newFolder = out
-        if (!path.isAbsolute(out)) {
-          newFolder = path.resolve(out)
-        }
-
-        if (!fs.existsSync(newFolder)) {
-          fs.mkdirSync(newFolder)
-        }
-      }
-
-      for (const fileId of fileIdToLuResourceMap.keys()) {
-        try {
-          if (newFolder) {
-            const fileName = path.basename(fileId)
-            const newFileId = path.join(newFolder, fileName)
-            await fs.writeFile(newFileId, fileIdToLuResourceMap.get(fileId).Content, 'utf-8')
-          } else {
-            await fs.writeFile(fileId, fileIdToLuResourceMap.get(fileId).Content, 'utf-8')
-          }
-        } catch (err) {
-          throw (new exception(retCode.errorCode.OUTPUT_FOLDER_INVALID, `Unable to write to file ${fileId}. Error: ${err.message}`))
-        }
-      }
-    }
   }
 }

packages/lu/src/parser/cross-train/crossTrainer.js

@@ -494,11 +494,11 @@ const parseAndValidateContent = async function (objectArray, verbose) {
     let fileContent = object.content
     if (object.content && object.content !== '') {
       if (object.id.toLowerCase().endsWith(fileExtEnum.LUFile)) {
-        let result = await LuisBuilderVerbose.build([object], true)
+        let result = await LuisBuilderVerbose.build([object], verbose)
         let luisObj = new Luis(result)
         fileContent = luisObj.parseToLuContent()
       } else {
-        let result = await qnaBuilderVerbose.build([object], true)
+        let result = await qnaBuilderVerbose.build([object], verbose)
         fileContent = result.parseToQnAContent()
       }
     }

packages/lu/src/utils/filehelper.ts

@@ -218,7 +218,7 @@ export function getParsedObjects(contents: {id: string, content: string}[]) {
   return parsedObjects
 }
 
-export function getConfigObject(configContent: any, intentName: string) {
+export function getConfigObject(configContent: any, intentName: string, verbose: boolean) {
   let finalLuConfigObj = Object.create(null)
   let rootLuFiles: string[] = []
   const configFileDir = path.dirname(configContent.id)
@@ -267,7 +267,7 @@ export function getConfigObject(configContent: any, intentName: string) {
       rootIds: rootLuFiles,
       triggerRules: finalLuConfigObj,
       intentName,
-      verbose: true
+      verbose
     }
 
     return crossTrainConfig

packages/lu/test/utils/filehelper.test.js

@@ -52,7 +52,7 @@ describe('utils/filehelper test', () => {
             }
         }
 
-        let configObject = fileHelper.getConfigObject({ id: path.join(__dirname, 'config.json'), content: JSON.stringify(configContent) }, '_Interruption')
+        let configObject = fileHelper.getConfigObject({ id: path.join(__dirname, 'config.json'), content: JSON.stringify(configContent) }, '_Interruption', true)
         assert.equal(configObject.rootIds[0].includes('main.lu'), true)
         assert.equal(configObject.rootIds[1].includes('main.fr-fr.lu'), true)
 

packages/luis/README.md

@@ -414,18 +414,22 @@ USAGE
   $ bf luis:cross-train
 
 OPTIONS
-  -h, --help               luis:cross-train help
-  -i, --in=in              source lu and qna files folder
+  -h, --help               Luis:cross-train help
+  -i, --in=in              Source lu and qna files folder
 
-  -o, --out=out            output folder name. If not specified, the cross trained files will be written to cross-trained
+  -o, --out=out            Output folder name. If not specified, the cross trained files will be written to cross-trained
                            folder under folder of current command
 
-  --config=config          path to config file of mapping rules
+  --config=config          Path to config file of mapping rules
 
   --intentName=intentName  [default: _Interruption] Interruption intent name
 
-  --rootDialog=rootDialog  rootDialog file path. If --config not specified,
+  --rootDialog=rootDialog  RootDialog file path. If --config not specified,
                            cross-trian will automatically construct the config from file system based on root dialog file
+
+  -f, --force              [default: false] If --out flag is provided with the path to an existing file, overwrites that file
+
+  --log                    [default: false] Write out log messages to console
 ```
 
 _See code: [src/commands/luis/cross-train.ts](https://github.com/microsoft/botframework-cli/tree/master/packages/luis/src/commands/luis/cross-train.ts)_

packages/luis/src/commands/luis/cross-train.ts

@@ -3,21 +3,24 @@
  * Licensed under the MIT License.
  */
 
-import {CLIError, Command, flags} from '@microsoft/bf-cli-command'
+import {CLIError, Command, flags, utils} from '@microsoft/bf-cli-command'
+const fs = require('fs-extra')
+const path = require('path')
 const crossTrain = require('@microsoft/bf-lu/lib/parser/cross-train/cross-train')
 const exception = require('@microsoft/bf-lu/lib/parser/utils/exception')
-const path = require('path')
 
 export default class LuisCrossTrain extends Command {
   static description = 'Lu and Qna cross train tool'
 
   static flags: flags.Input<any> = {
-    help: flags.help({char: 'h', description: 'luis:cross-train help'}),
-    in: flags.string({char: 'i', description: 'source lu and qna files folder'}),
-    out: flags.string({char: 'o', description: 'output folder name. If not specified, the cross trained files will be written to cross-trained folder under folder of current command'}),
-    config: flags.string({description: 'path to config file of mapping rules'}),
+    help: flags.help({char: 'h', description: 'Luis:cross-train help'}),
+    in: flags.string({char: 'i', description: 'Source lu and qna files folder'}),
+    out: flags.string({char: 'o', description: 'Output folder name. If not specified, the cross trained files will be written to cross-trained folder under folder of current command'}),
+    config: flags.string({description: 'Path to config file of mapping rules'}),
     intentName: flags.string({description: 'Interruption intent name', default: '_Interruption'}),
-    rootDialog: flags.string({description: 'rootDialog file path. If --config not specified, cross-trian will automatically construct the config from file system based on root dialog file'})
+    rootDialog: flags.string({description: 'RootDialog file path. If --config not specified, cross-trian will automatically construct the config from file system based on root dialog file'}),
+    force: flags.boolean({char: 'f', description: 'If --out flag is provided with the path to an existing file, overwrites that file', default: false}),
+    log: flags.boolean({description: 'Write out log messages to console', default: false})
   }
 
   async run() {
@@ -39,19 +42,52 @@ export default class LuisCrossTrain extends Command {
         throw new CLIError('Missing cross train config. Please provide config by --config or automatically construct config with --rootDialog.')
       }
 
-      const trainedResult = await crossTrain.train(flags.in, flags.intentName, flags.config)
+      const trainedResult = await crossTrain.train(flags.in, flags.intentName, flags.config, flags.log)
 
       if (flags.out === undefined) {
         flags.out = path.join(process.cwd(), 'cross-trained')
       }
 
-      await crossTrain.writeFiles(trainedResult.luResult, flags.out)
-      await crossTrain.writeFiles(trainedResult.qnaResult, flags.out)
+      await this.writeFiles(trainedResult.luResult, flags.out, flags.force)
+      await this.writeFiles(trainedResult.qnaResult, flags.out, flags.force)
     } catch (err) {
       if (err instanceof exception) {
         throw new CLIError(err.text)
       }
       throw err
     }
   }
+
+  async writeFiles(fileIdToLuResourceMap: any, out: string, force: boolean) {
+    if (fileIdToLuResourceMap) {
+      let newFolder
+      if (out) {
+        newFolder = out
+        if (!path.isAbsolute(out)) {
+          newFolder = path.resolve(out)
+        }
+
+        if (!fs.existsSync(newFolder)) {
+          fs.mkdirSync(newFolder)
+        }
+      }
+
+      for (const fileId of fileIdToLuResourceMap.keys()) {
+        try {
+          let validatedPath
+          if (newFolder) {
+            const fileName = path.basename(fileId)
+            const newFileId = path.join(newFolder, fileName)
+            validatedPath = utils.validatePath(newFileId, '', force)
+          } else {
+            validatedPath = utils.validatePath(fileId, '', force)
+          }
+
+          await fs.writeFile(validatedPath, fileIdToLuResourceMap.get(fileId).Content, 'utf-8')
+        } catch (err) {
+          throw new CLIError(`Unable to write to file ${fileId}. Error: ${err.message}`)
+        }
+      }
+    }
+  }
 }

packages/luis/test/commands/luis/crossTrain.test.ts

@@ -39,7 +39,8 @@ describe('luis:cross-train tests for lu and qna contents', () => {
       '--in', `${path.join(__dirname, './../../fixtures/testcases/interruption')}`,
       '--intentName', '_Interruption',
       '--config', `${path.join(__dirname, './../../fixtures/testcases/interruption/mapping_rules.json')}`,
-      '--out', './interruptionGen'])
+      '--out', './interruptionGen',
+      '--force'])
     .it('luis:cross training can get expected result when handling multi locales and duplications', async () => {
       expect(await compareLuFiles('./../../../interruptionGen/Main.lu', './../../fixtures/verified/interruption/Main.lu')).to.be.true
       expect(await compareLuFiles('./../../../interruptionGen/Main.qna', './../../fixtures/verified/interruption/Main.qna')).to.be.true
@@ -62,7 +63,8 @@ describe('luis:cross-train tests for lu and qna contents', () => {
       '--in', `${path.join(__dirname, './../../fixtures/testcases/interruption2')}`,
       '--intentName', '_Interruption',
       '--config', `${path.join(__dirname, './../../fixtures/testcases/interruption2/config.json')}`,
-      '--out', './interruptionGen'])
+      '--out', './interruptionGen',
+      '--force'])
     .it('luis:cross training can get expected result when nestedIntentSection is enabled', async () => {
       expect(await compareLuFiles('./../../../interruptionGen/main.lu', './../../fixtures/verified/interruption2/main.lu')).to.be.true
       expect(await compareLuFiles('./../../../interruptionGen/dia1.lu', './../../fixtures/verified/interruption2/dia1.lu')).to.be.true
@@ -75,7 +77,8 @@ describe('luis:cross-train tests for lu and qna contents', () => {
       '--in', `${path.join(__dirname, './../../fixtures/testcases/interruption3')}`,
       '--intentName', '_Interruption',
       '--config', `${path.join(__dirname, './../../fixtures/testcases/interruption3/config.json')}`,
-      '--out', './interruptionGen'])
+      '--out', './interruptionGen',
+      '--force'])
     .it('luis:cross training can get expected result when multiple dialog invocations occur in same trigger', async () => {
       expect(await compareLuFiles('./../../../interruptionGen/main.lu', './../../fixtures/verified/interruption3/main.lu')).to.be.true
       expect(await compareLuFiles('./../../../interruptionGen/dia1.lu', './../../fixtures/verified/interruption3/dia1.lu')).to.be.true
@@ -89,7 +92,8 @@ describe('luis:cross-train tests for lu and qna contents', () => {
       '--in', './test/fixtures/testcases/interruption4',
       '--intentName', '_Interruption',
       '--out', './interruptionGen',
-      '--rootDialog', './test/fixtures/testcases/interruption4/main/main.dialog'])
+      '--rootDialog', './test/fixtures/testcases/interruption4/main/main.dialog',
+      '--force'])
     .it('luis:cross training can get expected result when automatically detecting config based on rootdialog and file system', async () => {
       expect(await compareLuFiles('./../../../interruptionGen/main.lu', './../../fixtures/verified/interruption4/main.lu')).to.be.true
       expect(await compareLuFiles('./../../../interruptionGen/dia1.lu', './../../fixtures/verified/interruption4/dia1.lu')).to.be.true
@@ -106,9 +110,22 @@ describe('luis:cross-train tests for lu and qna contents', () => {
       '--config', `${path.join(__dirname, './../../fixtures/testcases/interruption5/mapping_rules.json')}`,
       '--out', './interruptionGen'])
     .it('luis:cross training can split large DeferToLUIS QA pair into smaller ones when it has more than 1000 questions', async () => {
-      expect(await compareLuFiles('./../../../interruptionGen/main.lu', './../../fixtures/verified/interruption5/main.lu')).to.be.true
-      expect(await compareLuFiles('./../../../interruptionGen/main.qna', './../../fixtures/verified/interruption5/main.qna')).to.be.true
-      expect(await compareLuFiles('./../../../interruptionGen/dia1.lu', './../../fixtures/verified/interruption5/dia1.lu')).to.be.true
-      expect(await compareLuFiles('./../../../interruptionGen/dia1.qna', './../../fixtures/verified/interruption5/dia1.qna')).to.be.true
+      expect(await compareLuFiles('./../../../interruptionGen/main(1).lu', './../../fixtures/verified/interruption5/main.lu')).to.be.true
+      expect(await compareLuFiles('./../../../interruptionGen/main(1).qna', './../../fixtures/verified/interruption5/main.qna')).to.be.true
+      expect(await compareLuFiles('./../../../interruptionGen/dia1(1).lu', './../../fixtures/verified/interruption5/dia1.lu')).to.be.true
+      expect(await compareLuFiles('./../../../interruptionGen/dia1(1).qna', './../../fixtures/verified/interruption5/dia1.qna')).to.be.true
+    })
+
+  test
+    .stdout()
+    .stderr()
+    .command(['luis:cross-train',
+    '--in', `${path.join(__dirname, './../../fixtures/testcases/interruption6')}`,
+    '--intentName', '_Interruption',
+    '--config', `${path.join(__dirname, './../../fixtures/testcases/interruption6/mapping_rules.json')}`,
+    '--out', './interruptionGen',
+    '--log'])
+    .it('displays a warning if log is set true', ctx => {
+      expect(ctx.stdout).to.contain('[WARN] line 1:0 - line 1:15: no utterances found for intent definition: "# hotelLocation"')
     })
 })

packages/luis/test/fixtures/testcases/interruption6/dia1/dia1.lu

@@ -0,0 +1 @@
+# hotelLocation

packages/luis/test/fixtures/testcases/interruption6/main/main.lu

@@ -0,0 +1,2 @@
+# dia1_trigger
+- book a hotel for me

packages/luis/test/fixtures/testcases/interruption6/mapping_rules.json

@@ -0,0 +1,8 @@
+{
+    "./main/Main.lu": {
+        "rootDialog": true,
+        "triggers": {
+            "dia1_trigger": "./Dia1/dia1.lu"
+        }
+    }
+}

packages/qnamaker/README.md

@@ -149,6 +149,10 @@ OPTIONS
                                          Defaults to current logged in user alias
 
   --endpoint=endpoint                    Qnamaker authoring endpoint for publishing
+
+  -f, --force                            [default: false] If --out flag is provided with the path to an existing file, overwrites that file
+
+  --log                                  [default: false] Write out log messages to console  
 
   --schema=schema                        Defines $schema for generated .dialog files