Expression for Context's Get entry/path with optional entries.
pme123 opened this issue · 14 comments
Is your feature request related to a problem? Please describe.
In contexts we have often the two cases a path / entry exists or it doesn't. With the construct (if then else) we have now, it is hard to work with.
Example:
{
a: 1,
b: {
c: {
d: 2
}
}
}
If b, c, and d is required the path is easy: b.c.d
But if these values are optional there is no simple way to get d if it exists.
Describe the solution you'd like
For Scripting like in Groovy, you can do b?.c?.d, which will return null if one of these entries are missing.
Supporting this would help a lot working with data structures in Camunda 8 processes.
@pme123 thank you for raising this up. 👍
As a workaround, you could write the following expression:
if b.c.d != null then b.c.d else null
However, the null check is not very elegant.
The proposal by using a new operator ? may conflict with the DMN spec because b? could be a name. Maybe, we could introduce a new built-in function instead.
if b.c.d != null then b.c.d else null works - also if c does not exist! I haven't expected that:).
Maybe, we could introduce a new built-in function instead.
That would work as well. Maybe also more readable for a non-programmer;). We could extend the existing one get value(), for Example:
get value({b: {}}, "b?.c?.d") // -> null
We could already use the get value() function in the following way:
get value(b.c, "d")
// returns either the value of D if B, C and D exists, or null
This is great.
So if I have only required fields I just use the path, like b.c.d, and if some of the path is optional I use get value(b.c, "d").
@saig0 I checked the code and added some Test cases.
So what you propose is working already with the following warning:
WARN org.camunda.feel.FeelEngine - Failed to invoke function: context contains no entry with key 'c'
The function would IMHO look a bit nicer if we only had to have the path as parameter. So I tried that and this worked exactly the same with the above warning.
So what you think - shall I make a pull request for: get value(b.c.d)?
And maybe we need to change the Warning to an Info.
I thought about the issue and different solutions. Currently, my favorite solution would be:
Modify the get value() function and introduce a new parameter list.
In addition to get value(context, key), we would add the parameter list get value(context, keys). The parameter keys must be a list of strings and is interpreted as the path.
get value({x: 1}, ["x"])
// 1
get value({x: {y: 2}}, ["x", "y"])
// 2
get value({}, ["x"])
// null
get value({}, ["x", "y"])
// null
If one of the keys is not part of the context then the function returns null and it doesn't print any warning.
The context put function has also a parameter keys that works in a similar way.
@pme123 would this solution fit your case?
Hi @saig0
It sure would work, but it does not look really nice - I could live with it though;).
What do you think is the problem with get value(b.c.d)?
What do you think is the problem with get value(b.c.d)?
The semantics. The get value() function is used to extract a key from a given context.
The function get value(b.c.d) would be static context access with the intention to ignore non-existing context entries and return null instead. So, a different intention.
Alternatively, we could introduce a new function for this case. For example, optional(b.c.d) or get or null(b.c.d).
Additional context for the get value(context, keys) function: camunda/camunda#11293
Ok, in this case my favourite would be
optional(b.c.d) or optional value(b.c.d)
Came up in the context of "null" safety discussion internally.
Up for clarification: What is the intention of the DMN spec.
// strict handling
{
a,
c
}.b // BOOM "b is not a member"
// lax handling
// non existing member access coherses to <null>
{
a,
c
}.b = null
Update: the get value(context, keys) function is available since version 1.16.0.
Based on the discussions (#582, #572), we came to the conclusion that the FEEL engine should handle non-existing variables (and properties) gracefully and return null. Together with this change, we want to improve error handling (#260) and introduce a way to express required variables (and properties).
There is an open issue for clarifying the behavior in the DMN-TCK: dmn-tck/tck#537.
The expected behavior was clarified by merging the new TCK test case.