From 05d0b13b25036f9d5f368b8d7d8eddc982170699 Mon Sep 17 00:00:00 2001 From: Emilio Munoz Date: Fri, 19 Feb 2021 16:48:20 -0800 Subject: [PATCH] Cherry pick from main (#1127) * fix import bug (#1124) * Add support for verifying $policies. (#1126) * Add support for verifying $policies. * Remove unneed package reference. Co-authored-by: Chris McConnell Co-authored-by: Emilio Munoz Co-authored-by: Fei Chen <43032123+feich-ms@users.noreply.github.com> Co-authored-by: Chris McConnell --- common/config/rush/pnpm-lock.yaml | 23 ++-- packages/dialog/src/library/schemaMerger.ts | 39 ++++++- .../dialog/test/commands/dialog/merge.test.ts | 12 +- .../test/commands/dialog/oracles/app.schema | 106 ++++++++++++++++++ .../badSchemas/missingPolicyKind.schema | 8 ++ .../dialog/schemas/policyExpandKind.schema | 6 + .../dialog/schemas/policyLevel1.schema | 11 ++ .../dialog/schemas/policyLevel2.schema | 12 ++ .../test/commands/dialog/verify.test.ts | 4 +- packages/lu/src/parser/lu/luMerger.js | 2 + .../test/fixtures/testcases/common.en-us.lu | 3 + .../lu/test/fixtures/testcases/root.en-us.lu | 5 + .../lu/test/parser/luis/luisBuilder.test.js | 14 +++ 13 files changed, 225 insertions(+), 20 deletions(-) create mode 100644 packages/dialog/test/commands/dialog/schemas/badSchemas/missingPolicyKind.schema create mode 100644 packages/dialog/test/commands/dialog/schemas/policyExpandKind.schema create mode 100644 packages/dialog/test/commands/dialog/schemas/policyLevel1.schema create mode 100644 packages/dialog/test/commands/dialog/schemas/policyLevel2.schema create mode 100644 packages/lu/test/fixtures/testcases/common.en-us.lu create mode 100644 packages/lu/test/fixtures/testcases/root.en-us.lu diff --git a/common/config/rush/pnpm-lock.yaml b/common/config/rush/pnpm-lock.yaml index 770434b9d..dde897fbd 100644 --- a/common/config/rush/pnpm-lock.yaml +++ b/common/config/rush/pnpm-lock.yaml @@ -63,7 +63,6 @@ dependencies: readline: 1.3.0 seedrandom: 3.0.5 sinon: 7.5.0 - source-map-support: 0.5.16 testdouble: 3.13.0 ts-node: 9.0.0_typescript@4.0.3 tslib: 2.0.3 @@ -6285,7 +6284,7 @@ packages: dev: false name: '@rush-temp/bf-chatdown' resolution: - integrity: sha512-L8f96JORWtP+IOkjYy17jqqR9jzwUtto/lUl0oziKUcZahrq6r7jJK2B9oH9u6RiwrXnzTw2q6SxVy9bxeNPlw== + integrity: sha512-LwnxaJqorI7QZOxe9fA42tJiw9KC018EJ4/Nb5TJdhnH4W9jI6hsFnai/7KrPok+1l4hdb2Dp0p2Yqft0+MM+w== tarball: 'file:projects/bf-chatdown.tgz' version: 0.0.0 'file:projects/bf-cli-command.tgz': @@ -6326,7 +6325,7 @@ packages: dev: false name: '@rush-temp/bf-cli-command' resolution: - integrity: sha512-GQhCR9Vo+N0KnJZsFAkZ5aERFQfnqqvsdN2SiaoumacyZVBUSX9FWrOW6Fgy7QL7vzs1IJXGsna32iwD9feW5Q== + integrity: sha512-j/EQpgif6mijXWVVa6eTxJ1W/rvMNHJ/1JQy+CHJz/M6NkCVCvLFlR5modLisPLKCDRRT+29eygP/SMpCRrWcg== tarball: 'file:projects/bf-cli-command.tgz' version: 0.0.0 'file:projects/bf-cli-config.tgz': @@ -6355,7 +6354,7 @@ packages: dev: false name: '@rush-temp/bf-cli-config' resolution: - integrity: sha512-6kmbihcW4qu947eZ8q+why5mF7l+XZm//mJEy2ywGrWPffSuZ3UcmYWGH+IeDUnHB/LAeKshDZhmH2oZrUgBFw== + integrity: sha512-jROoKQr6sygYp+6RJDEF5Yo+W4e4mJtjih6nMCDl+jllo13ZLkvXgrrxTLmJiVp1BD2r2La0Ydgcu0LKwH0u5w== tarball: 'file:projects/bf-cli-config.tgz' version: 0.0.0 'file:projects/bf-cli-plugins.tgz': @@ -6384,7 +6383,7 @@ packages: dev: false name: '@rush-temp/bf-cli-plugins' resolution: - integrity: sha512-27aGEoJCBxYPdlut3sUGgrrk/ppyxmtWRonVUHngWz4gX/LyZUJhfzFqrv4Fr2ZMO05PaNWMeC5KWwWU9Ickhg== + integrity: sha512-f4KIJd3jxLMwW29Gf/E7a1PdcJa1LqZTxYgcsgivlfGedKFVho+BSoJxhhJ99c7RYqiMI8vvoNzKmwuzzFzAtw== tarball: 'file:projects/bf-cli-plugins.tgz' version: 0.0.0 'file:projects/bf-dialog.tgz': @@ -6431,6 +6430,7 @@ packages: rimraf: 2.7.1 seedrandom: 3.0.5 semver: 7.3.4 + source-map-support: 0.5.19 ts-node: 9.0.0_typescript@4.0.3 tslib: 2.0.3 tslint: 5.20.1_typescript@4.0.3 @@ -6439,7 +6439,7 @@ packages: dev: false name: '@rush-temp/bf-dialog' resolution: - integrity: sha512-yrNQ0XAbeOIR2D6LC3MhfVUWK3Tt4Ja4940zppLTVX0XsBRwoS0lBVsVt5I5B9NuiP0WmA4fuES9TPixuN3XrA== + integrity: sha512-9NUxA6xR/5WktBbJ8gJYKOQ8GKWW7gWSWaXEVORxQLQvuzgQW2ys76auIHIvGUAB2VbMmK2aFgcnpcMlTwAGBg== tarball: 'file:projects/bf-dialog.tgz' version: 0.0.0 'file:projects/bf-lg-cli.tgz': @@ -6478,7 +6478,7 @@ packages: dev: false name: '@rush-temp/bf-lg-cli' resolution: - integrity: sha512-Gl8aNPJ19d1AYnUKAkb6YvLi4YL2JFRUWXqsQfJKRgfv8Wwkiq0nb5GpzrLkrp5lRSRV2qrv3JbccGsAQJnvlA== + integrity: sha512-n/fsTdjvlGsrNbOvuBZiKxWCb2OTQS/Cqt+C6rqrGKibe3hnTFF24RjJhuU1CBkPw3CDHJkfA9pAPnwoXNM4Jg== tarball: 'file:projects/bf-lg-cli.tgz' version: 0.0.0 'file:projects/bf-lu.tgz': @@ -6520,7 +6520,7 @@ packages: dev: false name: '@rush-temp/bf-lu' resolution: - integrity: sha512-PcbcFj891AAZmRLglAIb6Y2i0MUEqS6rKXP6xyRFoNy6Kmf0uehGTWxbzBsNHDdWguIOHfomeFnxdnP1TvodlA== + integrity: sha512-idgtiuhAkB8CwVNp0iQcGkBUCIbsmQab6zQujW1T1AOUwPZjE1Aml1y9NUizV4G5batuOhygJvtZHcYB4UZrKQ== tarball: 'file:projects/bf-lu.tgz' version: 0.0.0 'file:projects/bf-luis-cli.tgz': @@ -6564,7 +6564,7 @@ packages: dev: false name: '@rush-temp/bf-luis-cli' resolution: - integrity: sha512-zWWdczvEg2WbVpEnEcFQJpGaQlgONml7bhtitHm28qPdC1cL7pQFFdwc6FScScOa8GIawpsQbK0iOxWRo9FW9w== + integrity: sha512-fg5B1E3kaqUNQDT45ZWxBSAHE/gNVmIAbz2VCKEcK1/xK7DYLvcGlX5bFMC4AaYw7vSnJZ4Q5S3JYugPG6TinQ== tarball: 'file:projects/bf-luis-cli.tgz' version: 0.0.0 'file:projects/bf-qnamaker.tgz': @@ -6615,7 +6615,7 @@ packages: dev: false name: '@rush-temp/bf-qnamaker' resolution: - integrity: sha512-DY3Y3SjuGgpNJ1YS+zBaaNCLDKh0rD0k7y9A2OvrKSidAUR1850RvNh/eCZbK8k7my7UKyeP4Nb7MIElb1ay9w== + integrity: sha512-XZ3HKrN0Wmk+5WLyi9NfLxhOMPTkQN5R0H+Js+0vM2jaq00NmpGinUalQTDvYrJrgaTIsentVSedsEUHyUU/JQ== tarball: 'file:projects/bf-qnamaker.tgz' version: 0.0.0 'file:projects/botframework-cli.tgz': @@ -6651,7 +6651,7 @@ packages: dev: false name: '@rush-temp/botframework-cli' resolution: - integrity: sha512-MA88uqV++fXNqhjAgX3pIyXzGS5e75NsIty7TY56R0tRKZQy9ADqnPlIrjiod0DActvsl2t/phyU6aehODXttQ== + integrity: sha512-9/0dyLrvSjgWsNXuBSOXqwjTbMLJAZYYvD7wX2OQWvadN/vq5ghqSNXuB78IZiFaClFOdiuueRHQqPDVNL8VVQ== tarball: 'file:projects/botframework-cli.tgz' version: 0.0.0 registry: '' @@ -6720,7 +6720,6 @@ specifiers: readline: ^1.3.0 seedrandom: ~3.0.5 sinon: ^7.5.0 - source-map-support: ~0.5.16 testdouble: ^3.11.0 ts-node: ^9.0.0 tslib: ^2.0.3 diff --git a/packages/dialog/src/library/schemaMerger.ts b/packages/dialog/src/library/schemaMerger.ts index fd194a90a..e6e3556da 100644 --- a/packages/dialog/src/library/schemaMerger.ts +++ b/packages/dialog/src/library/schemaMerger.ts @@ -623,6 +623,7 @@ export class SchemaMerger { this.expandInterfaces() this.addComponentProperties() this.sortImplementations() + this.validateAndExpandPolicies() const oneOf = Object.keys(this.definitions) .filter(kind => !this.isInterface(kind) && this.definitions[kind].$role) .sort() @@ -889,6 +890,36 @@ export class SchemaMerger { return imports } + // Return all recursive extensions of a kind including the original kind + private allExtensions(kind: string): string[] { + let exts: string[] = [kind] + for (const [dkind, definition] of Object.entries(this.definitions)) { + for (const extension of this.roles(definition, 'extends')) { + if (extension === kind) { + exts = [...exts, ...this.allExtensions(dkind)] + } + } + } + return exts + } + + // Ensure kinds in policies exist and we expand to any extensions + private validateAndExpandPolicies(): void { + walkJSON(this.definitions, (val, _obj, path) => { + if (val.$policies?.requiresKind) { + let expanded: string[] = [] + for(const kind of val.$policies.requiresKind) { + if (!this.definitions[kind]) { + this.genericError(`Error ${path} has non-existent $kind ${kind}`) + } + expanded = [...expanded, ...this.allExtensions(kind)] + } + val.$policies.requiresKind = expanded + } + return false + }) + } + // Given schema properties object and ui schema properties object, check to ensure // each ui schema property exists in schema private validateProperties(path: string, schema: any, uiProps: object): void { @@ -1777,7 +1808,7 @@ export class SchemaMerger { const ref = `#/definitions/${name}${pointer}` const definition: any = ptr.get(schema, ref) if (!definition) { - this.refError(elt.$ref, ref) + this.genericError(`Error could not bundle ${elt.$ref} into ${ref}`) } else if (!elt.$bundled) { elt.$ref = ref elt.$bundled = true @@ -2054,9 +2085,9 @@ export class SchemaMerger { this.failed = true } - // Missing $ref - private refError(original: string, modified: string): void { - this.error(`Error could not bundle ${original} into ${modified}`) + // Generic error message + private genericError(msg: string) { + this.error(msg) this.failed = true } } diff --git a/packages/dialog/test/commands/dialog/merge.test.ts b/packages/dialog/test/commands/dialog/merge.test.ts index c37be94bc..1d06c6a17 100644 --- a/packages/dialog/test/commands/dialog/merge.test.ts +++ b/packages/dialog/test/commands/dialog/merge.test.ts @@ -5,7 +5,7 @@ // tslint:disable:no-console // tslint:disable:no-object-literal-type-assertion -import { assert } from 'chai' +import {assert} from 'chai' import * as fs from 'fs-extra' import 'mocha' import * as os from 'os' @@ -128,7 +128,7 @@ function checkMerged(merged: merger.Imports | undefined, adds: number, conflicts describe('dialog:merge', async () => { beforeEach(async () => { - // If you want to regenerate the oracle *.schema files, run schemas/makeschemas.cmd + // If you want to regenerate the oracle *.schema files, run makeOracles.cmd await fs.remove(tempDir) await fs.mkdirp(tempDir) process.chdir(srcDir) @@ -207,6 +207,14 @@ describe('dialog:merge', async () => { assert(countMatches('no implementations', lines) === 1, 'Did not detect missing implementations') }) + it('missing policy kind', async () => { + console.log('\nStart missing policy kind') + let [merged, lines] = await merge(['schemas/*.schema', 'schemas/badSchemas/missingPolicyKind.schema']) + assert(!merged, 'Merging should have failed') + assert(countMatches(/error|warning/i, lines) === 1, 'Wrong number of errors or warnings') + assert(countMatches('non-existent', lines) === 1, 'Did not detect missing $kind') + }) + it('csproj', async () => { console.log('\nStart csproj') let [merged, lines] = await merge(['projects/project3/project3.csproj'], 'project3.schema', true) diff --git a/packages/dialog/test/commands/dialog/oracles/app.schema b/packages/dialog/test/commands/dialog/oracles/app.schema index 6c1372276..96d665b03 100644 --- a/packages/dialog/test/commands/dialog/oracles/app.schema +++ b/packages/dialog/test/commands/dialog/oracles/app.schema @@ -7,6 +7,9 @@ { "$ref": "#/definitions/Recognizer" }, + { + "$ref": "#/definitions/policyLevel2" + }, { "$ref": "#/definitions/prompt" }, @@ -288,6 +291,109 @@ } ] }, + "policyExpandKind": { + "$policies": { + "requiresKind": [ + "policyLevel1", + "policyLevel2" + ] + }, + "type": "object", + "required": [ + "$kind" + ], + "additionalProperties": false, + "patternProperties": { + "^\\$": { + "title": "Tooling property", + "description": "Open ended property for tooling." + } + }, + "properties": { + "$kind": { + "title": "Kind of dialog object", + "description": "Defines the valid properties for the component you are configuring (from a dialog .schema file)", + "type": "string", + "pattern": "^[a-zA-Z][a-zA-Z0-9.]*$", + "const": "policyExpandKind" + }, + "$designer": { + "title": "Designer information", + "type": "object", + "description": "Extra information for the Bot Framework Composer." + } + } + }, + "policyLevel1": { + "type": "object", + "required": [ + "$kind" + ], + "additionalProperties": false, + "patternProperties": { + "^\\$": { + "title": "Tooling property", + "description": "Open ended property for tooling." + } + }, + "properties": { + "level1": { + "title": "level1", + "description": "level1", + "type": "string" + }, + "$kind": { + "title": "Kind of dialog object", + "description": "Defines the valid properties for the component you are configuring (from a dialog .schema file)", + "type": "string", + "pattern": "^[a-zA-Z][a-zA-Z0-9.]*$", + "const": "policyLevel1" + }, + "$designer": { + "title": "Designer information", + "type": "object", + "description": "Extra information for the Bot Framework Composer." + } + } + }, + "policyLevel2": { + "$role": "extends(policyLevel1)", + "type": "object", + "required": [ + "$kind" + ], + "additionalProperties": false, + "patternProperties": { + "^\\$": { + "title": "Tooling property", + "description": "Open ended property for tooling." + } + }, + "properties": { + "level2": { + "title": "level2", + "description": "level2", + "type": "string" + }, + "level1": { + "title": "level1", + "description": "level1", + "type": "string" + }, + "$kind": { + "title": "Kind of dialog object", + "description": "Defines the valid properties for the component you are configuring (from a dialog .schema file)", + "type": "string", + "pattern": "^[a-zA-Z][a-zA-Z0-9.]*$", + "const": "policyLevel2" + }, + "$designer": { + "title": "Designer information", + "type": "object", + "description": "Extra information for the Bot Framework Composer." + } + } + }, "prompt": { "$role": "extends(basePrompt)", "required": [ diff --git a/packages/dialog/test/commands/dialog/schemas/badSchemas/missingPolicyKind.schema b/packages/dialog/test/commands/dialog/schemas/badSchemas/missingPolicyKind.schema new file mode 100644 index 000000000..bb868ac74 --- /dev/null +++ b/packages/dialog/test/commands/dialog/schemas/badSchemas/missingPolicyKind.schema @@ -0,0 +1,8 @@ +{ + "$schema": "https://schemas.botframework.com/schemas/component/v1.0/component.schema", + "$policies": { + "requiresKind": [ + "missingKind" + ] + } +} \ No newline at end of file diff --git a/packages/dialog/test/commands/dialog/schemas/policyExpandKind.schema b/packages/dialog/test/commands/dialog/schemas/policyExpandKind.schema new file mode 100644 index 000000000..9befc54d4 --- /dev/null +++ b/packages/dialog/test/commands/dialog/schemas/policyExpandKind.schema @@ -0,0 +1,6 @@ +{ + "$schema": "https://schemas.botframework.com/schemas/component/v1.0/component.schema", + "$policies": { + "requiresKind": ["policyLevel1"] + } +} \ No newline at end of file diff --git a/packages/dialog/test/commands/dialog/schemas/policyLevel1.schema b/packages/dialog/test/commands/dialog/schemas/policyLevel1.schema new file mode 100644 index 000000000..ddc6d0a7e --- /dev/null +++ b/packages/dialog/test/commands/dialog/schemas/policyLevel1.schema @@ -0,0 +1,11 @@ +{ + "$schema": "https://schemas.botframework.com/schemas/component/v1.0/component.schema", + "type": "object", + "properties": { + "level1": { + "title": "level1", + "description": "level1", + "type": "string" + } + } +} \ No newline at end of file diff --git a/packages/dialog/test/commands/dialog/schemas/policyLevel2.schema b/packages/dialog/test/commands/dialog/schemas/policyLevel2.schema new file mode 100644 index 000000000..6ec044a9d --- /dev/null +++ b/packages/dialog/test/commands/dialog/schemas/policyLevel2.schema @@ -0,0 +1,12 @@ +{ + "$schema": "https://schemas.botframework.com/schemas/component/v1.0/component.schema", + "$role": "extends(policyLevel1)", + "type": "object", + "properties": { + "level2": { + "title": "level2", + "description": "level2", + "type": "string" + } + } +} \ No newline at end of file diff --git a/packages/dialog/test/commands/dialog/verify.test.ts b/packages/dialog/test/commands/dialog/verify.test.ts index 14b192dff..52692b72f 100644 --- a/packages/dialog/test/commands/dialog/verify.test.ts +++ b/packages/dialog/test/commands/dialog/verify.test.ts @@ -32,7 +32,7 @@ describe('dialog:verify', () => { expect(ctx.stderr.match(/DLG005/)!.length == 6, 'Wrong number of unsued ids') expect(ctx.stdout).to.contain('4 files') expect(ctx.stderr) - .to.contain('Warnings: 2') - .to.contain('Errors: 12') + .to.contain('Warnings: 2', 'Wrong number of warnings') + .to.contain('Errors: 13', 'Wrong number of errors') }) }) diff --git a/packages/lu/src/parser/lu/luMerger.js b/packages/lu/src/parser/lu/luMerger.js index 8b156ddb3..20bd244c0 100644 --- a/packages/lu/src/parser/lu/luMerger.js +++ b/packages/lu/src/parser/lu/luMerger.js @@ -392,6 +392,8 @@ const buildLuJsonObject = async function(luObjArray, log, luis_culture, luSearch continue } + // Set includeInCollate in addtionalFilesToParse to be false if current parsedContent's includeInCollate is false + parsedContent.additionalFilesToParse.forEach(addtionFileToParse => addtionFileToParse.includeInCollate = luOb.includeInCollate ? addtionFileToParse.includeInCollate : luOb.includeInCollate) let foundLuFiles = await luSearchFn(luOb.id, parsedContent.additionalFilesToParse) for( let i = 0; i < foundLuFiles.length; i++){ if (parsedFiles.includes(foundLuFiles[i].id)) { diff --git a/packages/lu/test/fixtures/testcases/common.en-us.lu b/packages/lu/test/fixtures/testcases/common.en-us.lu new file mode 100644 index 000000000..e1e7ffbde --- /dev/null +++ b/packages/lu/test/fixtures/testcases/common.en-us.lu @@ -0,0 +1,3 @@ +# greeting +- hi +- hello \ No newline at end of file diff --git a/packages/lu/test/fixtures/testcases/root.en-us.lu b/packages/lu/test/fixtures/testcases/root.en-us.lu new file mode 100644 index 000000000..af5b8bc31 --- /dev/null +++ b/packages/lu/test/fixtures/testcases/root.en-us.lu @@ -0,0 +1,5 @@ +[common](./common.en-us.lu) + +# MyIntent +- test 1 +- test 2 \ No newline at end of file diff --git a/packages/lu/test/parser/luis/luisBuilder.test.js b/packages/lu/test/parser/luis/luisBuilder.test.js index 11331f2bb..ad81b86c7 100644 --- a/packages/lu/test/parser/luis/luisBuilder.test.js +++ b/packages/lu/test/parser/luis/luisBuilder.test.js @@ -96,4 +96,18 @@ assert.isTrue(luisObject.validate()) assert.equal(luisObject.settings[1].name, 'UseAllTrainingData'); assert.equal(luisObject.settings[1].value, true); }); + + it('Intent import can work correctly when imported lu files also have further imports', async () => { + let luFile = ` + # Test + - [MyIntent](./test/fixtures/testcases/root.en-us.lu#MyIntent)`; + + const luisObject = await LUISBuilder.fromLUAsync(luFile) + + assert.equal(luisObject.intents.length, 1); + assert.equal(luisObject.intents[0].name, 'Test'); + assert.equal(luisObject.utterances.length, 2); + assert.equal(luisObject.utterances[0].text, 'test 1'); + assert.equal(luisObject.utterances[1].text, 'test 2'); + }); }); \ No newline at end of file