camunda / camunda-bpm-platform

Flexible framework for workflow and decision automation with BPMN and DMN. Integration with Quarkus, Spring, Spring Boot, CDI.
https://camunda.com/
Apache License 2.0
4.11k stars 1.55k forks source link

On message correlation, I can set local variables in the triggered execution #2266

Closed ThorbenLindhauer closed 10 months ago

ThorbenLindhauer commented 5 years ago

This issue was imported from JIRA:

Field Value
JIRA Link CAM-10680
Reporter @ThorbenLindhauer
Has restricted visibility comments false

There is an additional correlation parameter that takes variables with the following semantics:

Intermediate message event/receive task: Set the variables on the execution that is in the intermediate event (like setVariablesLocal) Event sub process: Set the variables on the execution that represents the scope instance of the event sub process (unlike setVariablesLocal)

User Problem

User Story (Required on creation):

As a developer, I want to be able to define in the BPMN, how a certain error should be handled based on a variable.

Functional Requirements (Required before implementation):

Technical Requirements (Required before implementation):

Limitations of Scope (Optional):

Breakdown

Hints

Execution Tree Examples

example-1.pdf example-2.pdf example-3.pdf example-4.pdf

Links:

### Pull requests
- [ ] https://github.com/camunda/camunda-bpm-platform/pull/3973
- [ ] https://github.com/camunda/camunda-bpm-platform-maintenance/pull/1117
yanavasileva commented 11 months ago

Message start event

  1. Don't do anything (don't add the variablesToTriggeredScope at all)
    • Pros
    • Cons
    • not user friendly, user will expect to see their variables processed/handled
    • there's a new scope for the start event when the process instance is created
  2. Set variables to the new process instance similarly to setVariablesLocal
    • Pros
    • consistency with the other variables
    • semantically correct behaviour

Message intermediate catching event

  1. Don't do anything (don't add the variablesToTriggeredScope at all)
    • Pros
    • theoretically there's no new scope for intermediate catch event so we can say the parameter in not applicable in this case
    • Cons
    • not user friendly, user will expect to see their variables processed/handled
  2. Treat the variables as local
    • Pros
    • semantically correct action (for intermediate catch event we just fall back to existing scope when there isn't a new one)
    • setVariablesLocal is semantically the same in this use case

Decision: For both use cases second option with arguments considering user expectations and existing implementation. Also, reached out to the customer to get their feedback. So far we are not aware if they are using those two events and if they have intentions to use the new parameter with them.

yanavasileva commented 11 months ago

To consider: add a test case with async continuation.

yanavasileva commented 10 months ago

Notes:

Therefore, I adjusted setting the variables at earlier point in PVM instead of PvmAtomicOperationActivityStart:

yanavasileva commented 10 months ago

@danielkelemen, I covered async continuation scenario and implemented the review hints. Ready for review on master and 7.20

yanavasileva commented 10 months ago

Handover to QA:

gbetances089 commented 10 months ago

Tested on 7.20.2 patch