Replace final parser.dereference to speed up merges (#1203) (#1219)

* Start exploring schema verification. * Verify $ref as part of verifySchema This makes merging much faster. Co-authored-by: Chris McConnell <chrimc> Co-authored-by: Chris McConnell <[email protected]>
microsoft · Apr 26, 2021 · 71bced4 · 71bced4
1 parent 639b6a1
commit 71bced4
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 32 deletions.
diff --git a/.vscode/launch.json b/.vscode/launch.json
@@ -58,7 +58,7 @@
         "999999",
         "--colors",
         "-g",
-        "dialog:merge.*"
+        "dialog:merge.*missing schema.*"
       ],
       "cwd": "${workspaceFolder}/packages/dialog",
       "internalConsoleOptions": "openOnSessionStart",

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
@@ -682,12 +682,7 @@ export class SchemaMerger {
             // Final verification
             this.verifySchema(finalSchema)
             if (!this.failed) {
-                // Verify all refs work
-                const start = process.hrtime.bigint()
-                fullSchema = await parser.dereference(clone(finalSchema))
-                const end = process.hrtime.bigint()
-                const elapsed = Number(end - start) / 1000000000
-                this.vlog(`Expanding all $ref took ${elapsed} seconds`)
+                fullSchema = finalSchema
                 if (!this.checkOnly) {
                     this.log(`Writing ${this.currentFile}`)
                     await fs.writeJSON(this.currentFile, finalSchema, this.jsonOptions)
@@ -1954,14 +1949,19 @@ export class SchemaMerger {
             const definition = schema.definitions[this.currentKind]
             const verifyProperty = (val, path) => {
                 if (val.$ref) {
-                    val = clone(val)
                     const ref: any = ptr.get(schema, val.$ref)
-                    for (const prop in ref) {
-                        if (!val[prop]) {
-                            val[prop] = ref[prop]
+                    if (!ref) {
+                        this.mergingError(`${path} $ref ${val.$ref} does not exist`)
+                    } else {
+                        // Expand $ref to check locally
+                        val = clone(val)
+                        for (const 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
@@ -1995,6 +1995,7 @@ export class SchemaMerger {
             }
             walkJSON(definition, (val, _, path) => {
                 if (val.$schema && path) {
+                    // Embedded non-component schema
                     return true
                 }
                 if (val.properties && (!path || !path.endsWith('properties'))) {

diff --git a/packages/dialog/test/commands/dialog/merge.test.ts b/packages/dialog/test/commands/dialog/merge.test.ts
@@ -195,7 +195,7 @@ describe('dialog:merge', async () => {
         console.log('\nStart missing schema reference')
         const [merged, lines] = await merge(['schemas/*.schema', 'schemas/badSchemas/missingSchemaRef.schema'])
         assert(!merged, 'Merging should have failed')
-        assert(countMatches(/error|warning/i, lines) === 1, 'Wrong number of errors or warnings')
+        assert(countMatches(/error|warning/i, lines) === 3, 'Wrong number of errors or warnings')
         assert(countMatches('does not exist', lines) === 1, 'Did not detect missing schema ref')
     })
 

diff --git a/packages/dialog/test/commands/dialog/schemas/badSchemas/missingSchemaRef.schema b/packages/dialog/test/commands/dialog/schemas/badSchemas/missingSchemaRef.schema
@@ -1,5 +1,6 @@
 {
     "$schema": "https://schemas.botframework.com/schemas/component/v1.0/component.schema",
+    "$role": [],
     "properties": {
         "foo": {
             "$ref": "schema:#/definitions/foo"