Don't overwrite local multi-instance variables #9947
Conversation
When the output element expression is a variable (or a nested property of a variable) then it needs to be nil initialized, to make sure that the variable exists at the local scope. However, when the output element expression refers to the same variable as the input element, then it shouldn't overwrite this variable. Note that the input variable is already created at the local scope. This adds a test case that fails if the output element variable is nil initialized when it refers to the same variable as the input element.
When the output element expression is a variable (or a nested property of a variable) then it needs to be nil initialized, to make sure that the variable exists at the local scope. However, when the output element expression refers to the same variable as the input element, then it shouldn't overwrite this variable. Note that the input variable is already created at the local scope. We can simply filter out this case, and only nil init the other output element variables.
The zeebe extension property Output Element must be an expression. The method only exists to allow a test case that rejects a process when the output element is a static value.
When the output element expression is a variable (or a nested property of a variable) then it needs to be nil initialized, to make sure that the variable exists at the local scope. However, when the output element expression refers to the loopCounter variable, then it shouldn't overwrite this variable. Note that the loopCounter is already created at the local scope. This adds a test case that fails if the output element variable is nil initialized when it refers to the loopCounter variable.
Similar to the scenario of the input element variable, if the output element expressions refers to the `loopCounter` variable, then we shouldn't NIL initialize it.
Test Results 799 files ± 0 1 errors 798 suites ±0 1h 38m 56s ⏱️ + 6m 36s For more details on these parsing errors, see this check. Results for commit 64a4309. ± Comparison against base commit d678d46. ♻️ This comment has been updated with latest results. |
korthout
changed the title
| @@ -894,6 +894,57 @@ public void shouldSetOutputElementVariable() { | |||
| OUTPUT_COLLECTION.stream().map(JsonUtil::toJson).collect(Collectors.toList())); | |||
| } | |||
|
|
|||
| @Test | |||
| public void shouldNotInitializeOutputElementVariableIfSameNameAsInputElement() { | |||
There was a problem hiding this comment.
🔧 Could this problem also occur for an output collection? If so please also add a testcase for this.
There was a problem hiding this comment.
Good question.
This was somewhat discussed in another issue. To summarize, the input collection variable exists at a higher scope, the output collection variable is created at the multi-instance element's scope. So the output collection variable can shadow the input collection variable.
What that analysis doesn't discuss is the case combined with an input mapping to set the input collection variable at the multi-instance element scope. However, that shouldn't work because the input mappings are only applied for each inner activity, not for the entire multi-instance element. As far as I know, we consider it not a valid use case at this time.
|
bors merge |
|
Build succeeded: |
zeebe-bors-camunda
bot
deleted the
korthout-4687-same-input-output-variable-name
branch
|
Successfully created backport PR #9957 for |
|
Successfully created backport PR #9958 for |
saig0
added
release/8.0.8
version:1.3.14
Description
This PR makes it possible to refer to the same variable in the multi-instance
inputElementand theoutputElementexpression.Technically, this PR makes sure that the 'NIL initialization' (explained below) of the
outputElementvariable doesn't overwrite:inputElementvariableloopCountervariableWhen the
outputElementexpression (of a multi-instance element) refers directly to a variable (or the property of a variable), we NIL initialize the referenced variable. This is to make sure theoutputElementexists on the local scope of the inner instance of a multi-instance body. However, when theoutputElementexpression refers to the same variable as theinputElement, then we would overwrite the locally scopedinputElementvariable with this NIL value. Effectively erasing theinputElementvariable's value.I wondered whether we could also overwrite other locally scoped variables. However, there aren't many local variables that can exist on the inner instance of a multi-instance body (see below). User-defined variables (e.g. input mappings) cannot reach this scope during the activation of the inner instance. So apart from the
inputElementvariables, only theloopCountervariable can beNILinitialized by theoutputElementexpression.inputElementvariableoutputElementexpression that refers to variable (or property of a variable)loopCountervariableRelated issues
closes #4687
Definition of Done
Not all items need to be done depending on the issue and the pull request.
Code changes:
backport stable/1.3) to the PR, in case that fails you need to create backports manually.Testing:
Documentation:
Please refer to our review guidelines.