fix luis-cross train does not find imported files with locale (#1209)

* fix import file with locale

* simplify test

* change folder structures

* remove only

* add cross train config

* fix spelling

* modify test

* change naming

* fix readme

* fix typo

Co-authored-by: Emilio Munoz <emmunozp@microsoft.com>

Author: Shuai Wang

packages/cli/README.md

@@ -891,6 +891,8 @@ OPTIONS
   --[no-]intra-dialog      Only do intra dialog cross train
 
   --log                    Writes out log messages to console
+
+  --exclude                Excludes given folders under the input directory, for example, --exclude bin,obj,lib, this will ignore the /bin, /obj, /lib folders under the input path
 ```
 
 _See code: [@microsoft/bf-luis-cli](https://github.com/microsoft/botframework-cli/tree/master/packages/luis/src/commands/luis/cross-train.ts)_

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

@@ -4,7 +4,8 @@
  */
 
 const path = require('path')
-const file = require('../../utils/filehelper')
+const fs = require('fs-extra')
+const filehelper = require('../../utils/filehelper')
 const fileExtEnum = require('../utils/helpers').FileExtTypeEnum
 const crossTrainer = require('./crossTrainer')
 
@@ -18,28 +19,63 @@ module.exports = {
    * @param {inner: boolean, intra: boolean} trainingOpt trainingOpt indicates whether you want to control do the inner or intra dialog training seperately
    * @returns {luResult: any, qnaResult: any} trainedResult of luResult and qnaResult or undefined if no results.
    */
-  train: async function (input, intentName, config, verbose, trainingOpt) {
+  train: async function (input, intentName, config, verbose, trainingOpt, exclude) {
+    // get excluded foleders
+    let excludedFolders = undefined
+    if (exclude) {
+      excludedFolders = exclude.split(',').map(e => e.trim())
+    }
+
     // Get all related file content.
-    const luContents = await file.getFilesContent(input, fileExtEnum.LUFile)
-    const qnaContents = await file.getFilesContent(input, fileExtEnum.QnAFile)
-    const configContent = await file.getConfigContent(config)
+    const luContents = await filehelper.getFilesContent(input, fileExtEnum.LUFile, excludedFolders)
+    const qnaContents = await filehelper.getFilesContent(input, fileExtEnum.QnAFile, excludedFolders)
+    const configContent = await filehelper.getConfigContent(config)
+    const defaultLocale = 'en-us'
 
-    let importResolver = async function (_, idsToFind) {
+    let importResolver = async function (id, idsToFind) {
       let importedContents = []
+      const idWithoutExt = path.basename(id, path.extname(id))
+      const locale = /\w\.\w/.test(idWithoutExt) ? idWithoutExt.split('.').pop() : defaultLocale;
       for (let idx = 0; idx < idsToFind.length; idx++) {
         let file = idsToFind[idx]
         if (path.isAbsolute(file.filePath)) {
           if (file.filePath.endsWith(fileExtEnum.LUFile)) {
-            importedContents.push(...await file.getFilesContent(file.filePath, fileExtEnum.LUFile))
+            importedContents.push(...await filehelper.getFilesContent(file.filePath, fileExtEnum.LUFile))
           } else if (file.filePath.endsWith(fileExtEnum.QnAFile)) {
-            importedContents.push(...await file.getFilesContent(file.filePath, fileExtEnum.QnAFile))
+            importedContents.push(...await filehelper.getFilesContent(file.filePath, fileExtEnum.QnAFile))
           }
         } else {
           const fileName = path.basename(file.filePath)
+          const updateImportedContents = async function(typedContents, fileExt) {
+            let found = []
+            // import resolver should be capable to find implicit import files with locale, for example '[import](b.lu)' is defined in a.en-us.lu, the resolver should find b.en-us.lu
+            const foundWithLocale = typedContents.filter(content => content.id === `${path.basename(fileName, fileExt)}.${locale}`)
+            if (foundWithLocale.length > 0) {
+              found = foundWithLocale
+            } else {
+              //if no locale specified file is found, just to check whether there is file without locale matched
+              found =  typedContents.filter(content => content.id === path.basename(fileName, fileExt))
+            }
+
+            if(found.length > 0) {
+              importedContents.push(...found)
+            } else {
+              const matchedLuisFiles = typedContents.filter(content => path.basename(content.fullPath) === id)
+              for (const matchFile of matchedLuisFiles) {
+                const sourceFileDir = path.dirname(matchFile.fullPath)
+                const targetPath = path.resolve(sourceFileDir, file.filePath)
+                if (fs.existsSync(targetPath)) {
+                  const importContent = await filehelper.getFilesContent(targetPath, fileExt)
+                  importedContents.push(...importContent)
+                }
+              }
+            }
+          }
+
           if (fileName.endsWith(fileExtEnum.LUFile)) {
-            importedContents.push(...luContents.filter(luContent => luContent.id === path.basename(fileName, fileExtEnum.LUFile)))
+            await updateImportedContents(luContents, fileExtEnum.LUFile)
           } else if (fileName.endsWith(fileExtEnum.QnAFile)) {
-            importedContents.push(...qnaContents.filter(qnaContent => qnaContent.id === path.basename(fileName, fileExtEnum.QnAFile)))
+            await updateImportedContents(qnaContents, fileExtEnum.QnAFile)
           }
         }
       }
@@ -60,4 +96,4 @@ module.exports = {
 
     return trainedResult
   }
-}
+}

packages/lu/src/utils/filehelper.ts

@@ -171,7 +171,7 @@ export async function detectLuContent(stdin: string, input: string) {
   return false
 }
 
-export async function getFilesContent(input: string, extType: string) {
+export async function getFilesContent(input: string, extType: string, ignoredFolders?: string[]) {
   let fileStat = await fs.stat(input)
   if (fileStat.isFile()) {
     const filePath = path.resolve(input)
@@ -182,7 +182,20 @@ export async function getFilesContent(input: string, extType: string) {
   if (!fileStat.isDirectory()) {
     throw (new exception(retCode.errorCode.INVALID_INPUT_FILE, 'Sorry, ' + input + ' is not a folder or does not exist'))
   }
-  const paths = await globby([`**/*${extType}`], {cwd: input, dot: true})
+
+  const allPaths = (await globby([`**/*${extType}` ], {cwd: input, dot: true}))
+  let paths: string[] = []
+  if (ignoredFolders) {
+    for (const path of allPaths) {
+      const isIgnored = ignoredFolders.filter(e => path.startsWith(e)).length > 0
+      if (!isIgnored) {
+        paths.push(path)
+      }
+    }
+  } else {
+    paths = allPaths
+  }
+  
   return Promise.all(paths.map(async (item: string) => {
     const itemPath = path.resolve(path.join(input, item))
     const content = await getContentFromFile(itemPath)

packages/luis/README.md

@@ -431,6 +431,8 @@ OPTIONS
   --[no-]intra-dialog      Only do intra dialog cross train
 
   --log                    Writes out log messages to console
+
+  --exclude                Excludes given folders under the input directory, for example, --exclude bin,obj,lib, this will ignore the /bin, /obj, /lib folders under the input path
 ```
 
 _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

@@ -22,7 +22,8 @@ export default class LuisCrossTrain extends Command {
     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: 'Writes out log messages to console', default: false}),
     'inner-dialog': flags.boolean({description: 'Only do inner dialog cross train', default: true, allowNo: true}),
