camunda / feel-scala

FEEL parser and interpreter written in Scala
https://camunda.github.io/feel-scala/
Apache License 2.0
120 stars 50 forks source link

Assert that a variable is not null #675

Closed saig0 closed 1 year ago

saig0 commented 1 year ago

Is your feature request related to a problem? Please describe. The engine's behavior for non-existing variables will change with https://github.com/camunda/feel-scala/issues/582 and https://github.com/camunda/feel-scala/issues/674. Instead of failing the evaluation, a non-existing variable will be replaced with null.

So, the evaluation will be null-friendly. If the engine can't evaluate the expression successfully, it will return null.

This behavior is aligned with the DMN spec. However, there could be cases that require a verification that a variable exists or is not null.

A special case is the migration of existing expression to the new behavior. For example, if a process uses an expression and assumes that the expression fails if the variable doesn't exist (e.g. on an output variable mapping).

Describe the solution you'd like Add a new built-in function to verify that a variable or value is not null.

assert not null(value: x)
// if x is null, the evaluation fails
// if x is not null, the function returns x

assert not null(value: x, cause: "x is null")
// with an optional failure message

Or, a more general built-in function to verify that a condition is met.

assert(value: x, condition: x != null)
// if the condition is false or null, the evaluation fails
// if the condition is true, the function returns the value 

assert(value: x, condition: x != null, cause: "x is null")
// with an optional failure message

Related issues

saig0 commented 1 year ago

@camunda/zeebe-process-automation please have a look at this proposal and share your thoughts.

cc: @aleksander-dytko

remcowesterhoud commented 1 year ago

So If I understand correct, the assert will fail an evaluation exceptionally when the condition is not met? I'd consider that a valuable function to have.

My preference would go to the second option. That way it's not limited to just null. The hard part of the second solution is, what do you do if the condition evaluates to null 😄 Nevermind you wrote that down already.

aleksander-dytko commented 1 year ago

@saig0 how would you see the priority of this? Is this something that we can prioritize after 8.3 release?

saig0 commented 1 year ago

Is this something that we can prioritize after 8.3 release?

No. This should be part of the 8.3 release. If a user relies on the current behavior (i.e. raise an incident if a variable is not present), this new function would be the way to have a similar behavior in version 8.3.

In my PoV, it is part of the migration path to the new version. Otherwise, we change the behavior in version 8.3 and the users have no chance to keep the old behavior.

The implementation effort for adding this new function is low. I assume that it should not take more than a day for a dev to implement it.

aleksander-dytko commented 1 year ago

@saig0 thank you for the explanation!

I'll add this to the ZPA board to prioritize before 8.3

CC: @abbasadel

berkaycanbc commented 1 year ago

Moving this to Backlog as the size is assigned and it is upcoming.