it should not be possible to create Messages with invalid payload

[ultraschuppi]

Describe the Bug

<bpmn:message> entries are not deletable
OR
it is possible to create invalid <bpmn:messages>

Steps to Reproduce

1. drag a task in a new model
2. create an end event after the newly created task
3. change task to "Receive Task"
4. in Details add new message
5. provide bad value for subscription correlation key (here: wurst)
6. delete task again

<?xml version="1.0" encoding="UTF-8"?>
<bpmn:definitions xmlns:bpmn="http://www.omg.org/spec/BPMN/20100524/MODEL" xmlns:bpmndi="http://www.omg.org/spec/BPMN/20100524/DI" xmlns:dc="http://www.omg.org/spec/DD/20100524/DC" xmlns:zeebe="http://camunda.org/schema/zeebe/1.0" xmlns:di="http://www.omg.org/spec/DD/20100524/DI" id="Definitions_0mnzkdb" targetNamespace="http://bpmn.io/schema/bpmn" exporter="Zeebe Modeler" exporterVersion="0.10.0">
  <bpmn:process id="Process_0nzjv7w" isExecutable="true">
    <bpmn:startEvent id="StartEvent_1">
      <bpmn:outgoing>Flow_1mnj7jr</bpmn:outgoing>
    </bpmn:startEvent>
    <bpmn:sequenceFlow id="Flow_1mnj7jr" sourceRef="StartEvent_1" targetRef="Event_18hf282" />
    <bpmn:endEvent id="Event_18hf282">
      <bpmn:incoming>Flow_1mnj7jr</bpmn:incoming>
    </bpmn:endEvent>
  </bpmn:process>
  <bpmn:message id="Message_1xwvlqo" name="Message_3rds04h">
    <bpmn:extensionElements>
      <zeebe:subscription correlationKey="wurst" />
    </bpmn:extensionElements>
  </bpmn:message>
  <bpmndi:BPMNDiagram id="BPMNDiagram_1">
    <bpmndi:BPMNPlane id="BPMNPlane_1" bpmnElement="Process_0nzjv7w">
      <bpmndi:BPMNEdge id="Flow_1mnj7jr_di" bpmnElement="Flow_1mnj7jr">
        <di:waypoint x="215" y="117" />
        <di:waypoint x="432" y="117" />
      </bpmndi:BPMNEdge>
      <bpmndi:BPMNShape id="_BPMNShape_StartEvent_2" bpmnElement="StartEvent_1">
        <dc:Bounds x="179" y="99" width="36" height="36" />
      </bpmndi:BPMNShape>
      <bpmndi:BPMNShape id="Event_18hf282_di" bpmnElement="Event_18hf282">
        <dc:Bounds x="432" y="99" width="36" height="36" />
      </bpmndi:BPMNShape>
    </bpmndi:BPMNPlane>
  </bpmndi:BPMNDiagram>
</bpmn:definitions>

despite my model is not using the created message, it is still part of the model and therefor not deployable

Command 'CREATE' rejected with code 'INVALID_ARGUMENT': Expected to deploy new resources, but encountered the following errors:
'diagram_3.bpmn': - Element: Message_1xwvlqo > extensionElements > subscription
    - ERROR: Expected expression but found static value 'wurst'. An expression must start with '=' (e.g. '=wurst').
 [ deploy-error ]

Expected Behavior

it should not be possible to create messages with invalid payload messing my model w/o even using it

Environment

• OS: Linux
• Zeebe Modeler Version: 0.10.0

[MaxTru]

Solution I would propose:

• Add validation for expression for the "Subscription Correlation Key" (ie. validate when the string does not start with =)
• Prefill the subscription correlation key with some value, that would not lead to an error in the Zeebe engine

=> This way we can ensure that there will always be a Feel expression like value in the Subscription Correlation Key field. When a user enters a value without a starting = that value would not be committed / save (due to validation error).

Will need to check with Zeebe team what a appropriate default value would be.

[menski]

Prefill the subscription correlation key with some value, that would not lead to an error in the Zeebe engine

I don't think we should fill a correlation key randomly, there is no sane default as this depends on the variables the user adds to the process.

IMHO I think we should validate that field and make sure that the process model does not contain any unreferenced message elements

/cc @saig0 can you share you opinion please 🍪

[MaxTru]

The solution I outlined in #272 (comment) would be a "quick fix".

If there is no sane default then we would need IMO the following solution:

• Validate the field for valid FEEL (easy to do).
• If field is not valid, ask the user to delete the message

Note that the "Delete the message" functionality is not present in the Properties Panel (ie. see camunda/camunda-modeler#136). Therefore we would be blocked for this issue as of now. But note that there is activity to add this "deletion" functionality (see camunda/camunda-modeler#1930)

[pinussilvestrus]

We can validate the field to be a FEEL expression (starting with =) or go on improving the overall FEEL support: #253.

Also, I have a feeling we mixing some things up inside this issue
a) creating valid FEEL expressions for the correlation key
b) messages can't be deleted / won't be deleted after task is gone

We should keep this issue focus on the first one and create another one for the second issue.

[saig0]

I'm okay with all the suggestions. We should have validation for the field. We can prefill it, for example with = key, or leave it blank. Until we have a dedicated solution for this kind of field, I like the idea of prefilling it. So, the user sees how it should look like. But I'm not a UX expert :)

[menski]

My concern with prefilling it with a valid expression like = key is that it can be deployed by the user and then will lead to an incident on runtime, without the user understanding what happend I assume.

[nikku]

Let's not pre-fill it but go for validation within the panel instead. ➡️ it should be visible immediately to the user that something is missing as she creates the message.

Mid-term we'll likely add linting and deployment feedback to make these issues even more visible.

[MaxTru]

Okay, so concluding from what we discussed. We can do the following things:

1. We will adjust the validation for the "Subscription Correlation Key" field to (at least rudimentary) ensure that a FEEL expression is entered by checking for leading = followed by a charcter-sequence. (More sophisticated FEEL parsing could be a second step). [This can be done rather easily] (Validate subscription correlation key for FEEL expression #275)
2. In the mid-term / long-term we will add a feature to allow that messages once created can be deleted again (also see BPMN Errors can be viewed (summary) and deleted individually camunda/camunda-modeler#1982 or Graphical removal of intermediate events doesn't remove the referenced definitions camunda/camunda-modeler#1930). This will address @menski point to "make sure that the process model does not contain any unreferenced message elements" (Allow global overview and deletion of <bpmn:message> elements #276)
3. Mid-term / long-term linting or deployment feedback is to be added to make errors even more apparent.

=> I created separate issue for to make the progress and next steps clearer. I will close this issue. Please feel free to re-open in case you feel something is missing / got lost.