Remove $id from schema:merge and enable authoringKey in luis:build (#744)

* Remove $id from schema:merge.
Enable authoringKey config to work for luis:build.

* Fix case issue in test.

Author: Chris McConnell

.vscode/launch.json

@@ -4,6 +4,24 @@
     // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
     "version": "0.2.0",
     "configurations": [
+        {
+            "type": "node",
+            "request": "launch",
+            "name": "bf CLI",
+            "preLaunchTask": "${defaultBuildTask}",
+            "program": "${workspaceFolder}/packages/cli/bin/run",
+            "outputCapture": "std",
+            "outFiles": [
+                "${workspaceFolder}/packages/luis/lib/**"
+            ],
+            "args": [
+                "luis:build",
+                "--luConfig", 
+                "${env:TEMP}/sandwich.out/luconfig.json"
+            ],
+            "internalConsoleOptions": "openOnSessionStart",
+            "cwd": "${workspaceFolder}"
+        },
         {
             "type": "node",
             "request": "launch",
@@ -198,9 +216,7 @@
                 "--in",
                 ".",
                 "--luConfig",
-                "luconfig.json",
-                "--authoringKey",
-                "${env:LUIS_AUTHORING_KEY}"
+                "luconfig.json"
             ],
             "internalConsoleOptions": "openOnSessionStart",
             "cwd": "${env:TEMP}/sandwich.out"

common/config/rush/pnpm-lock.yaml

@@ -5,6 +5,7 @@
     "module": "commonjs",
     "outDir": "lib",
     "rootDir": "src",
+    "sourceMap": true,
     "strict": true,
     "target": "es2017",
     "composite": true

packages/cli/tsconfig.json

@@ -72,7 +72,7 @@ export default class SchemaMerger {
     private readonly missingKinds = new Set()
     private currentFile = ''
     private currentKind = ''
-    private readonly jsonOptions = { spaces: '\t', EOL: os.EOL }
+    private readonly jsonOptions = {spaces: '\t', EOL: os.EOL}
 
     /**
      * Merger to combine copmonent .schema files to make a custom schema.
@@ -128,8 +128,8 @@ export default class SchemaMerger {
                         this.log(`Parsing ${componentPath}`)
                     }
                     let component = await fs.readJSON(componentPath)
-                    if (component.$id) {
-                        this.parsingWarning(`Skipping because of top-level $id ${component.$id}`)
+                    if (component.definitions && component.definitions.component) {
+                        this.parsingWarning('Skipping merged schema')
                     } else {
                         this.relativeToAbsoluteRefs(component, componentPath)
 
@@ -181,7 +181,7 @@ export default class SchemaMerger {
                 .filter(kind => !this.isInterface(kind) && this.definitions[kind].$role)
                 .sort()
                 .map(kind => {
-                    return { $ref: `#/definitions/${kind}` }
+                    return {$ref: `#/definitions/${kind}`}
                 })
             this.addSchemaDefinitions()
 
@@ -194,7 +194,6 @@ export default class SchemaMerger {
                 }
                 let finalSchema: any = {
                     $schema: this.metaSchemaId,
-                    $id: `file:///${ppath.resolve(this.output).replace(/\\/g, '/')}#`,
                     type: 'object',
                     title: 'Component kinds',
                     description: 'These are all of the kinds that can be created by the loader.',
@@ -423,7 +422,7 @@ export default class SchemaMerger {
     async findGlobalNuget(): Promise<string> {
         let result = ''
         try {
-            const { stdout } = await exec('dotnet nuget locals global-packages --list')
+            const {stdout} = await exec('dotnet nuget locals global-packages --list')
             const name = 'global-packages:'
             let start = stdout.indexOf(name)
             if (start > -1) {
@@ -524,7 +523,7 @@ export default class SchemaMerger {
 
             if (extension.patternProperties) {
                 if (definition.patternProperties) {
-                    definition.patternPropties = { ...definition.patternProperties, ...extension.patternProperties }
+                    definition.patternPropties = {...definition.patternProperties, ...extension.patternProperties}
                 } else {
                     definition.patternProperties = clone(extension.patternProperties)
                 }
@@ -666,7 +665,7 @@ export default class SchemaMerger {
     expandKinds(): void {
         for (this.currentKind in this.definitions) {
             walkJSON(this.definitions[this.currentKind], val => {
-                if (val.$kind) {
+                if (val.$kind && typeof val.$kind === 'string') {
                     if (this.definitions.hasOwnProperty(val.$kind)) {
                         val.$ref = '#/definitions/' + val.$kind
                     } else {
@@ -727,7 +726,7 @@ export default class SchemaMerger {
     // Add schema definitions and turn schema: or full definition URI into local reference
     addSchemaDefinitions(): void {
         const scheme = 'schema:'
-        this.definitions = { ...this.metaSchema.definitions, ...this.definitions }
+        this.definitions = {...this.metaSchema.definitions, ...this.definitions}
         for (this.currentKind in this.definitions) {
             walkJSON(this.definitions[this.currentKind], val => {
                 if (typeof val === 'object' && val.$ref && (val.$ref.startsWith(scheme) || val.$ref.startsWith(this.metaSchemaId))) {
@@ -761,8 +760,8 @@ export default class SchemaMerger {
 
     // Remove any child $id because their references have been changed to be local
     removeId(bundle: any) {
-        walkJSON(bundle, (val, _obj, path) => {
-            if (path && val.$id) {
+        walkJSON(bundle, val => {
+            if (val.$id) {
                 delete val.$id
             }
             return false

packages/dialog/src/library/schemaMerger.ts

@@ -69,8 +69,6 @@ describe('dialog:merge', async () => {
         assert(countMatches(/error|warning/i, lines) == 1, 'Error merging schemas')
         let oracle = await fs.readJSON('schemas/app.schema')
         let generated = await fs.readJSON('generated.schema')
-        delete oracle.$id
-        delete generated.$id
         let oracles = JSON.stringify(oracle)
         let generateds = JSON.stringify(generated)
         if (oracles !== generateds) {

packages/dialog/test/commands/dialog/merge.test.ts

@@ -1,6 +1,5 @@
 {
 	"$schema": "https://raw.githubusercontent.com/microsoft/botframework-sdk/master/schemas/component/component.schema",
-	"$id": "file:///C:/Users/chrim/source/repos/botframework-cli/packages/dialog/test/commands/dialog/schemas/app.schema#",
 	"type": "object",
 	"title": "Component kinds",
 	"description": "These are all of the kinds that can be created by the loader.",

packages/dialog/test/commands/dialog/schemas/app.schema

@@ -64,10 +64,12 @@
     "@types/lodash": "~4.14.146",
     "@types/mocha": "^5.2.7",
     "@types/node": "^10.17.4",
+    "@types/nock": "^11.1.0",
     "@types/rimraf": "^2.0.3",
     "chai": "^4.2.0",
     "globby": "^10.0.1",
     "mocha": "^6.2.2",
+    "nock": "^11.7.0",
     "nyc": "^14.1.1",
     "rimraf": "^3.0.0",
     "sinon": "^7.5.0",
@@ -76,4 +78,4 @@
     "typescript": "^3.5.3",
     "uuid": "^3.3.3"
   }
-}
+}

packages/luis/package.json

@@ -46,10 +46,8 @@ export default class LuisBuild extends Command {
   async run() {
     try {
       const {flags} = this.parse(LuisBuild)
-      const userConfig = await utils.getUserConfig(this.config.configDir)
-
-      flags.stdin = await this.readStdin()
 
+      // Luconfig overrides flags
       let files: string[] = []
       if (flags.luConfig) {
         const configFilePath = path.resolve(flags.luConfig)
@@ -67,32 +65,36 @@ export default class LuisBuild extends Command {
         }
       }
 
-      if (!flags.stdin && !flags.in && files.length === 0) {
+      // Flags override userConfig
+      let {inVal, authoringKey, botName, region, out, defaultCulture, fallbackLocale, suffix, dialog, force, luConfig, deleteOldVersion, log}
+        = await utils.processInputs(flags, Object.keys(LuisBuild.flags), this.config.configDir)
+
+      flags.stdin = await this.readStdin()
+
+      if (!flags.stdin && !inVal && files.length === 0) {
         throw new CLIError('Missing input. Please use stdin or pass a file or folder location with --in flag')
       }
 
-      if (!flags.authoringKey && (!userConfig || !userConfig.authoringKey)) {
-        throw new CLIError('Missing luis authoring key. Please pass authoring key with --authoringKey flag')
-      } else {
-        flags.authoringKey = flags.authoringKey || userConfig.authoringKey
+      if (!authoringKey) {
+        throw new CLIError('Missing LUIS authoring key. Please pass authoring key with --authoringKey flag or specify via bf config:set:luis.')
       }
 
-      if (!flags.botName) {
-        throw new CLIError('Missing bot name. Please pass bot name with --botName flag')
+      if (!botName) {
+        throw new CLIError('Missing bot name. Please pass bot name with --botName flag.')
       }
 
-      if (flags.dialog && flags.dialog !== recognizerType.MULTILANGUAGE && flags.dialog !== recognizerType.CROSSTRAINED) {
-        throw new CLIError('Recognizer type specified by --dialog is not right. Please specify [multiLanguage|crosstrained]')
+      if (dialog && dialog !== recognizerType.MULTILANGUAGE && dialog !== recognizerType.CROSSTRAINED) {
+        throw new CLIError('Recognizer type specified by --dialog is not right. Please specify [multiLanguage|crosstrained].')
       }
 
-      flags.defaultCulture = flags.defaultCulture && flags.defaultCulture !== '' ? flags.defaultCulture : 'en-us'
-      flags.region = flags.region && flags.region !== '' ? flags.region : 'westus'
-      flags.suffix = flags.suffix && flags.suffix !== '' ? flags.suffix : await username() || 'development'
-      flags.fallbackLocale = flags.fallbackLocale && flags.fallbackLocale !== '' ? flags.fallbackLocale : 'en-us'
+      defaultCulture = defaultCulture && defaultCulture !== '' ? defaultCulture : 'en-us'
+      region = region && region !== '' ? region : 'westus'
+      suffix = suffix && suffix !== '' ? suffix : await username() || 'development'
+      fallbackLocale = fallbackLocale && fallbackLocale !== '' ? fallbackLocale : 'en-us'
 
       // create builder class
       const builder = new Builder((input: string) => {
-        if (flags.log) this.log(input)
+        if (log) this.log(input)
       })
 
       let luContents: any[] = []
@@ -102,19 +104,19 @@ export default class LuisBuild extends Command {
 
       if (flags.stdin && flags.stdin !== '') {
         // load lu content from stdin and create default recognizer, multiRecognier and settings
-        if (flags.log) this.log('Load lu content from stdin\n')
-        const content = new Content(flags.stdin, new LUOptions('stdin', true, flags.defaultCulture, path.join(process.cwd(), 'stdin')))
+        if (log) this.log('Load lu content from stdin\n')
+        const content = new Content(flags.stdin, new LUOptions('stdin', true, defaultCulture, path.join(process.cwd(), 'stdin')))
         luContents.push(content)
         multiRecognizers.set('stdin', new MultiLanguageRecognizer(path.join(process.cwd(), 'stdin.lu.dialog'), {}))
-        settings.set('stdin', new Settings(path.join(process.cwd(), `luis.settings.${flags.suffix}.${flags.region}.json`), {}))
+        settings.set('stdin', new Settings(path.join(process.cwd(), `luis.settings.${suffix}.${region}.json`), {}))
         const recognizer = Recognizer.load(content.path, content.name, path.join(process.cwd(), `${content.name}.dialog`), settings.get('stdin'), {})
         recognizers.set(content.name, recognizer)
       } else {
-        if (flags.log) this.log('Loading files...\n')
+        if (log) this.log('Loading files...\n')
 
-        // get lu files from flags.in.
-        if (flags.in && flags.in !== '') {
-          const luFiles = await file.getLuFiles(flags.in, true, fileExtEnum.LUFile)
+        // get lu files from in.
+        if (inVal && inVal !== '') {
+          const luFiles = await file.getLuFiles(inVal, true, fileExtEnum.LUFile)
           files.push(...luFiles)
         }
 
@@ -123,22 +125,22 @@ export default class LuisBuild extends Command {
 
         // load lu contents from lu files
         // load existing recognizers, multiRecogniers and settings or create default ones
-        const loadedResources = await builder.loadContents(files, flags.defaultCulture, flags.suffix, flags.region)
+        const loadedResources = await builder.loadContents(files, defaultCulture, suffix, region)
         luContents = loadedResources.luContents
         recognizers = loadedResources.recognizers
         multiRecognizers = loadedResources.multiRecognizers
         settings = loadedResources.settings
       }
 
       // update or create and then train and publish luis applications based on loaded resources
-      if (flags.log) this.log('Handling applications...')
-      const dialogContents = await builder.build(luContents, recognizers, flags.authoringKey, flags.region, flags.botName, flags.suffix, flags.fallbackLocale, flags.deleteOldVersion, multiRecognizers, settings)
+      if (log) this.log('Handling applications...')
+      const dialogContents = await builder.build(luContents, recognizers, authoringKey, region, botName, flags.suffix, fallbackLocale, deleteOldVersion, multiRecognizers, settings)
 
       // write dialog assets based on config
-      if (flags.dialog) {
-        const writeDone = await builder.writeDialogAssets(dialogContents, flags.force, flags.out, flags.dialog, flags.luConfig)
-        const dialogFilePath = (flags.stdin || !flags.in) ? process.cwd() : flags.in.endsWith(fileExtEnum.LUFile) ? path.dirname(path.resolve(flags.in)) : path.resolve(flags.in)
-        const outputFolder = flags.out ? path.resolve(flags.out) : dialogFilePath
+      if (dialog) {
+        const writeDone = await builder.writeDialogAssets(dialogContents, force, out, dialog, luConfig)
+        const dialogFilePath = (flags.stdin || !inVal) ? process.cwd() : inVal.endsWith(fileExtEnum.LUFile) ? path.dirname(path.resolve(inVal)) : path.resolve(inVal)
+        const outputFolder = out ? path.resolve(out) : dialogFilePath
         if (writeDone) {
           this.log(`Successfully wrote .dialog files to ${outputFolder}\n`)
         } else {

packages/luis/src/commands/luis/build.ts

@@ -70,7 +70,7 @@ const isDirectory = (path: string): boolean => {
 }
 
 const filterByAllowedConfigValues = (configObj: any, prefix: string) => {
-  const allowedConfigValues = [`${prefix}appId`, `${prefix}endpoint`, `${prefix}region`, `${prefix}subscriptionKey`, `${prefix}versionId`]
+  const allowedConfigValues = [`${prefix}appId`, `${prefix}endpoint`, `${prefix}region`, `${prefix}subscriptionKey`, `${prefix}versionId`, `${prefix}authoringKey`]
   const filtered = Object.keys(configObj)
   .filter(key => allowedConfigValues.includes(key))
   .reduce((filteredConfigObj: any, key) => {

packages/luis/src/utils/index.ts

@@ -26,7 +26,7 @@ describe('luis:build cli parameters test', () => {
     .stderr()
     .command(['luis:build', '--in', `${path.join(__dirname, './../../fixtures/testcases/lubuild')}`, '--botName', 'Contoso'])
     .it('displays an error if any required input parameters are missing', ctx => {
-      expect(ctx.stderr).to.contain('Missing luis authoring key. Please pass authoring key with --authoringKey flag')
+      expect(ctx.stderr).to.contain('Missing LUIS authoring key. Please pass authoring key with --authoringKey flag')
     })
 
   test