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 <chrimc> Co-authored-by: Emilio Munoz <[email protected]> Co-authored-by: Fei Chen <[email protected]> Co-authored-by: Chris McConnell <[email protected]>
microsoft · Feb 20, 2021 · 05d0b13 · 05d0b13
1 parent 8fcf25e
commit 05d0b13
Show file tree

Hide file tree

Showing 13 changed files with 225 additions and 20 deletions.
diff --git a/common/config/rush/pnpm-lock.yaml b/common/config/rush/pnpm-lock.yaml
diff --git 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
@@ -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
@@ -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
@@ -0,0 +1,8 @@
+{
+    "$schema": "https://schemas.botframework.com/schemas/component/v1.0/component.schema",
+    "$policies": {
+        "requiresKind": [
+            "missingKind"
+        ]
+    }
+}
diff --git a/packages/dialog/test/commands/dialog/schemas/policyExpandKind.schema 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"]
+    }
+}
diff --git a/packages/dialog/test/commands/dialog/schemas/policyLevel1.schema 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"
+        }
+    }
+}
diff --git a/packages/dialog/test/commands/dialog/schemas/policyLevel2.schema 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"
+        }
+    }
+}
diff --git 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
@@ -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)) {