From 4f7f1f407ff53abbf28d0212ac61bd8a6cc392fc Mon Sep 17 00:00:00 2001 From: leilzh Date: Wed, 12 Feb 2020 23:10:10 +0800 Subject: [PATCH 1/3] update the validation --- .../indexers/src/dialogUtils/dialogChecker.ts | 79 ++++++++++--------- .../indexers/src/dialogUtils/validation.ts | 78 ++++++++++++++++++ 2 files changed, 119 insertions(+), 38 deletions(-) create mode 100644 Composer/packages/lib/indexers/src/dialogUtils/validation.ts diff --git a/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts b/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts index a0d67ec67a..ddbcc48cc2 100644 --- a/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts +++ b/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts @@ -2,15 +2,13 @@ // Licensed under the MIT License. import get from 'lodash/get'; -import { ExpressionEngine } from 'botframework-expressions'; -import formatMessage from 'format-message'; -import { SDKTypes, FieldNames } from '@bfc/shared'; +import { FieldNames } from '@bfc/shared'; import { Diagnostic } from '../diagnostic'; +import { ExpressionType } from './validation'; import { CheckerFunc } from './types'; - -const ExpressionParser = new ExpressionEngine(); +import { validate } from './validation'; const createPath = (path: string, type: string): string => { const steps = [FieldNames.Events, FieldNames.Actions, FieldNames.ElseActions]; @@ -24,27 +22,7 @@ const createPath = (path: string, type: string): string => { return `${list[0]}${focused}#${type}#${list[1]}`; }; -export const checkExpression = (exp: string, required: boolean, path: string, type: string): Diagnostic | null => { - let message = ''; - if (!exp && required) { - message = formatMessage(`is missing or empty`); - } else { - try { - ExpressionParser.parse(exp); - } catch (error) { - message = `${formatMessage('must be an expression:')} ${error})`; - } - } - if (message) { - const diagnostic = new Diagnostic(message, ''); - diagnostic.path = createPath(path, type); - return diagnostic; - } - - return null; -}; - -function findAllRequiredType(schema: any): { [key: string]: boolean } { +function findAllRequiredProperties(schema: any): { [key: string]: boolean } { if (!schema) return {}; const types = schema.anyOf?.filter(x => x.title === 'Type'); const required = {}; @@ -62,17 +40,38 @@ function findAllRequiredType(schema: any): { [key: string]: boolean } { return required; } +function findAllTypes(schema: any): string[] { + if (!schema) return []; + let types: string[] = []; + if (schema.type) { + if (Array.isArray(schema.type)) { + types = [...types, ...schema.type]; + } else { + types.push(schema.type); + } + } else { + types = schema.oneOf?.filter(item => !!ExpressionType[item.type]).map(item => item.type); + } + + return types; +} + export const IsExpression: CheckerFunc = (path, value, type, schema) => { if (!schema) return []; const diagnostics: Diagnostic[] = []; - const requiredTypes = findAllRequiredType(schema); + const requiredProperties = findAllRequiredProperties(schema); Object.keys(value).forEach(key => { const property = value[key]; if (Array.isArray(property)) { const itemsSchema = get(schema, ['properties', key, 'items'], null); if (itemsSchema?.$role === 'expression') { property.forEach((child, index) => { - const diagnostic = checkExpression(child, !!requiredTypes[key], `${path}.${key}[${index}]`, type); + const diagnostic = validate( + child, + !!requiredProperties[key], + createPath(`${path}.${key}[${index}]`, type), + findAllTypes(itemsSchema) + ); if (diagnostic) diagnostics.push(diagnostic); }); } else if (itemsSchema?.type === 'object') { @@ -82,7 +81,12 @@ export const IsExpression: CheckerFunc = (path, value, type, schema) => { }); } } else if (get(schema.properties[key], '$role') === 'expression') { - const diagnostic = checkExpression(property, !!requiredTypes[key], `${path}.${key}`, type); + const diagnostic = validate( + property, + !!requiredProperties[key], + createPath(`${path}.${key}`, type), + findAllTypes(schema.properties[key]) + ); if (diagnostic) diagnostics.push(diagnostic); } }); @@ -90,16 +94,15 @@ export const IsExpression: CheckerFunc = (path, value, type, schema) => { }; //the type of 'Microsoft.ChoiceInput' has anyof schema in choices -export const checkChoices: CheckerFunc = (path, value, type, schema) => { - const choices = value.choices; - if (typeof choices === 'string') { - const diagnostic = checkExpression(choices, false, `${path}.choices`, type); - if (diagnostic) return [diagnostic]; - } - return null; -}; +// export const checkChoices: CheckerFunc = (path, value, type) => { +// const choices = value.choices; +// if (typeof choices === 'string') { +// const diagnostic = checkExpression(choices, false, `${path}.choices`, type); +// if (diagnostic) return [diagnostic]; +// } +// return null; +// }; export const checkerFuncs: { [type: string]: CheckerFunc[] } = { '.': [IsExpression], //this will check all types - [SDKTypes.ChoiceInput]: [checkChoices], }; diff --git a/Composer/packages/lib/indexers/src/dialogUtils/validation.ts b/Composer/packages/lib/indexers/src/dialogUtils/validation.ts new file mode 100644 index 0000000000..a3688dab20 --- /dev/null +++ b/Composer/packages/lib/indexers/src/dialogUtils/validation.ts @@ -0,0 +1,78 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +import { ExpressionEngine, ReturnType } from 'botframework-expressions'; +import formatMessage from 'format-message'; + +import { Diagnostic } from '../diagnostic'; + +export const ExpressionType = { + number: 'number', + integer: 'integer', + boolean: 'boolean', + string: 'string', +}; + +const ExpressionParser = new ExpressionEngine(); + +const isExpression = (value: string | boolean | number, types: string[]): boolean => { + if (typeof value === 'string' && value[0] === '=') return true; + //StringExpression always assumes string interpolation unless prefixed with =, producing a string + if (types.length === 1 && types[0] === ExpressionType.string) return false; + return true; +}; + +//The return type should match the schema type +const checkReturnType = (returnType: ReturnType, types: string[]): string => { + if (types.indexOf(returnType) > -1) return ''; + if (returnType === ReturnType.Number && types.indexOf(ExpressionType.integer) > -1) { + return ''; + } + return formatMessage('the expression type is not match'); +}; + +export const checkExpression = (exp: string | boolean | number, required: boolean, types: string[]): string => { + let message = ''; + if (!exp && required) { + message = formatMessage(`is missing or empty`); + } else { + try { + let returnType: ReturnType; + if (typeof exp === 'boolean') { + returnType = ReturnType.Boolean; + } else if (typeof exp === 'number') { + returnType = ReturnType.Number; + } else { + returnType = ExpressionParser.parse(exp).returnType; + } + message = checkReturnType(returnType, types); + } catch (error) { + message = `${formatMessage('must be an expression:')} ${error})`; + } + } + + return message; +}; + +export const validate = ( + value: string | boolean | number, + required: boolean, + path: string, + types: string[] +): Diagnostic | null => { + //if there is no type do nothing + if (types.length === 0) return null; + + if (!isExpression(value, types)) return null; + + //remove '=' + if (typeof value === 'string' && value[0] === '=') { + value = value.substring(1); + } + + const message = checkExpression(value, required, types); + if (!message) return null; + + const diagnostic = new Diagnostic(message, ''); + diagnostic.path = path; + return diagnostic; +}; From ed693065b8b1052f98cc3d16a428b1982bb2ab00 Mon Sep 17 00:00:00 2001 From: leilzh Date: Thu, 13 Feb 2020 08:36:34 +0800 Subject: [PATCH 2/3] fix some comments --- .../indexers/src/dialogUtils/dialogChecker.ts | 10 ---------- .../lib/indexers/src/dialogUtils/validation.ts | 18 +++++++----------- 2 files changed, 7 insertions(+), 21 deletions(-) diff --git a/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts b/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts index ddbcc48cc2..bc4c2983e2 100644 --- a/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts +++ b/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts @@ -93,16 +93,6 @@ export const IsExpression: CheckerFunc = (path, value, type, schema) => { return diagnostics; }; -//the type of 'Microsoft.ChoiceInput' has anyof schema in choices -// export const checkChoices: CheckerFunc = (path, value, type) => { -// const choices = value.choices; -// if (typeof choices === 'string') { -// const diagnostic = checkExpression(choices, false, `${path}.choices`, type); -// if (diagnostic) return [diagnostic]; -// } -// return null; -// }; - export const checkerFuncs: { [type: string]: CheckerFunc[] } = { '.': [IsExpression], //this will check all types }; diff --git a/Composer/packages/lib/indexers/src/dialogUtils/validation.ts b/Composer/packages/lib/indexers/src/dialogUtils/validation.ts index a3688dab20..0b6623eb29 100644 --- a/Composer/packages/lib/indexers/src/dialogUtils/validation.ts +++ b/Composer/packages/lib/indexers/src/dialogUtils/validation.ts @@ -15,19 +15,15 @@ export const ExpressionType = { const ExpressionParser = new ExpressionEngine(); const isExpression = (value: string | boolean | number, types: string[]): boolean => { - if (typeof value === 'string' && value[0] === '=') return true; //StringExpression always assumes string interpolation unless prefixed with =, producing a string - if (types.length === 1 && types[0] === ExpressionType.string) return false; - return true; + return (typeof value === 'string' && value[0] === '=') || types.length !== 1 || types[0] !== ExpressionType.string; }; //The return type should match the schema type const checkReturnType = (returnType: ReturnType, types: string[]): string => { - if (types.indexOf(returnType) > -1) return ''; - if (returnType === ReturnType.Number && types.indexOf(ExpressionType.integer) > -1) { - return ''; - } - return formatMessage('the expression type is not match'); + return ~types.indexOf(returnType) || (returnType === ReturnType.Number && ~types.indexOf(ExpressionType.integer)) + ? '' + : formatMessage('the expression type is not match'); }; export const checkExpression = (exp: string | boolean | number, required: boolean, types: string[]): string => { @@ -60,9 +56,9 @@ export const validate = ( types: string[] ): Diagnostic | null => { //if there is no type do nothing - if (types.length === 0) return null; - - if (!isExpression(value, types)) return null; + if (!types.length || !isExpression(value, types)) { + return null; + } //remove '=' if (typeof value === 'string' && value[0] === '=') { From 8f820b448db70da57290da728f9a2a7914566988 Mon Sep 17 00:00:00 2001 From: leilzh Date: Thu, 13 Feb 2020 13:21:33 +0800 Subject: [PATCH 3/3] update some logic --- .../packages/lib/indexers/src/dialogUtils/validation.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Composer/packages/lib/indexers/src/dialogUtils/validation.ts b/Composer/packages/lib/indexers/src/dialogUtils/validation.ts index 0b6623eb29..040f04c34a 100644 --- a/Composer/packages/lib/indexers/src/dialogUtils/validation.ts +++ b/Composer/packages/lib/indexers/src/dialogUtils/validation.ts @@ -21,7 +21,9 @@ const isExpression = (value: string | boolean | number, types: string[]): boolea //The return type should match the schema type const checkReturnType = (returnType: ReturnType, types: string[]): string => { - return ~types.indexOf(returnType) || (returnType === ReturnType.Number && ~types.indexOf(ExpressionType.integer)) + return returnType === ReturnType.Object || + ~types.indexOf(returnType) || + (returnType === ReturnType.Number && ~types.indexOf(ExpressionType.integer)) ? '' : formatMessage('the expression type is not match'); }; @@ -56,7 +58,8 @@ export const validate = ( types: string[] ): Diagnostic | null => { //if there is no type do nothing - if (!types.length || !isExpression(value, types)) { + //if the json type length more than 2, the type assumes string interpolation + if (!types.length || types.length > 2 || !isExpression(value, types)) { return null; }