search

viadee / vPAV

viadee Process Application Validator
https://www.viadee.de/java/process-application-validator
BSD 3-Clause "New" or "Revised" License
48 stars 14 forks source link

ProcessVariablesModelChecker unable to handle loop within process #212

Open MLukman opened 3 years ago

MLukman commented 3 years ago

Describe the bug ProcessVariablesModelChecker unable to handle loop within process

To Reproduce Steps to reproduce the behavior:

  1. Add the attached BPMN file to a vPAV-enabled project
  2. Build the project.
  3. The project will fail vPAV test. Open the test result to see that ProcessVariablesModelChecker complains about "There is a read access to variable 'xxxx' in activity 'xxxx' but no value has been set before. (Compare model: 'Details','Expression')"
  4. Open the BPMN file in a BPMN editor (e.g. Camunda Modeler)
  5. Delete the link with name = "DELETE THIS LINK AND RE-RUN VPAV"
  6. Re-build the project.
  7. The project will now pass the vPAV test.

Expected behavior ProcessVariablesModelChecker should not complain since the variables are set inside "Initialize variables" task. It seems the loop stumps ProcessVariablesModelChecker.

Screenshots ProcessVariablesModelChecker complains image But the variables have been set image

Desktop (please complete the following information):

Attachments cyclic-process.zip

sdibernardo commented 3 years ago

Hi @MLukman,

thanks for reporting this issue. I'll try to reproduce the bug given your description and get back to you ASAP. May I ask in what kind of context you´re using vPAV?

Cheers, Sascha

MLukman commented 3 years ago

Hi @MLukman,

thanks for reporting this issue. I'll try to reproduce the bug given your description and get back to you ASAP. May I ask in what kind of context you´re using vPAV?

Cheers, Sascha

I'm using vPAV inside a Spring Boot-based Camunda BPM engine with embedded BPMN process file. Since the BPMN file will be part of the application, I'm using vPAV to validate if the BPMN file is well-formed before building the application Jar file.

sdibernardo commented 3 years ago

Hi @MLukman

after reviewing the submitted issue, I have come to the conclusion, that the analysis works correctly. As this might sound odd at first, I'll try to provide informaton regarding the current implementation and the reasoning behind it.

vPAV uses a variant of the _Reaching _Definitions__ algorithm. This algorithm in its earlier form used the union operator to propagate variable usage. In your example, the first Service Task would receive all variable usages from its predecessors, namely the sequence flows. There were a few cases where this would lead to variable access without prior definitions, thus leading to errors and exceptions (false negatives). Changing the union operator to the intersection allows to ensure that a variable is present on all preceeding paths (this is called conservatism in data flow analysis). This in turn might result in false positives, but we decided that those cases are way better than unfound errors. In any case, the analysis is only an approximation and has its boundaries, especially with regards to loops.

In your case, as can be verified manually, you could use the ignore list to disregard the false positives. Regarding the future: Would you be interested to configure the analysis in order to use either the union or intersection operator?

Hope this clears things up and I'm happy to take feedback on the analysis Sascha

MLukman commented 3 years ago

@sdibernardo,

Thank you for taking time to address this issue submission. I duly noted on the explanation on union vs intersection operators and its impact on false negatives vs false positives.

Regarding the future: Would you be interested to configure the analysis in order to use either the union or intersection operator?

Yes, I would like to request such flexibility by perhaps configuring a flag in ruleSet.xml. Since for my case, it would be difficult to keep modifying ignore list to disregard false positives especially when using Git and CI/CD automation, thus I will disable ProcessVariablesModelChecker for my application until such a configuration option exist. Thank you again for your kind consideration.

Lukman

sdibernardo commented 3 years ago

I'll open another issue with this request and see if I can include it in the next release. Until then disabling the analysis or ignoring the results might do the trick.

Feel free to report further issues or any kind of feedback! Sascha