From 6537b646aba79b0fd5575162327b3061e7725dae Mon Sep 17 00:00:00 2001 From: Philipp Fromme Date: Mon, 25 Aug 2025 17:06:24 +0200 Subject: [PATCH 1/5] test: assert variables with different scopes not filtered by uniqueness --- test/fixtures/zeebe/simple.bpmn | 34 ++++++++++----- test/spec/zeebe/ZeebeVariableResolver.spec.js | 41 ++++++++++++++++++- 2 files changed, 62 insertions(+), 13 deletions(-) diff --git a/test/fixtures/zeebe/simple.bpmn b/test/fixtures/zeebe/simple.bpmn index e77e398..af26a56 100644 --- a/test/fixtures/zeebe/simple.bpmn +++ b/test/fixtures/zeebe/simple.bpmn @@ -1,37 +1,49 @@ - + - - Flow_16gvdav - - - Flow_0elge1o + Flow_00zecls - + + + Flow_16gvdav Flow_0elge1o + + Flow_0elge1o + Flow_00zecls + + + Flow_16gvdav + - - - + + + + + + - + + + + + diff --git a/test/spec/zeebe/ZeebeVariableResolver.spec.js b/test/spec/zeebe/ZeebeVariableResolver.spec.js index 2890534..b797f0d 100644 --- a/test/spec/zeebe/ZeebeVariableResolver.spec.js +++ b/test/spec/zeebe/ZeebeVariableResolver.spec.js @@ -365,7 +365,7 @@ describe('ZeebeVariableResolver', function() { ); - it('should merge variables of same scope', inject(async function(variableResolver, elementRegistry) { + it('should merge variables of same scope and same name', inject(async function(variableResolver, elementRegistry) { // given const root = elementRegistry.get('Process_1'); @@ -385,7 +385,44 @@ describe('ZeebeVariableResolver', function() { const variables = await variableResolver.getVariablesForElement(root); // then - expect(variables).to.variableEqual([ { name: 'foo', type: 'String', scope: 'Process_1', origin: [ 'ServiceTask_1', 'Process_1' ] } ]); + expect(variables).to.variableEqual([ + { name: 'foo', type: 'String', scope: 'Process_1', origin: [ 'ServiceTask_1', 'Process_1' ] } + ]); + })); + + + it('should not merge variables of different scopes and same name', inject(async function(variableResolver, elementRegistry) { + + // given + const serviceTask1 = elementRegistry.get('ServiceTask_1'), + serviceTask2 = elementRegistry.get('ServiceTask_2'); + + createProvider({ + variables: [ { name: 'foo', type: 'String', scope: serviceTask1 } ], + variableResolver, + origin: 'ServiceTask_1' + }); + createProvider({ + variables: [ { name: 'foo', type: 'String', scope: serviceTask2 } ], + variableResolver, + origin: 'ServiceTask_2' + }); + + // when + let variables = await variableResolver.getVariablesForElement(serviceTask1); + + // then + expect(variables).to.variableEqual([ + { name: 'foo', type: 'String', scope: 'ServiceTask_1', origin: [ 'ServiceTask_1' ] }, + ]); + + // when + variables = await variableResolver.getVariablesForElement(serviceTask2); + + // then + expect(variables).to.variableEqual([ + { name: 'foo', type: 'String', scope: 'ServiceTask_2', origin: [ 'ServiceTask_2' ] }, + ]); })); From 02e9c98262c5560884b406c89b3777abff67b94f Mon Sep 17 00:00:00 2001 From: Philipp Fromme Date: Wed, 10 Sep 2025 16:17:50 +0200 Subject: [PATCH 2/5] fix: filter by uniqueness after filtering by scope * also, don't filter out external variables Closes #54 --- lib/base/VariableResolver.js | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/lib/base/VariableResolver.js b/lib/base/VariableResolver.js index 53611bc..3107762 100644 --- a/lib/base/VariableResolver.js +++ b/lib/base/VariableResolver.js @@ -1,7 +1,6 @@ import { getBusinessObject, is } from 'bpmn-js/lib/util/ModelUtil'; import CachedValue from './util/CachedValue'; import { getParents, getScope } from './util/scopeUtil'; -import { uniqueBy } from 'min-dash'; /** * @typedef {Object} AdditionalVariable @@ -258,24 +257,32 @@ export class BaseVariableResolver { const root = getRootElement(bo); const allVariables = await this.getProcessVariables(root); - // keep only unique variables based on name property - const uniqueVariables = uniqueBy('name', allVariables.reverse()); - // (1) get variables for given scope - var scopeVariables = uniqueVariables.filter(function(variable) { + var scopeVariables = allVariables.filter(function(variable) { return variable.scope.id === bo.id; }); // (2) get variables for parent scopes var parents = getParents(bo); - var parentsScopeVariables = uniqueVariables.filter(function(variable) { + var parentsScopeVariables = allVariables.filter(function(variable) { return parents.find(function(parent) { return parent.id === variable.scope.id; }); }); - return [ ...scopeVariables, ...parentsScopeVariables ]; + const reversedVariables = [ ...scopeVariables, ...parentsScopeVariables ].reverse(); + + return reversedVariables.filter(variable => { + + // if external variable, keep + if (variable.provider.find(extractor => extractor !== this._baseExtractor)) { + return true; + } + + // if not external, keep only if first of its name + return reversedVariables.find(v => v.name === variable.name) === variable; + }); } } From 3d95fd74692d86b9a29f7ecc21d50d5110b1a024 Mon Sep 17 00:00:00 2001 From: Philipp Fromme Date: Wed, 10 Sep 2025 16:50:45 +0200 Subject: [PATCH 3/5] chore: fix type --- lib/base/VariableResolver.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/base/VariableResolver.js b/lib/base/VariableResolver.js index 3107762..6ec5e46 100644 --- a/lib/base/VariableResolver.js +++ b/lib/base/VariableResolver.js @@ -16,6 +16,7 @@ import { getParents, getScope } from './util/scopeUtil'; * @typedef {AdditionalVariable} ProcessVariable * @property {Array} origin * @property {ModdleElement} scope + * @property {Array} provider */ /** From d19818314a267333f21c17e46af5df41645122a9 Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Thu, 11 Sep 2025 15:33:32 +0200 Subject: [PATCH 4/5] docs: improve jsdoc --- lib/base/VariableResolver.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/base/VariableResolver.js b/lib/base/VariableResolver.js index 6ec5e46..373601d 100644 --- a/lib/base/VariableResolver.js +++ b/lib/base/VariableResolver.js @@ -49,7 +49,7 @@ export class BaseVariableResolver { } /** - * To be implemented by super class. This should be an instance of `getProcessVariables` from `@bpmn-io/extract-process-variables`, + * To be implemented by a subclass. This should be an instance of `getProcessVariables` from `@bpmn-io/extract-process-variables`, * either C7 or C8. * * @returns {Promise>} From d6af0a61d156f9ef4fbeb18ed4e199b0d26e534a Mon Sep 17 00:00:00 2001 From: Philipp Fromme Date: Mon, 15 Sep 2025 10:12:16 +0200 Subject: [PATCH 5/5] chore: improve performance of check --- lib/base/VariableResolver.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/base/VariableResolver.js b/lib/base/VariableResolver.js index 373601d..a5e0523 100644 --- a/lib/base/VariableResolver.js +++ b/lib/base/VariableResolver.js @@ -274,6 +274,8 @@ export class BaseVariableResolver { const reversedVariables = [ ...scopeVariables, ...parentsScopeVariables ].reverse(); + const seenNames = new Set(); + return reversedVariables.filter(variable => { // if external variable, keep @@ -281,8 +283,14 @@ export class BaseVariableResolver { return true; } - // if not external, keep only if first of its name - return reversedVariables.find(v => v.name === variable.name) === variable; + // if not external, keep only the first occurrence of each name + if (!seenNames.has(variable.name)) { + seenNames.add(variable.name); + + return true; + } + + return false; }); } }