Add validation check for BPMN event variable listener with empty vari…

…able name (flowable#3908)
filiphr · Jun 11, 2024 · 473fc25 · 473fc25
1 parent 727841e
commit 473fc25
Show file tree

Hide file tree

Showing 7 changed files with 211 additions and 1 deletion.
diff --git a/...rc/main/java/org/flowable/bpmn/converter/child/VariableListenerEventDefinitionParser.java b/...rc/main/java/org/flowable/bpmn/converter/child/VariableListenerEventDefinitionParser.java
@@ -43,7 +43,7 @@ public void parseChildElement(XMLStreamReader xtr, BaseElement parentElement, Bp
 
         String variableName = xtr.getAttributeValue(null, ATTRIBUTE_VARIABLE_NAME);
         if (StringUtils.isEmpty(variableName)) {
-            throw new FlowableException("variable name is required for variable listener with activity id " + parentElement.getId());
+            LOGGER.warn("variable name is required for variable listener with activity id {}", parentElement.getId());
         }
         eventDefinition.setVariableName(variableName);
 

diff --git a/...test/java/org/flowable/editor/language/xml/VariableListenerEventDefinitionParserTest.java b/...test/java/org/flowable/editor/language/xml/VariableListenerEventDefinitionParserTest.java
@@ -0,0 +1,39 @@
+/* Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.flowable.editor.language.xml;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.flowable.bpmn.constants.BpmnXMLConstants.ELEMENT_EVENT_VARIABLELISTENERDEFINITION;
+
+import org.flowable.bpmn.model.BpmnModel;
+import org.flowable.bpmn.model.EventSubProcess;
+import org.flowable.bpmn.model.FlowElement;
+import org.flowable.bpmn.model.StartEvent;
+import org.flowable.editor.language.xml.util.BpmnXmlConverterTest;
+
+class VariableListenerEventDefinitionParserTest {
+
+    @BpmnXmlConverterTest("missingVariableListenerVariableName.bpmn")
+    void validateModel(BpmnModel model) {
+
+        FlowElement flowElement = model.getMainProcess().getFlowElement("eventSubProcess");
+        assertThat(flowElement).isInstanceOf(EventSubProcess.class);
+        EventSubProcess subProcess = (EventSubProcess) flowElement;
+        assertThat(subProcess.getId()).isEqualTo("eventSubProcess");
+        flowElement = subProcess.getFlowElement("variableListenerStartEvent");
+        assertThat(flowElement).isInstanceOf(StartEvent.class);
+        StartEvent startEvent = (StartEvent) flowElement;
+        assertThat(startEvent.getId()).isEqualTo("variableListenerStartEvent");
+        assertThat(startEvent.getExtensionElements().get(ELEMENT_EVENT_VARIABLELISTENERDEFINITION)).isNull();
+    }
+}
diff --git a/modules/flowable-bpmn-converter/src/test/resources/missingVariableListenerVariableName.bpmn b/modules/flowable-bpmn-converter/src/test/resources/missingVariableListenerVariableName.bpmn
@@ -0,0 +1,72 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<definitions xmlns="http://www.omg.org/spec/BPMN/20100524/MODEL" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:flowable="http://flowable.org/bpmn" xmlns:bpmndi="http://www.omg.org/spec/BPMN/20100524/DI" xmlns:omgdc="http://www.omg.org/spec/DD/20100524/DC" xmlns:omgdi="http://www.omg.org/spec/DD/20100524/DI" xmlns:design="http://flowable.org/design" typeLanguage="http://www.w3.org/2001/XMLSchema" expressionLanguage="http://www.w3.org/1999/XPath" targetNamespace="http://flowable.org/test" design:palette="flowable-work-process-palette">
+  <process id="missingVariableListenerVariableName" name="missingVariableListenerVariableName" isExecutable="true" flowable:candidateStarterGroups="flowableUser">
+    <extensionElements>
+      <design:stencilid><![CDATA[BPMNDiagram]]></design:stencilid>
+      <design:creationdate><![CDATA[2024-06-11T06:08:01.488Z]]></design:creationdate>
+      <design:modificationdate><![CDATA[2024-06-11T06:11:42.890Z]]></design:modificationdate>
+    </extensionElements>
+    <subProcess id="eventSubProcess" name="Event-subprocess" triggeredByEvent="true">
+      <extensionElements>
+        <design:stencilid><![CDATA[EventSubprocess]]></design:stencilid>
+      </extensionElements>
+      <userTask id="subTask" name="subTask" flowable:assignee="${initiator}" flowable:formFieldValidation="false">
+        <extensionElements>
+          <flowable:task-candidates-type><![CDATA[all]]></flowable:task-candidates-type>
+          <design:stencilid><![CDATA[FormTask]]></design:stencilid>
+          <design:stencilsuperid><![CDATA[Task]]></design:stencilsuperid>
+        </extensionElements>
+      </userTask>
+      <startEvent id="variableListenerStartEvent" isInterrupting="false">
+        <extensionElements>
+          <flowable:variableListenerEventDefinition variableChangeType="all"></flowable:variableListenerEventDefinition>
+          <flowable:work-form-field-validation><![CDATA[false]]></flowable:work-form-field-validation>
+          <design:stencilid><![CDATA[StartVariableListenerEvent]]></design:stencilid>
+        </extensionElements>
+      </startEvent>
+      <endEvent id="subEndEvent">
+        <extensionElements>
+          <design:stencilid><![CDATA[EndNoneEvent]]></design:stencilid>
+        </extensionElements>
+      </endEvent>
+      <sequenceFlow id="bpmnSequenceFlow_6" sourceRef="subTask" targetRef="subEndEvent">
+        <extensionElements>
+          <design:stencilid><![CDATA[SequenceFlow]]></design:stencilid>
+        </extensionElements>
+      </sequenceFlow>
+      <sequenceFlow id="bpmnSequenceFlow_4" sourceRef="variableListenerStartEvent" targetRef="subTask">
+        <extensionElements>
+          <design:stencilid><![CDATA[SequenceFlow]]></design:stencilid>
+        </extensionElements>
+      </sequenceFlow>
+    </subProcess>
+    <userTask id="mainTask" name="mainTask" flowable:assignee="${initiator}" flowable:formFieldValidation="false">
+      <extensionElements>
+        <flowable:task-candidates-type><![CDATA[all]]></flowable:task-candidates-type>
+        <design:stencilid><![CDATA[FormTask]]></design:stencilid>
+        <design:stencilsuperid><![CDATA[Task]]></design:stencilsuperid>
+      </extensionElements>
+    </userTask>
+    <startEvent id="mainStartEvent" flowable:initiator="initiator" flowable:formFieldValidation="false">
+      <extensionElements>
+        <flowable:work-form-field-validation><![CDATA[false]]></flowable:work-form-field-validation>
+        <design:stencilid><![CDATA[StartNoneEvent]]></design:stencilid>
+      </extensionElements>
+    </startEvent>
+    <endEvent id="mainEndEvent">
+      <extensionElements>
+        <design:stencilid><![CDATA[EndNoneEvent]]></design:stencilid>
+      </extensionElements>
+    </endEvent>
+    <sequenceFlow id="bpmnSequenceFlow_10" sourceRef="mainTask" targetRef="mainEndEvent">
+      <extensionElements>
+        <design:stencilid><![CDATA[SequenceFlow]]></design:stencilid>
+      </extensionElements>
+    </sequenceFlow>
+    <sequenceFlow id="bpmnSequenceFlow_8" sourceRef="mainStartEvent" targetRef="mainTask">
+      <extensionElements>
+        <design:stencilid><![CDATA[SequenceFlow]]></design:stencilid>
+      </extensionElements>
+    </sequenceFlow>
+  </process>
+</definitions>
diff --git a/...-engine/src/test/java/org/flowable/standalone/validation/DefaultProcessValidatorTest.java b/...-engine/src/test/java/org/flowable/standalone/validation/DefaultProcessValidatorTest.java
@@ -422,6 +422,26 @@ void testMailTask() {
                         tuple(Problems.MAIL_TASK_NO_RECIPIENT, "No recipient is defined on the mail activity", "sendMailWithoutanything", false)
                 );
     }
+
+    @Test
+    public void testEventSubProcessWithoutVariableName() throws Exception {
+
+        InputStream xmlStream = this.getClass().getClassLoader()
+                .getResourceAsStream("org/flowable/engine/test/validation/missingVariableEventListenerVariableName.xml");
+        XMLInputFactory xif = XMLInputFactory.newInstance();
+        InputStreamReader in = new InputStreamReader(xmlStream, StandardCharsets.UTF_8);
+        XMLStreamReader xtr = xif.createXMLStreamReader(in);
+        BpmnModel bpmnModel = new BpmnXMLConverter().convertToBpmnModel(xtr);
+        assertThat(bpmnModel).isNotNull();
+
+        List<ValidationError> allErrors = processValidator.validate(bpmnModel);
+        assertThat(allErrors).hasSize(1);
+        ValidationError error = allErrors.get(0);
+        assertThat(error.getProblem()).isEqualTo(Problems.EVENT_SUBPROCESS_INVALID_START_EVENT_VARIABLE_NAME);
+        assertThat(error.getDefaultDescription()).isEqualTo("variable name is required for variable listener with activity id variableListenerStartEvent");
+
+    }
+
 
     protected void assertCommonProblemFieldForActivity(ValidationError error) {
         assertProcessElementError(error);

diff --git a/...esources/org/flowable/engine/test/validation/missingVariableEventListenerVariableName.xml b/...esources/org/flowable/engine/test/validation/missingVariableEventListenerVariableName.xml
@@ -0,0 +1,71 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<definitions xmlns="http://www.omg.org/spec/BPMN/20100524/MODEL" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:flowable="http://flowable.org/bpmn" xmlns:bpmndi="http://www.omg.org/spec/BPMN/20100524/DI" xmlns:omgdc="http://www.omg.org/spec/DD/20100524/DC" xmlns:omgdi="http://www.omg.org/spec/DD/20100524/DI" xmlns:design="http://flowable.org/design" typeLanguage="http://www.w3.org/2001/XMLSchema" expressionLanguage="http://www.w3.org/1999/XPath" targetNamespace="http://flowable.org/test" design:palette="flowable-work-process-palette">
+  <process id="missingVariableListenerVariableName" name="missingVariableListenerVariableName" isExecutable="true" flowable:candidateStarterGroups="flowableUser">
+    <extensionElements>
+      <design:stencilid><![CDATA[BPMNDiagram]]></design:stencilid>
+      <design:creationdate><![CDATA[2024-06-11T06:08:01.488Z]]></design:creationdate>
+      <design:modificationdate><![CDATA[2024-06-11T06:11:42.890Z]]></design:modificationdate>
+    </extensionElements>
+    <subProcess id="eventSubProcess" name="Event-subprocess" triggeredByEvent="true">
+      <extensionElements>
+        <design:stencilid><![CDATA[EventSubprocess]]></design:stencilid>
+      </extensionElements>
+      <userTask id="subTask" name="subTask" flowable:assignee="${initiator}" flowable:formFieldValidation="false">
+        <extensionElements>
+          <flowable:task-candidates-type><![CDATA[all]]></flowable:task-candidates-type>
+          <design:stencilid><![CDATA[FormTask]]></design:stencilid>
+          <design:stencilsuperid><![CDATA[Task]]></design:stencilsuperid>
+        </extensionElements>
+      </userTask>
+      <startEvent id="variableListenerStartEvent" isInterrupting="false">
+        <extensionElements>
+          <flowable:variableListenerEventDefinition variableChangeType="all"></flowable:variableListenerEventDefinition>
+          <flowable:work-form-field-validation><![CDATA[false]]></flowable:work-form-field-validation>
+        </extensionElements>
+      </startEvent>
+      <endEvent id="subEndEvent">
+        <extensionElements>
+          <design:stencilid><![CDATA[EndNoneEvent]]></design:stencilid>
+        </extensionElements>
+      </endEvent>
+      <sequenceFlow id="bpmnSequenceFlow_6" sourceRef="subTask" targetRef="subEndEvent">
+        <extensionElements>
+          <design:stencilid><![CDATA[SequenceFlow]]></design:stencilid>
+        </extensionElements>
+      </sequenceFlow>
+      <sequenceFlow id="bpmnSequenceFlow_4" sourceRef="variableListenerStartEvent" targetRef="subTask">
+        <extensionElements>
+          <design:stencilid><![CDATA[SequenceFlow]]></design:stencilid>
+        </extensionElements>
+      </sequenceFlow>
+    </subProcess>
+    <userTask id="mainTask" name="mainTask" flowable:assignee="${initiator}" flowable:formFieldValidation="false">
+      <extensionElements>
+        <flowable:task-candidates-type><![CDATA[all]]></flowable:task-candidates-type>
+        <design:stencilid><![CDATA[FormTask]]></design:stencilid>
+        <design:stencilsuperid><![CDATA[Task]]></design:stencilsuperid>
+      </extensionElements>
+    </userTask>
+    <startEvent id="mainStartEvent" flowable:initiator="initiator" flowable:formFieldValidation="false">
+      <extensionElements>
+        <flowable:work-form-field-validation><![CDATA[false]]></flowable:work-form-field-validation>
+        <design:stencilid><![CDATA[StartNoneEvent]]></design:stencilid>
+      </extensionElements>
+    </startEvent>
+    <endEvent id="mainEndEvent">
+      <extensionElements>
+        <design:stencilid><![CDATA[EndNoneEvent]]></design:stencilid>
+      </extensionElements>
+    </endEvent>
+    <sequenceFlow id="bpmnSequenceFlow_10" sourceRef="mainTask" targetRef="mainEndEvent">
+      <extensionElements>
+        <design:stencilid><![CDATA[SequenceFlow]]></design:stencilid>
+      </extensionElements>
+    </sequenceFlow>
+    <sequenceFlow id="bpmnSequenceFlow_8" sourceRef="mainStartEvent" targetRef="mainTask">
+      <extensionElements>
+        <design:stencilid><![CDATA[SequenceFlow]]></design:stencilid>
+      </extensionElements>
+    </sequenceFlow>
+  </process>
+</definitions>
diff --git a/...flowable-process-validation/src/main/java/org/flowable/validation/validator/Problems.java b/...flowable-process-validation/src/main/java/org/flowable/validation/validator/Problems.java
@@ -90,6 +90,7 @@ public interface Problems {
     String SUBPROCESS_START_EVENT_EVENT_DEFINITION_NOT_ALLOWED = "flowable-subprocess-start-event-event-definition-not-allowed";
 
     String EVENT_SUBPROCESS_INVALID_START_EVENT_DEFINITION = "flowable-event-subprocess-invalid-start-event-definition";
+    String EVENT_SUBPROCESS_INVALID_START_EVENT_VARIABLE_NAME = "flowable-event-subprocess-invalid-start-event-variable-name";
     String EVENT_SUBPROCESS_BOUNDARY_EVENT = "flowable-event-subprocess-boundary-event";
 
     String BOUNDARY_EVENT_NO_EVENT_DEFINITION = "flowable-boundary-event-no-event-definition";

diff --git a/...dation/src/main/java/org/flowable/validation/validator/impl/EventSubprocessValidator.java b/...dation/src/main/java/org/flowable/validation/validator/impl/EventSubprocessValidator.java
@@ -14,6 +14,7 @@
 
 import java.util.List;
 
+import org.apache.commons.lang3.StringUtils;
 import org.flowable.bpmn.model.BoundaryEvent;
 import org.flowable.bpmn.model.BpmnModel;
 import org.flowable.bpmn.model.ConditionalEventDefinition;
@@ -56,6 +57,12 @@ protected void executeValidation(BpmnModel bpmnModel, Process process, List<Vali
                         addError(errors, Problems.EVENT_SUBPROCESS_INVALID_START_EVENT_DEFINITION, process, eventSubprocess, eventDefinition,
                                 "start event of event subprocess must be of type 'error', 'timer', 'message' or 'signal'");
                     }
+
+                    if (eventDefinition instanceof VariableListenerEventDefinition variableListenerEventDefinition
+                            && StringUtils.isEmpty(variableListenerEventDefinition.getVariableName())) {
+                        addError(errors, Problems.EVENT_SUBPROCESS_INVALID_START_EVENT_VARIABLE_NAME,
+                                process, startEvent, "variable name is required for variable listener with activity id " + startEvent.getId());
+                    }
                 }
             }