From 71bced40b774da198fdf1e8c002e9ff4dc500f7e Mon Sep 17 00:00:00 2001 From: Emilio Munoz Date: Mon, 26 Apr 2021 14:06:30 -0700 Subject: [PATCH] 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 Co-authored-by: Chris McConnell --- .vscode/launch.json | 2 +- common/config/rush/pnpm-lock.yaml | 44 +++++++++++-------- packages/dialog/src/library/schemaMerger.ts | 23 +++++----- .../dialog/test/commands/dialog/merge.test.ts | 2 +- .../badSchemas/missingSchemaRef.schema | 1 + 5 files changed, 40 insertions(+), 32 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 30dcc9240..dd716021f 100644 --- 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 index 5cb7987fd..c754f2902 100644 --- a/common/config/rush/pnpm-lock.yaml +++ b/common/config/rush/pnpm-lock.yaml @@ -31,7 +31,7 @@ dependencies: applicationinsights: 1.7.3 argparse: 1.0.10 axios: 0.21.1 - botbuilder-lg: 4.12.0 + botbuilder-lg: 4.13.0 botframework-schema: 4.7.2 camelcase: 4.1.0 chai: 4.2.0 @@ -646,6 +646,10 @@ packages: dev: false resolution: integrity: sha512-7bjymPR7Ffa1/L3HskkaxMgTQDtwFObbISzHm9g3T12VyD89IiHS3BBVojlQHyZRiIilzdh0WT1gwwgyyBtLGQ== + /@types/btoa-lite/1.0.0: + dev: false + resolution: + integrity: sha512-wJsiX1tosQ+J5+bY5LrSahHxr2wT+uME5UDwdN1kg4frt40euqA+wzECkmq4t5QbveHiJepfdThgQrPw6KiSlg== /@types/chai/4.2.10: dev: false resolution: @@ -811,6 +815,10 @@ packages: dev: false resolution: integrity: sha1-xEKLDKhtO4gUdXJv2UmAs4onw4E= + /@types/xmldom/0.1.30: + dev: false + resolution: + integrity: sha512-edqgAFXMEtVvaBZ3YnhamvmrHjoYpuxETmnb0lbTZmf/dXpAsO9ZKotUO4K2rn2SIZBDFCMOuA7fOe0H6dRZcA== /@typescript-eslint/eslint-plugin/2.22.0_dd02fbebb9a2a09e673beb1cc27b6731: dependencies: '@typescript-eslint/experimental-utils': 2.22.0_eslint@5.16.0+typescript@4.0.3 @@ -985,28 +993,29 @@ packages: hasBin: true resolution: integrity: sha512-gac8OEcQ2Li1dxIEWGZzsp2BitJxwkwcOm0zHAJLcPJaVvm58FRnk6RkuLRpU1EujipU2ZFODv2P9DLMfnV8mw== - /adaptive-expressions/4.12.0: + /adaptive-expressions/4.13.0: dependencies: '@microsoft/recognizers-text-data-types-timex-expression': 1.3.0 '@types/atob-lite': 2.0.0 + '@types/btoa-lite': 1.0.0 '@types/lru-cache': 5.1.0 - '@types/xmldom': 0.1.29 + '@types/xmldom': 0.1.30 antlr4ts: 0.5.0-alpha.3 atob-lite: 2.0.0 big-integer: 1.6.48 + btoa-lite: 1.0.0 d3-format: 1.4.5 dayjs: 1.10.4 jspath: 0.4.0 - lodash: 4.17.19 lru-cache: 5.1.1 uuid: 8.3.2 x2js: 3.4.1 xml2js: 0.4.23 - xmldom: 0.4.0 + xmldom: 0.5.0 xpath: 0.0.32 dev: false resolution: - integrity: sha512-gJf2Uq/FT/iX4mIxdnr3gcw6KMjchFc971q1LKt1hrfAMFJtxTy9ukBMgTIbjlOFASMUGSLSU0CKEOzmNJWLbA== + integrity: sha512-+MZbnDuJhXDSXvl/xqmPB4ytgb6vBZMUTm7CydqMszwL9IC+BFy+sypRVexedEaETZKSPC/SDQCl6L5qqR9pIw== /adaptive-expressions/4.8.0-preview: dependencies: '@microsoft/recognizers-text-data-types-timex-expression': 1.1.4 @@ -1396,16 +1405,16 @@ packages: dev: false resolution: integrity: sha512-SaLwpHoDmarRMi9KeTBefzhAhCfyu7ImB/hxdYDHxJFIz2otcyp6WqiWQRD8SpfXeo6VUl/jKD3dm+2LHrwveg== - /botbuilder-lg/4.12.0: + /botbuilder-lg/4.13.0: dependencies: - adaptive-expressions: 4.12.0 + adaptive-expressions: 4.13.0 antlr4ts: 0.5.0-alpha.3 lodash: 4.17.19 path: 0.12.7 uuid: 8.3.2 dev: false resolution: - integrity: sha512-scjO5YfpVr1CYeIHeoz8vEKg8q+iYJ8ShStM4N8qlzYkwUoq2pP+alp8xCf38m6v07/KXgGkV6Nmyzsnl8/8cA== + integrity: sha512-bJZxDZHsE5+OCouD+Pin7ToTYEKKjUSNx1+EqoMjyHIMQH/5aKwK5LLeK5QZAvjZ6XXiHuEP4QM67pKxjFTCSA== /botbuilder-lg/4.8.0-preview: dependencies: adaptive-expressions: 4.8.0-preview @@ -1461,6 +1470,10 @@ packages: dev: false resolution: integrity: sha512-qhAVI1+Av2X7qelOfAIYwXONood6XlZE/fXaBSmW/T5SzLAmCgzi+eiWE7fUvbHaeNBQH13UftjpXxsfLkMpgw== + /btoa-lite/1.0.0: + dev: false + resolution: + integrity: sha1-M3dm2hWAEhD92VbCLpxokaudAzc= /buffer-from/1.1.1: dev: false resolution: @@ -7232,13 +7245,6 @@ packages: node: '>=4.0' resolution: integrity: sha512-fDlsI/kFEx7gLvbecc0/ohLG50fugQp8ryHzMTuW9vSa1GJ0XYWKnhsUx7oie3G98+r56aTQIUB4kht42R3JvA== - /xmldom/0.4.0: - deprecated: Deprecated due to CVE-2021-21366 resolved in 0.5.0 - dev: false - engines: - node: '>=10.0.0' - resolution: - integrity: sha512-2E93k08T30Ugs+34HBSTQLVtpi6mCddaY8uO+pMNk1pqSjV5vElzn4mmh6KLxN3hki8rNcHSYzILoh3TEWORvA== /xmldom/0.5.0: dev: false engines: @@ -7564,7 +7570,7 @@ packages: '@types/node': 10.17.17 '@types/node-fetch': 2.5.4 '@types/readline-sync': 1.4.3 - botbuilder-lg: 4.12.0 + botbuilder-lg: 4.13.0 chai: 4.2.0 delay: 4.3.0 eslint: 5.16.0 @@ -7585,7 +7591,7 @@ packages: dev: false name: '@rush-temp/bf-lg-cli' resolution: - integrity: sha512-W/0N73RLtJWpxsEKBb8fTEft9YHLBHuWNJ8u+ohOvkKI1FtuHuNq51hu7jrfI4/gisCvTdIg9WCzEYBPSxbbzQ== + integrity: sha512-FmtvJ8tiv8TVNyiBtWVIBuz11c+D1vp9l6eMEOZrUltDR633uMkAKGbsjPNcWnSRsTEElkZSXyaVuWq5n6g4Xw== tarball: 'file:projects/bf-lg-cli.tgz' version: 0.0.0 'file:projects/bf-lu.tgz': @@ -7861,7 +7867,7 @@ specifiers: applicationinsights: ^1.0.8 argparse: ~1.0.10 axios: ~0.21.1 - botbuilder-lg: 4.12.0 + botbuilder-lg: 4.13.0 botframework-schema: ^4.5.1 camelcase: ^4.1.0 chai: ^4.2.0 diff --git a/packages/dialog/src/library/schemaMerger.ts b/packages/dialog/src/library/schemaMerger.ts index 4c016fa9c..dc0d0ca08 100644 --- 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 index 5020dcbf3..504451d63 100644 --- 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 index 4bba78f44..17348a36b 100644 --- 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"