-    'intra-dialog': flags.boolean({description: 'Only do intra dialog cross train', default: true, allowNo: true})
+    'intra-dialog': flags.boolean({description: 'Only do intra dialog cross train', default: true, allowNo: true}),
+    exclude: flags.string({description: 'Excludes folders under the input directory, separated by ",". If not specified, all luis and qna files will be included in the cross-train'})
   }
 
   async run() {
@@ -46,7 +47,7 @@ export default class LuisCrossTrain extends Command {
         intra: flags['intra-dialog']
       }
 
-      const trainedResult = await crossTrain.train(flags.in, flags.intentName, flags.config, flags.log, trainingOpt)
+      const trainedResult = await crossTrain.train(flags.in, flags.intentName, flags.config, flags.log, trainingOpt, flags.exclude)
 
       if (flags.out === undefined) {
         flags.out = path.join(process.cwd(), 'cross-trained')

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

@@ -148,4 +148,30 @@ describe('luis:cross-train tests for lu and qna contents', () => {
       expect(await compareLuFiles('./../../../interruptionGen/dia1.lu', './../../fixtures/verified/interruption7/dia1.lu')).to.be.true
       expect(await compareLuFiles('./../../../interruptionGen/dia1.qna', './../../fixtures/verified/interruption7/dia1.qna')).to.be.true
     })
+
+    test
+    .stdout()
+    .command(['luis:cross-train',
+      '--in', `${path.join(__dirname, './../../fixtures/testcases/application')}`,
+      '--config', `${path.join(__dirname, './../../fixtures/testcases/application/cross-train.config')}`,
+      '--out', './interruptionGen',
+      '--force'])
+    .it('luis:cross training should able to import files out of current directory', async () => {
+      expect(await compareLuFiles('./../../../interruptionGen/application.lu', './../../fixtures/verified/interruption8/application.lu')).to.be.true
+      expect(await compareLuFiles('./../../../interruptionGen/application.qna', './../../fixtures/verified/interruption8/application.qna')).to.be.true
+    })
+
+    test
+    .stdout()
+    .command(['luis:cross-train',
+      '--in', `${path.join(__dirname, './../../fixtures/testcases/testImportWithLocale')}`,
+      '--config', `${path.join(__dirname, './../../fixtures/testcases/testImportWithLocale/cross-train.config')}`,
+      '--out', './interruptionGen',
+      '--exclude', 'bin',
+      '--force'])
+    .it('luis:cross training should able to import files with locale and ignore files under the directory specified', async () => {
+      expect(await compareLuFiles('./../../../interruptionGen/ChitchatDialog.en-us.lu', './../../fixtures/verified/interruption9/ChitchatDialog.en-us.lu')).to.be.true
+      expect(await compareLuFiles('./../../../interruptionGen/ChitchatDialog.en-us.qna', './../../fixtures/verified/interruption9/ChitchatDialog.en-us.qna')).to.be.true
+      expect(fs.existsSync('./../../../interruptionGen/extra.en-us.lu')).to.be.false
+    })
 })

packages/luis/test/fixtures/testcases/testImportWithLocale/ChitchatDialog.en-us.lu

@@ -0,0 +1 @@
+[import](chitchat_professional.source.qna)

packages/luis/test/fixtures/testcases/testImportWithLocale/ChitchatDialog.en-us.qna

@@ -0,0 +1,43 @@
+# BotTour
+- bot tour
+- onboarding
+- get started
+- start the tour
+- what can you do
+- what can i ask you
+
+# Cancel
+- cancel
+- quit
+- abort
+- exit
+- never mind
+- forget about it
+- just stop already
+- end this now
+
+# Help
+- help
+- im stuck
+- how do you work
+- what can you do
+- what can you help me with
+- i need help
+- i need some assistance
+
+# Feedback
+- i want to give feedback
+- i have a problem
+- something is  broken
+- this is a bad experience
+- fill out survey
+
+# None
+- where is my car?
+- I want to order a pizza
+- place an item on hold
+- Is it going to rain?
+- turn on the lights
+- check my account balance
+- do unicorns really exist
+- duck