From 7f39209154f5d9e69eef3fd80409cf90be1a1b15 Mon Sep 17 00:00:00 2001 From: Niklas Kiefer Date: Sun, 9 Jul 2023 14:58:59 +0200 Subject: [PATCH 1/2] fix(FEEL): vars in filters are properly parsed Related to https://github.com/camunda/team-hto/issues/279 --- .../variableExtractionHelpers.js | 64 ++++++++++++- packages/form-js-viewer/src/util/index.js | 2 - .../variableExtractionHelpers.spec.js | 96 +++++++++++++++++++ .../test/spec/ships-example.json | 19 ++++ .../test/spec/util/GetSchemaVariables.spec.js | 12 +++ 5 files changed, 186 insertions(+), 7 deletions(-) create mode 100644 packages/form-js-viewer/test/spec/features/expression-language/variableExtractionHelpers.spec.js create mode 100644 packages/form-js-viewer/test/spec/ships-example.json diff --git a/packages/form-js-viewer/src/features/expression-language/variableExtractionHelpers.js b/packages/form-js-viewer/src/features/expression-language/variableExtractionHelpers.js index b1caefc1b..d1146cee1 100644 --- a/packages/form-js-viewer/src/features/expression-language/variableExtractionHelpers.js +++ b/packages/form-js-viewer/src/features/expression-language/variableExtractionHelpers.js @@ -1,6 +1,6 @@ import { parseExpression, parseUnaryTests } from 'feelin'; -export const getFlavouredFeelVariableNames = (feelString, feelFlavour, options = {}) => { +export const getFlavouredFeelVariableNames = (feelString, feelFlavour = 'expression', options = {}) => { const { depth = 0, @@ -13,7 +13,7 @@ export const getFlavouredFeelVariableNames = (feelString, feelFlavour, options = const simpleExpressionTree = _buildSimpleFeelStructureTree(tree, feelString); - return (function _unfoldVariables(node) { + const variables = (function _unfoldVariables(node) { if (node.name === 'PathExpression') { @@ -29,14 +29,19 @@ export const getFlavouredFeelVariableNames = (feelString, feelFlavour, options = // for any other kind of node, traverse its children and flatten the result if (node.children) { - return node.children.reduce((acc, child) => { + const variables = node.children.reduce((acc, child) => { return acc.concat(_unfoldVariables(child)); }, []); + + // if we are within a filter context, we need to remove the item variable as it is used for iteration there + return node.name === 'FilterContext' ? variables.filter((name) => name !== 'item') : variables; } return []; })(simpleExpressionTree); + return [ ...new Set(variables) ]; + }; @@ -163,5 +168,54 @@ const _buildSimpleFeelStructureTree = (parseTree, feelString) => { } }); - return stack[0].children[0]; -}; \ No newline at end of file + return _extractFilterExpressions(stack[0].children[0]); +}; + +/** + * Restructure the tree in such a way to bring filters (which create new contexts) to the root of the tree. + * This is done to simplify the extraction of variables and match the context hierarchy. + */ +const _extractFilterExpressions = (tree) => { + + const flattenedExpressionTree = { + name: 'Root', + children: [ tree ] + }; + + const iterate = (node) => { + + if (node.children) { + for (let x = 0; x < node.children.length; x++) { + + if (node.children[x].name === 'FilterExpression') { + + const filterTarget = node.children[x].children[0]; + const filterExpression = node.children[x].children[2]; + + // bypass the filter expression + node.children[x] = filterTarget; + + const taggedFilterExpression = { + name: 'FilterContext', + children: [ filterExpression ] + }; + + // append the filter expression to the root + flattenedExpressionTree.children.push(taggedFilterExpression); + + // recursively iterate the expression + iterate(filterExpression); + + } else { + iterate(node.children[x]); + } + } + + } + + }; + + iterate(tree); + + return flattenedExpressionTree; +}; diff --git a/packages/form-js-viewer/src/util/index.js b/packages/form-js-viewer/src/util/index.js index 02103a722..56ed40893 100644 --- a/packages/form-js-viewer/src/util/index.js +++ b/packages/form-js-viewer/src/util/index.js @@ -143,9 +143,7 @@ export function getSchemaVariables(schema, expressionLanguage = new FeelExpressi const property = get(component, prop.split('.')); if (property && !expressionLanguage.isExpression(property) && templating.isTemplate(property)) { - const templateVariables = templating.getVariableNames(property); - variables = [ ...variables, ...templateVariables ]; } }); diff --git a/packages/form-js-viewer/test/spec/features/expression-language/variableExtractionHelpers.spec.js b/packages/form-js-viewer/test/spec/features/expression-language/variableExtractionHelpers.spec.js new file mode 100644 index 000000000..5cc5f19b0 --- /dev/null +++ b/packages/form-js-viewer/test/spec/features/expression-language/variableExtractionHelpers.spec.js @@ -0,0 +1,96 @@ +import { getFlavouredFeelVariableNames } from '../../../../src/features/expression-language/variableExtractionHelpers'; + +describe('getFlavouredFeelVariableNames', () => { + + expectVariables('SimpleAddition', '2 + 2', []); + + expectVariables('SingleVariable', 'variable1 + 2', [ 'variable1' ]); + + expectVariables('MultipleVariables', 'variable1 + variable2', [ 'variable1', 'variable2' ]); + + expectVariables('ExpressionWithConstants', 'variable1 + 2*3/4 - 5', [ 'variable1' ]); + + expectVariables('MultipleOccurrences', 'variable1 + variable1 - variable1 * variable1 / variable1', [ 'variable1' ]); + + expectVariables('ExpressionWithList', '[variable1, variable2, variable3]', [ 'variable1', 'variable2', 'variable3' ]); + + expectVariables('VariableInContextEntry', '{entry1: variable1}', [ 'variable1' ]); + + expectVariables('VariableInNestedContext', '{entry1: {entry2: variable1}}', [ 'variable1' ]); + + expectVariables('VariableInBooleanExpression', 'variable1 > variable2', [ 'variable1', 'variable2' ]); + + expectVariables('VariableInIfExpression', 'if variable1 > variable2 then "yes" else "no"', [ 'variable1', 'variable2' ]); + + expectVariables('VariableInInterval', '[variable1..variable2]', [ 'variable1', 'variable2' ]); + + expectVariables('VariableInDateFunction', 'date(variable1)', [ 'variable1' ]); + + expectVariables('VariableInTimeFunction', 'time(variable1)', [ 'variable1' ]); + + expectVariables('VariableInDateTimeFunction', 'date and time(variable1)', [ 'variable1' ]); + + expectVariables('NestedList', '[[variable1, variable2], [variable3, variable4]]', [ 'variable1', 'variable2', 'variable3', 'variable4' ]); + + expectVariables('ListInContextEntry', '{entry1: [variable1, variable2]}', [ 'variable1', 'variable2' ]); + + expectVariables('VariableInUnaryTest', 'variable1 in (variable2..variable3)', [ 'variable1', 'variable2', 'variable3' ]); + + expectVariables('ArrayAccessorSingleVariable', 'variable1[1]', [ 'variable1' ]); + + expectVariables('ArrayAccessorMultipleVariables', 'variable1[variable2]', [ 'variable1', 'variable2' ]); + + expectVariables('ArrayAccessorNestedVariables', 'variable1[variable2[1]]', [ 'variable1', 'variable2' ]); + + expectVariables('NestedArrayAccessor', 'variable1[variable2[1]][2]', [ 'variable1', 'variable2' ]); + + expectVariables('VariableInArrayFilter', 'variable1[item > 3]', [ 'variable1' ]); + + expectVariables('ComplexArrayFilter', 'variable1[item.a > 3 and item.b < 5]', [ 'variable1' ]); + + + // TODO(@skaiir) these tests should ideally pass, but right now our variable extraction logic doesn't ignore certain keywords + // https://github.com/bpmn-io/form-js/issues/710 + + describe.skip('Oversensitive', () => { + + expectVariables('FunctionWithConstants', 'sum(1, 2, 3)', []); + + expectVariables('FunctionWithVariable', 'sum(variable1, 2, 3)', [ 'variable1' ]); + + expectVariables('FunctionWithMultipleVariables', 'sum(variable1, variable2, variable3)', [ 'variable1', 'variable2', 'variable3' ]); + + expectVariables('FunctionNestedInFunction', 'sum(product(variable1, variable2), sum(variable3, variable4))', [ 'variable1', 'variable2', 'variable3', 'variable4' ]); + + expectVariables('FunctionInList', '[sum(variable1, variable2), product(variable3, variable4)]', [ 'variable1', 'variable2', 'variable3', 'variable4' ]); + + expectVariables('VariableInFunction', 'sum(variable1, variable2)', [ 'variable1', 'variable2' ]); + + expectVariables('NestedFunctions', 'sum(product(variable1, variable2), variable3)', [ 'variable1', 'variable2', 'variable3' ]); + + expectVariables('ContextEntryWithFunction', '{entry1: sum(variable1, variable2)}', [ 'variable1', 'variable2' ]); + + expectVariables('FunctionInContextEntry', '{entry1: sum(variable1, variable2), entry2: product(variable3, variable4)}', [ 'variable1', 'variable2', 'variable3', 'variable4' ]); + + expectVariables('VariableInUnaryNotTest', 'variable1 not in (variable2..variable3)', [ 'variable1', 'variable2', 'variable3' ]); + + expectVariables('VariableInQuantifiedExpression', 'some x in variable1 satisfies x > variable2', [ 'variable1', 'variable2' ]); + + expectVariables('VariableInInstanceOfTest', 'variable1 instance of tNumber', [ 'variable1' ]); + + expectVariables('VariableInForExpression', 'for x in variable1 return x > variable2', [ 'variable1', 'variable2' ]); + + expectVariables('ArrayAccessorInFunction', 'sum(variable1[1], variable2[2])', [ 'variable1', 'variable2' ]); + + expectVariables('ArrayFilterInFunction', 'sum(variable1[item > 3])', [ 'variable1' ]); + + }); + +}); + + +// helpers /////////////// + +function expectVariables(name, feel, variables) { + it(name, () => expect(getFlavouredFeelVariableNames(feel)).to.eql(variables)); +} \ No newline at end of file diff --git a/packages/form-js-viewer/test/spec/ships-example.json b/packages/form-js-viewer/test/spec/ships-example.json new file mode 100644 index 000000000..fa721e0b4 --- /dev/null +++ b/packages/form-js-viewer/test/spec/ships-example.json @@ -0,0 +1,19 @@ +{ + "components": [ + { + "text": "#### Ship Name:\n {{shipsForSale[item.name = selectedShip].name}}\n#### Ship Description: \n {{shipsForSale[item.name = selectedShip].description}}\n#### Ship Price: \n {{shipsForSale[item.name = selectedShip].purchasePrice}}\n#### Frame:\n {{shipsForSale[item.name = selectedShip].frame.name}}\n#### Reactor:\n {{shipsForSale[item.name = selectedShip].reactor.name}}\n#### Engine:\n {{shipsForSale[item.name = selectedShip].engine.name}}\n#### Moduels:\n {{shipsForSale[item.name = selectedShip].modules.name}}\n", + "type": "text", + "layout": { + "row": "Row_1oxhkbh", + "columns": null + }, + "id": "Field_10y8si5", + "conditional": { + "hide": "=selectedShip = null" + } + } + ], + "schemaVersion": 9, + "type": "default", + "id": "Form_0ysvmoe" +} \ No newline at end of file diff --git a/packages/form-js-viewer/test/spec/util/GetSchemaVariables.spec.js b/packages/form-js-viewer/test/spec/util/GetSchemaVariables.spec.js index 770f8e53a..24ef78c85 100644 --- a/packages/form-js-viewer/test/spec/util/GetSchemaVariables.spec.js +++ b/packages/form-js-viewer/test/spec/util/GetSchemaVariables.spec.js @@ -15,6 +15,7 @@ import adornersSchema from '../appearance.json'; import imagesSchema from '../images.json'; import valuesExpressionSchema from '../valuesExpression.json'; import validateSchema from '../validate.json'; +import shipsExampleSchema from '../ships-example.json'; describe('util/getSchemaVariables', () => { @@ -148,4 +149,15 @@ describe('util/getSchemaVariables', () => { ]); }); + + it('should include variables in ships example', () => { + + const variables = getSchemaVariables(shipsExampleSchema); + + expect(variables).to.eql([ + 'selectedShip', + 'shipsForSale' + ]); + }); + }); \ No newline at end of file From 8c9a139a681acb8a81922e25292a0b8a60eaf5cd Mon Sep 17 00:00:00 2001 From: Valentin Serra Date: Wed, 12 Jul 2023 11:14:39 +0200 Subject: [PATCH 2/2] fix(FEEL): ignore null and undef vars Related to camunda/team-hto#279 --- packages/form-js-viewer/src/util/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/form-js-viewer/src/util/index.js b/packages/form-js-viewer/src/util/index.js index 56ed40893..cac8bf6b9 100644 --- a/packages/form-js-viewer/src/util/index.js +++ b/packages/form-js-viewer/src/util/index.js @@ -148,7 +148,7 @@ export function getSchemaVariables(schema, expressionLanguage = new FeelExpressi } }); - return variables; + return variables.filter(variable => variable !== undefined || variable !== null); }, []); // remove duplicates