Boolean input without feel property in a template throws an error

[barmac]

Describe the bug

Applying this template causes an error to be thrown:

{
  "$schema": "https://unpkg.com/@camunda/zeebe-element-templates-json-schema/resources/schema.json",
  "name": "Boolean input without feel",
  "id": "io.booleanWithoutFeel.test",
  "version": 1,
  "appliesTo": [
    "bpmn:Task"
  ],
  "elementType": {
    "value": "bpmn:ServiceTask"
  },
  "properties": [{
    "label" : "Analyze tables",
    "optional" : false,
    "value" : true,
    "constraints" : {
      "notEmpty" : true
    },
    "binding" : {
      "name" : "input.analyzeTables",
      "type" : "zeebe:input"
    },
    "type" : "Boolean"
  }, {
    "label" : "Dummy required for test failure",
    "value" : "3",
    "feel" : "optional",
    "binding" : {
      "name" : "retries",
      "type" : "zeebe:input"
    },
    "type" : "String"
  }]
}

Error:

[/tmp/test/test.bpmn] r.source.substring is not a function
    at file:///Users/maciej/.asdf/installs/camunda-modeler/5.28.0.app/Contents/Resources/app.asar/public/303.js:1:6429
    at file:///Users/maciej/.asdf/installs/camunda-modeler/5.28.0.app/Contents/Resources/app.asar/public/303.js:1:6482
    at Array.forEach (<anonymous>)
    at file:///Users/maciej/.asdf/installs/camunda-modeler/5.28.0.app/Contents/Resources/app.asar/public/303.js:1:6197
    at Array.forEach (<anonymous>)
    at forEach (webpack://camundanode_modules/@bpmn-io/variable-resolver/lib/zeebe/util/feelUtility.js:17:12)
    at parseIoMappings (webpack://camundanode_modules/@bpmn-io/variable-resolver/lib/zeebe/VariableResolver.js:111:27)
    at file:///Users/maciej/.asdf/installs/camunda-modeler/5.28.0.app/Contents/Resources/app.asar/public/427.js:5:98136
    at listener (webpack://camundanode_modules/diagram-js/lib/core/EventBus.js:457:33)
    at _invokeListener (webpack://camundanode_modules/diagram-js/lib/core/EventBus.js:431:23) [ error ]
This error may be the result of a plug-in compatibility issue. [ info ]
Disable plug-ins (restarts the app) [ info ]

Steps to reproduce

1. Download the template
2. Create an element from this
3. now this happens

Expected behavior

No error is thrown. Two options:

1. Template can be applied and it works.
2. Template is rejected by JSON schema.

Environment

• OS: [e.g. MacOS 10.2, Windows 10]
• Camunda Modeler Version: 5.28.0 and older
• Execution Platform: [e.g. Camunda 7, Camunda 8]
• Installed plug-ins: [...]

Additional context

Related to #4517, but it's a new issue.

SUPPORT-25273

[barmac]

This problem can be mitigated by adding feel field to the property.

[barmac]

I think we should just make "feel": "static" the default way of handling boolean inputs. We know that this needs to be an expression to apply boolean value.

[philippfromme]

I had a look with @marstamm. The root cause is that certain properties must be FEEL expressions from the engine's point of view (e.g., input and output source). The variable resolver is built on that assumption which results in an error when the source isn't a FEEL expression (and therefore not a string) but a boolean or number. The information what properties must be FEEL expressions is not part of the meta model but hard-coded in the properties panel. Unless you're using an element template you cannot set the source to a boolean or number which is probably why we haven't seen this issue pop up earlier. We could now hard-code this information in bpmn-js-element-templates, too. But this would mean there is no single source of truth. Ideally, this information should be read from the meta model using the meta field which is already used for allowedIn. This is what it would look like:

{
  "name": "InputOutputParameter",
  "properties": [
    {
      "name": "source",
      "isAttr": true,
      "type": "String"
    },
    {
      "name": "target",
      "isAttr": true,
      "type": "String"
    }
  ],
  "meta": {
    "feel": "required"
  }
}

Both bpmn-js-properties-panel and bpmn-js-element-templates would then read this information from the meta model. When adding a new property to the meta model the information that it must be a FEEL expression could be added without having to adjust the bpmn-js-properties-panel and bpmn-js-element-templates which could easily be forgotten.

@nikku @barmac Open for suggestions.

[barmac]

Sorry for a way too late feedback. So essentially you're proposing to shift the information on whether a property can/must be FEEL to the metamodel. At first sight, this sounds correct. We need to carefully consider whether it blocks any existing behaviors though. What if a property can be a FEEL expression in the engine, but an element template author wants it to be provided as plain text only? Is this a use case already?

[philippfromme]

What if a property can be a FEEL expression in the engine, but an element template author wants it to be provided as plain text only? Is this a use case already?

I did some digging and this is something that doesn't work at the moment. The "feel": "static" option doesn't work with a "type": "text". There's a test to verify this: https://github.com/camunda/element-templates-json-schema/blob/4216f8d28c64b95457935aeaed3c8ecdf618074b/packages/zeebe-element-templates-json-schema/test/fixtures/feel-type-mismatch.js#L9 But interestingly for other binding types there's no error at all but the template doesn't show up. Example:

{
  "$schema": "https://unpkg.com/@camunda/zeebe-element-templates-json-schema/resources/schema.json",
  "name": "FEEL static",
  "id": "feel.static",
  "appliesTo": [
    "bpmn:ServiceTask"
  ],
  "properties": [
    {
      "label": "Static",
      "type": "Text",
      "feel": "static",
      "binding": {
        "type": "zeebe:property",
        "name": "Property"
      }
    }
  ]
}

No error, but template doesn't show up:

Since the template defines what the UI looks like we could convert whenever we find that the meta model requires a FEEL expression. We currently do this only when dealing with boolean and number values (cf. https://github.com/bpmn-io/bpmn-js-element-templates/blob/main/src/cloud-element-templates/util/FeelUtil.js#L1).

[barmac]

After a second look, perhaps we could mitigate this with the validator? So instead of encoding the FEEL information in the meta-model, we could just outright reject the templates which don't contain the required feel property.

[philippfromme]

We now customers running into this issue: https://jira.camunda.com/browse/SUPPORT-25273?atlLinkOrigin=c2xhY2staW50ZWdyYXRpb258aXNzdWU%3D

[philippfromme]

After a second look, perhaps we could mitigate this with the validator? So instead of encoding the FEEL information in the meta-model, we could just outright reject the templates which don't contain the required feel property.

Yes, this would be the most straight forward solution I think. This would mean that the affected element template would need a fix.

[philippfromme]

This is the affected element template: https://github.com/camunda/connectors/blob/59eefdc4fe27048f1be1f52da2c764fe5211d32f/connectors/http/rest/element-templates/http-json-connector.json#L393

And this the property:

{
  "id" : "storeResponse",
  "label" : "Store response",
  "description" : "Store the response as a document in the document store",
  "optional" : false,
  "value" : false,
  "group" : "endpoint",
  "binding" : {
    "name" : "storeResponse",
    "type" : "zeebe:input"
  },
  "type" : "Boolean"
}

[philippfromme]

The issue with adding a new condition that requires zeebe:input and zeebe:output properties to have a feel property is that it is a breaking change. It breaks a lot of tests of what were previously valid properties: https://github.com/camunda/element-templates-json-schema/actions/runs/12866432072/job/35869122450

Not sure if this is a good idea. @barmac 👀

[barmac]

I've just verified with the engine that inputs and outputs have to be expressions regardless of the property type (text, number, boolean). If we pass a value which does not start with =, the deployment is rejected. This is actually a confirmation of your previous finding in #4593 (comment)

This means that even if we fix the top-comment bug and allow users to apply the template, they still won't be able to deploy the elements to the engine.

So the proper solution is to make sure that the property value is set to a FEEL expression. There are essentially two possible ways to proceed:

1. Make the template developer always choose which type of FEEL handling they want for their property. This is the way from Boolean input without feel property in a template throws an error #4593 (comment). I wouldn't worry much about breaking the existing tests as the tests are already using broken templates as we have established in the first paragraph.
2. Make the "feel": "static" the default value for any properties which require an expression. This is non-breaking, but requires additional work from the team. We have already used default values in element templates, e.g. https://github.com/camunda/element-templates-json-schema/blob/e217c49ba954dc4e6095b53c3be6c7e576d3e351/packages/element-templates-json-schema-shared/src/defs/condition.json#L13

The decision seems to be a trade-off between no magic and do the hard work to make it simple. However, if we frame the template developer expectation as plain UI when feel is unset, I see the default value way (2) as a no-brainer.

[philippfromme]

Turns out we attempted to fix this issue before: bpmn-io/bpmn-js-element-templates#121 The issue is that we added a casting mechanism but didn't enforce the "feel": "static" property that would make sure the casting works.

[philippfromme]

@barmac I was able to deploy a process with

<zeebe:ioMapping>
  <zeebe:input source="true" target="invalid" />
  <zeebe:input source="=true" target="valid" />
</zeebe:ioMapping>

and according to Operate the value is cast to a string. 🤔

[philippfromme]

Furthermore, the docs seem to be incorrect: https://docs.camunda.io/docs/components/modeler/desktop-modeler/element-templates/defining-templates/#feel-static

The value of feel: static is only valid for Boolean and Number fields.

should instead be _For Boolean and Number fields the value of feel must be static. 🤔

[barmac]

OK so it fails for output only:

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

[marstamm]

For Boolean and Number fields the value of feel must be static.

This implied that feel: optional is not allowed. Instead, it should be For Boolean and Number fields the value of feel must be defined.

[nikku]

This implied that feel: optional is not allowed. Instead, it should be For Boolean and Number fields the value of feel must be defined.

Yes, unless you template core moddle properties, such as bpmn:Process#isExecutable, which hosts the raw boolean property.

[philippfromme]

My understanding is that for zeebe:input and zeebe:output properties of type boolean and number we need to enforce "feel": "static" and cannot make it optional. Am I right?

[barmac]

Yes, for correct handling it has to always be serialized to expression: =true, =222.

[nikku]

My understanding is that for zeebe:input and zeebe:output properties of type boolean and number we need to enforce "feel": "static" and cannot make it optional. Am I right?

Yes.

---

Let's summarize what we found out:

• Output mapping source MUST BE an expression
• Input mapping source MAY BE an expression, or otherwise is of type string
• For input mappings we previously improved the run-time behavior to map from boolean or number representations of default value to = value (fix(cloud-templates): cast default number and boolean numbers to feel bpmn-io/bpmn-js-element-templates#121)

A proper working versions for Boolean and Number fields, in conjunction with zeebe:input / zeebe:output is therefore:

• Such properties must be feel: "static", they will always be FEEL expressions, or they'll otherwise not yield a number or boolean at run-time

For other properties (bpmn:Process#isExecutable), there exists the use-case of feel = none, when used in conjunction with moddle properties of type boolean or integer.