Dialog cherry pick of 932 and 944 (#945)

* Change dialog:merge bundling (#932)

* Working bundle.

* Figuring out $bundled for $ref

* Remove $bundled.

* Add tests for more complex schema.

* Single process schema:

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

* Sibling $ref required removed from definition. (#944)

* Sibling $ref required removed from definition.

* Update readme.

Co-authored-by: Chris McConnell <chrimc@microsoft.com>

Author: munozemilio

.vscode/launch.json

@@ -138,7 +138,8 @@
                 "libraries/**/*.schema",
                 "-o",
                 "${env:TEMP}/sdk.schema",
-                "--verbose"
+                "--verbose",
+                "--debug"
             ],
             "internalConsoleOptions": "openOnSessionStart",
             "cwd": "${workspaceFolder}/../botbuilder-dotnet"
@@ -193,12 +194,14 @@
             ],
             "args": [
                 "dialog:merge",
-                "schemas/*.schema",
+                "libraries/**/*.schema",
+                "tests/**/*.schema",
                 "-o",
-                "schemas/app.schema"
+                "tests/tests.schema",
+                "--verbose"
             ],
             "internalConsoleOptions": "openOnSessionStart",
-            "cwd": "${workspaceFolder}/packages/dialog/test/commands/dialog"
+            "cwd": "${workspaceFolder}/../botbuilder-dotnet/"
         },
         {
             "type": "node",
@@ -228,8 +231,8 @@
             "program": "${workspaceFolder}/packages/luis/bin/run",
             "outputCapture": "std",
             "outFiles": [
-                "./packages/luis/lib/**",
-                "./packages/lu/lib/**"
+                "${workspaceFolder}/packages/luis/lib/**",
+                "${workspaceFolder}/packages/lu/lib/**"
             ],
             "args": [
                 "luis:build",
@@ -239,7 +242,8 @@
                 "${env:LUIS_AUTHORING_KEY}"
             ],
             "internalConsoleOptions": "openOnSessionStart",
-            "cwd": "${env:TEMP}/sandwich.out"
+            "cwd": "${env:TEMP}/generate.out",
+            "sourceMaps": true
         },
         {
             "type": "node",

packages/dialog/src/commands/dialog/merge.ts

@@ -7,7 +7,7 @@ import {Command, flags} from '@microsoft/bf-cli-command'
 import SchemaMerger from '../../library/schemaMerger'
 
 export default class DialogMerge extends Command {
-    static description = 'Merge <kind>.schema and <kind>[.<locale>].uischema definitions from a project and its dependencies into a single .schema for describing .dialog files and a per locale .uischema for describing how Composer shows them.  For C#, ensures all nuget declarative resources are included in the same location.'
+    static description = 'Merge `<kind>.schema` and `<kind>[.<locale>].uischema` definitions from a project and its dependencies into a single .schema for describing .dialog files and a per locale .uischema for describing how Composer shows them.  For C#, ensures all nuget declarative resources are included in the same location.'
 
     static args = [
         {name: 'patterns', required: true, description: 'Any number of glob regex patterns to match .csproj, .nuspec or package.json files.'},

packages/dialog/src/library/schemaMerger.ts

@@ -441,7 +441,9 @@ export default class SchemaMerger {
             .map(kind => {
                 return {$ref: `#/definitions/${kind}`}
             })
-        this.addSchemaDefinitions()
+
+        // Add component schema definitions
+        this.definitions = {...this.metaSchema.definitions, ...this.definitions}
 
         if (!this.failed) {
             this.currentFile = this.output + '.schema'
@@ -463,7 +465,7 @@ export default class SchemaMerger {
             }
 
             // Convert all remote references to local ones
-            finalSchema = await parser.bundle(finalSchema as parser.JSONSchema, this.schemaProtocolResolver())
+            await this.bundle(finalSchema)
             finalSchema = this.expandAllOf(finalSchema)
             this.removeId(finalSchema)
             if (this.debug) {
@@ -1333,20 +1335,132 @@ export default class SchemaMerger {
         }
     }
 
-    // Add schema definitions and turn schema: or full definition URI into local reference
-    private addSchemaDefinitions(): void {
-        const scheme = 'schema:'
-        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))) {
-                    val.$ref = val.$ref.substring(val.$ref.indexOf('#'))
+    // Split a $ref into path, pointer and name for definition
+    private splitRef(ref: string): {path: string, pointer: string, name: string} {
+        const hash = ref.indexOf('#')
+        const path = hash < 0 ? '' : ref.substring(0, hash)
+        const pointer = hash < 0 ? '' : ref.substring(hash + 1)
+        let name = ppath.basename(path)
+        if (name.endsWith('#')) {
+            name = name.substring(0, name.length - 1)
+        }
+        return {path, pointer, name}
+    }
+
+    // Bundle remote references into schema while pruning to minimally needed definitions.
+    // Remote references will be found under definitions/<pathBasename> which must be unique.
+    // There is special code to handle requires siblings to $ref where we remove the requires from
+    // the bundled definition.  This is similar to what JSON schema 8 does.
+    private async bundle(schema: any): Promise<void> {
+        const current = this.currentFile
+        let sources: string[] = []
+        await this.bundleFun(schema, schema, sources, '')
+        for (let source of sources) {
+            this.prune(schema.definitions[source])
+        }
+        walkJSON(schema, elt => {
+            if (typeof elt === 'object') {
+                delete elt.$bundled
+            }
+            return false
+        })
+        this.currentFile = current
+    }
+
+    private async bundleFun(schema: any, elt: any, sources: string[], source: string): Promise<void> {
+        if (typeof elt === 'object' || Array.isArray(elt)) {
+            for (let key in elt) {
+                const val = elt[key]
+                if (key === '$ref' && typeof val === 'string') {
+                    if (val.startsWith('schema:') || val.startsWith(this.metaSchemaId)) {
+                        // Component schema reference
+                        elt.$ref = val.substring(val.indexOf('#'))
+                    } else {
+                        const {path, pointer, name} = this.splitRef(val)
+                        if (path) {
+                            if (!schema.definitions[name]) {
+                                // New source
+                                this.currentFile = path
+                                this.vlog(`Bundling ${path}`)
+                                schema.definitions[name] = await getJSON(path)
+                                sources.push(name)
+                            }
+                            let ref = `#/definitions/${name}${pointer}`
+                            let definition: any = ptr.get(schema, ref)
+                            if (!definition) {
+                                this.refError(elt.$ref, ref)
+                            } else if (!elt.$bundled) {
+                                elt.$ref = ref
+                                elt.$bundled = true
+                                if (elt.required) {
+                                    // Strip required from destination
+                                    // This is to support a required sibling to $ref
+                                    delete definition.required
+                                }
+                                if (!definition.$bundled) {
+                                    // First outside reference mark it to keep and follow internal $ref
+                                    definition.$bundled = true
+                                    let cd = ''
+                                    try {
+                                        if (path.startsWith('file:')) {
+                                            cd = process.cwd()
+                                            process.chdir(ppath.dirname(path))
+                                        }
+                                        await this.bundleFun(schema, definition, sources, name)
+                                    } finally {
+                                        if (cd) {
+                                            process.chdir(cd)
+                                        }
+                                    }
+                                }
+                            }
+                        } else if (source) {
+                            // Internal reference in external source
+                            const ref = `#/definitions/${source}${pointer}`
+                            const definition: any = ptr.get(schema, ref)
+                            if (!elt.$bundled) {
+                                elt.$ref = ref
+                                elt.$bundled = true
+                                if (!definition.$bundled) {
+                                    definition.$bundled = true
+                                    await this.bundleFun(schema, definition, sources, source)
+                                }
+                            }
+                        }
+                    }
+                } else {
+                    await this.bundleFun(schema, val, sources, source)
                 }
-                return false
-            })
+            }
         }
     }
 
+    // Prune out any unused keys inside of external schemas
+    private prune(elt: any): boolean {
+        let keep = false
+        if (typeof elt === 'object') {
+            keep = elt.$bundled
+            if (!keep) {
+                for (let [key, val] of Object.entries(elt)) {
+                    if (typeof val === 'object' || Array.isArray(val)) {
+                        let childBundled = this.prune(val)
+                        if (!childBundled) {
+                            // Prune any keys of unused structured object
+                            delete elt[key]
+                        }
+                        keep = keep || childBundled
+                    }
+                }
+            }
+        } else if (Array.isArray(elt)) {
+            for (let child of elt) {
+                const childKeep = this.prune(child)
+                keep = keep || childKeep
+            }
+        }
+        return keep
+    }
+
     // Expand $ref below allOf and remove allOf
     private expandAllOf(bundle: any): any {
         walkJSON(bundle, val => {
@@ -1386,17 +1500,18 @@ export default class SchemaMerger {
             this.currentKind = entry.$ref.substring(entry.$ref.lastIndexOf('/') + 1)
             let definition = schema.definitions[this.currentKind]
             let verifyProperty = (val, path) => {
-                if (!val.$schema) {
-                    if (val.$ref) {
-                        val = clone(val)
-                        let ref: any = ptr.get(schema, val.$ref)
-                        for (let prop in ref) {
-                            if (!val[prop]) {
-                                val[prop] = ref[prop]
-                            }
+                if (val.$ref) {
+                    val = clone(val)
+                    let ref: any = ptr.get(schema, val.$ref)
+                    for (let prop in ref) {
+                        if (!val[prop]) {
+                            val[prop] = ref[prop]
                         }
-                        delete val.$ref
                     }
+                    delete val.$ref
+                }
+                if (!val.$schema) {
+                    // Assume $schema is an external reference and ignore error checking
                     if (val.$kind) {
                         let kind = schema.definitions[val.$kind]
                         if (this.roles(kind, 'interface').length > 0) {
@@ -1542,4 +1657,10 @@ export default class SchemaMerger {
         this.error(`Error ${path} does not exist in schema`)
         this.failed = true
     }
+
+    // Missing $ref
+    private refError(original: string, modified: string): void {
+        this.error(`Error could not bundle ${original} into ${modified}`)
+        this.failed = true
+    }
 }

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

@@ -180,7 +180,7 @@ describe('dialog:merge', async () => {
     it('csproj-errors', async () => {
         console.log('\nStart csproj-errors')
         let [merged, lines] = await merge(['projects/project1/project1.csproj'], undefined, true)
-        assert(!merged, 'Merging should faile')
+        assert(!merged, 'Merging should fail')
         assert(countMatches(/error|warning/i, lines) === 3, 'Wrong number of errors or warnings')
         assert(countMatches(/Following.*project1/, lines) === 1, 'Did not follow project1')
         assert(countMatches(/Following nuget.*nuget1.*10.0.1/, lines) === 1, 'Did not follow nuget1')

packages/dialog/test/commands/dialog/nuget/nuget3/1.0.0/Microsoft.AdaptiveDialog.schema

@@ -0,0 +1,65 @@
+{
+    "$schema": "https://schemas.botframework.com/schemas/component/v1.0/component.schema",
+    "$role": "implements(Microsoft.IDialog)",
+    "title": "Adaptive Dialog",
+    "description": "Flexible, data driven dialog that can adapt to the conversation.",
+    "type": "object",
+    "properties": {
+        "id": {
+            "type": "string",
+            "title": "Id",
+            "description": "Optional dialog ID."
+        },
+        "autoEndDialog": {
+            "$ref": "schema:#/definitions/booleanExpression",
+            "title": "Auto end dialog",
+            "description": "If set to true the dialog will automatically end when there are no further actions.  If set to false, remember to manually end the dialog using EndDialog action.",
+            "default": true
+        },
+        "defaultResultProperty": {
+            "type": "string",
+            "title": "Default result property",
+            "description": "Value that will be passed back to the parent dialog.",
+            "default": "dialog.result"
+        },
+        "recognizer": {
+            "$kind": "Microsoft.IRecognizer",
+            "title": "Recognizer",
+            "description": "Input recognizer that interprets user input into intent and entities."
+        },
+        "generator": {
+            "$kind": "Microsoft.ILanguageGenerator",
+            "title": "Language Generator",
+            "description": "Language generator that generates bot responses."
+        },
+        "selector": {
+            "$kind": "Microsoft.ITriggerSelector",
+            "title": "Selector",
+            "description": "Policy to determine which trigger is executed. Defaults to a 'best match' selector (optional)."
+        },
+        "triggers": {
+            "type": "array",
+            "description": "List of triggers defined for this dialog.",
+            "title": "Triggers",
+            "items": {
+                "$kind": "Microsoft.ITrigger",
+                "title": "Event triggers",
+                "description": "Event triggers for handling events."
+            }
+        },
+        "schema": {
+            "title": "Schema",
+            "description": "Schema to fill in.",
+            "anyOf": [
+                {
+                    "$ref": "http://json-schema.org/draft-07/schema#"
+                },
+                {
+                    "type": "string",
+                    "title": "Reference to JSON schema",
+                    "description": "Reference to JSON schema .dialog file."
+                }
+            ]
+        }
+    }
+}

packages/dialog/test/commands/dialog/nuget/nuget3/1.0.0/Microsoft.AdaptiveDialog.uischema

@@ -0,0 +1,24 @@
+{
+    "$schema": "https://schemas.botframework.com/schemas/ui/v1.0/ui.schema",
+    "form": {
+        "label": "Adaptive dialog",
+        "description": "This configures a data driven dialog via a collection of events and actions.",
+        "helpLink": "https://aka.ms/bf-composer-docs-dialog",
+        "order": [
+            "recognizer",
+            "*"
+        ],
+        "hidden": [
+            "triggers",
+            "generator",
+            "selector",
+            "schema"
+        ],
+        "properties": {
+            "recognizer": {
+                "label": "Language Understanding",
+                "description": "To understand what the user says, your dialog needs a \"Recognizer\"; that includes example words and sentences that users may use."
+            }
+        }
+    }
+}

packages/dialog/test/commands/dialog/nuget/nuget3/1.0.0/Microsoft.Ask.schema

@@ -0,0 +1,34 @@
+{
+    "$schema": "https://schemas.botframework.com/schemas/component/v1.0/component.schema",
+    "$role": [ "implements(Microsoft.IDialog)", "extends(Microsoft.SendActivity)" ],
+    "title": "Send Activity to Ask a question",
+    "description": "This is an action which sends an activity to the user when a response is expected",
+    "type": "object",
+    "properties": {
+        "expectedProperties": {
+            "$ref": "schema:#/definitions/arrayExpression",
+            "title": "Expected Properties",
+            "description": "Properties expected from the user.",
+            "items": {
+                "type": "string",
+                "title": "Name",
+                "description": "Name of the property"
+            },
+            "examples": [
+                [
+                    "age",
+                    "name"
+                ]
+            ]
+        },
+        "defaultOperation": {
+            "$ref": "schema:#/definitions/stringExpression",
+            "title": "Default Operation",
+            "description": "Sets the default operation that will be used when no operation is recognized in the response to this Ask.",
+            "examples": [
+                "Add()",
+                "Remove()"
+            ]
+        }
+    }
+}