is defined() always returns true

[saig0]

Describe the bug
The FEEL engine contains a function is defined(). It can be used to check if a variable or context entry/property exists.

The behavior of this function changed since version 1.17.0.

// === before 1.17.0 ===
is defined("a value")
-> true

is defined(null) 
-> true

is defined(non_existing_variable) 
-> false 

is defined({}.non_existing_key) 
-> false

// === since 1.17.0 ===
is defined("a value")
-> true

is defined(null) 
-> true

is defined(non_existing_variable) 
-> true 

is defined({}.non_existing_key) 
-> true

To Reproduce
Steps to reproduce the behavior:

1. Evaluate the expression is defined(non_existing_variable)
2. Verify that the evaluation returns true

Expected behavior
The function is defined() behaves the same as in version 1.16.0

Environment

• FEEL engine version: 1.17.0 (not yet released - on main branch)
• Affects:
  • Camunda Automation Platform 7: [7.x]
  • Zeebe broker: [0.x]

[saig0]

Background

The engine's behavior changed with version 1.17.0 to be more null-friendly. As a result, non-existing variables and context entries result in null instead of failing the evaluation. More generally, the engine returns null for all expressions that can't be applied successfully (i.e. as intended).

Related issues:

• #582
• #582
• #670
• #260

[saig0]

Workaround

The is defined() function can't be used anymore to check if a context contains a given key. To mitigate the issue, we can use the get entries() function with one of the following expressions:

//Check if a context contains a key
"x" in get entries({x:1}).key
-> true

"x" in get entries({x:null}).key
-> true


"y" in get entries({x:1}).key
-> false

// Or using the contains function
list contains(get entries({x:1}).key, "y")
-> false

[saig0]

Idea

With the new null-friendly behavior in version 1.17.0, it is hard to restore the original behavior of the function.

Instead, we could add a new function to check if a context contains an entry with a given key.

// function signature
context contains(context: context, key: string): boolean

// examples
context contains({x:1}, "x")
-> true

context contains({x:null}, "x")
-> true

context contains({x:1}, "y")
-> false

context contains(null, "y")
-> false

This function could be used together with Zeebe's expression context to check if a variable exists.

context contains(camunda.variables, "x")

[saig0]

cc: @camunda/zeebe-process-automation @aleksander-dytko

[aleksander-dytko]

@saig0 could you maybe help us with planing by assigning a size to the issue? Thanks!

[korthout]

@aleksander-dytko Philipp is FTO at this time.

ZPA triage (on idea):

• size: medium (team is still unfamiliar with feel-scala codebase, but we think we can resolve this)
• however we think the regression would not be resolved by this idea
• we'll favor other feel-scala issues because we don't know how to resolve the regression
• marking as later

[saig0]

Idea 2

We can't restore the original behavior in version 1.17.0. However, we can mitigate the behavior by changing the semantics of the function slightly.

Instead of checking for a non-existing variable or context entry, we could check for null. Similar to a null-check x != null.

• If the variable or the context entry doesn't exist, the function would return false. Like in version 1.17.0. ✔️
• If the variable or the context entry exists but is null, the function would return false. Different to version 1.17.0. ❌

// === before 1.17.0 ===
is defined(null) 
-> true

is defined(non_existing_variable) 
-> false 

is defined({}.non_existing_key) 
-> false

// === since 1.17.0 ===
is defined(null) 
-> false

is defined(non_existing_variable) 
-> false 

is defined({}.non_existing_key) 
-> false

The regression would impact only users if a variable or context entry exists and is null.