Skip to content

Commit

Permalink
fix(FEEL): vars in filters are properly parsed
Browse files Browse the repository at this point in the history
  • Loading branch information
Niklas Kiefer authored and Skaiir committed Jul 12, 2023
1 parent ce14684 commit 8461dc4
Show file tree
Hide file tree
Showing 5 changed files with 187 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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') {

Expand All @@ -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) ];

};


Expand Down Expand Up @@ -163,5 +168,54 @@ const _buildSimpleFeelStructureTree = (parseTree, feelString) => {
}
});

return stack[0].children[0];
};
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;
};
2 changes: 0 additions & 2 deletions packages/form-js-viewer/src/util/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ];
}
});
Expand Down
Original file line number Diff line number Diff line change
@@ -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));
}
19 changes: 19 additions & 0 deletions packages/form-js-viewer/test/spec/ships-example.json
Original file line number Diff line number Diff line change
@@ -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"
}
13 changes: 13 additions & 0 deletions packages/form-js-viewer/test/spec/util/GetSchemaVariables.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {

Expand Down Expand Up @@ -148,4 +149,16 @@ describe('util/getSchemaVariables', () => {
]);
});


it('should include variables in ships example', () => {

const variables = getSchemaVariables(shipsExampleSchema);

// currently produces: ['selectedShip', undefined] :zap:
expect(variables).to.eql([
'selectedShip',
'shipsForSale'
]);
});

});

0 comments on commit 8461dc4

Please sign in to comment.