Issue with casting None (null type) parameters in the rules when invoking actions

[Kami]

This was originally reported by @jjm on Slack.

Right now there is an issue when you want to pass parameter which value is None (null) to an action via the rule.

Let's say we have the following trigger payload (JSON):

{"value": null}

And the following rule:

action:
    ref: "mypack.myaction"
    parameters:
        parameter: "{{ trigger.value }}"

Since every value in Jinja is a string, this parameter will incorrectly be cast to string None instead of the actual None type.

The problem is that we can't infer that the parameter should be None from the actual parameter metadata since valid type for every parameter is null + original parameter type and there is no other parameter metadata attribute which could tell us that (we could potentially infer this is required: false, but that's not ideal either).

Right now we can only infer this from the original TriggerInstance payload value, but mapping this back to the action parameters is too complex and costly.

From top of my head I can only think of the following solutions and each of them has limitation / is not ideal:

1. Infer None type from TriggerInstance payload value, use this information for cast overrides. As mentioned above this approach is complex and something I would like to avoid doing in the rules enforcer (mapping trigger payload attribute name to parameter name is not trivial and gets expensive for nested dictionaries).
2. Cast every None string to actual None type. This is also not ideal since we can't differentiate between actual string None and None type - this basically means user can never use string None as a parameter value inside the rule since it will get cast to None type.
3. Same as number 2, but only do casting to None type if value is string None and if the target action parameter declares required: false. Similar to the above, this approach is also not ideal, but it reduces a potential surface area when valid string None is cast to None type.
4. We could write a custom renderer for Jinja which renders / serializes None types as some magic string which we could then correctly cast to None inside the rules enforce. Imo, that's the most sensible approach and we would only need to do that inside the rules enforcer.

I actually started with the approach 1) since it doesn't cause any false positives, but it turned out that actual of mapping trigger attributes to action parameter names is too expensive and complex...

Imo, the main question at this point really is which one is better - not supporting actual None types (current state of affairs) or casting all string None to actual None types.

I'm open to suggestions.

P.S. Yes, I know the core issue is that in Jinja everything is a string and we are already "abusing" that system by performing casts, but I'm trying to come up with a reasonable short term solution.

[emedvedev]

I'm fine with either 3 or 4, with 4 being slightly more desirable. :)

[Kami]

To clarify, approach 4) also has issues. Jinja renderer is not context aware, so it doesn't know in what context that value is being serialized.

If, for example, user does something like that inside the rule:

action:
    ref: "mypack.myaction"
    parameters:
        parameter: "{{ trigger.value }}-{{ trigger.value2 }}"

The None value would still get serialized to a magic string which would make up for a weird result.

In that sense, 1) really is the most correct solution, but not something we should do on every rule enforcement, since imo, it's simply too expensive.

[emedvedev]

We could do the magic string thing only if the full string is None, otherwise (like in your case above) cast None to an empty string. Also not ideal, just thinking out loud

[Kami]

Then it would be a lot more complicated and I don't think we would be able to do this in the Jinja renderer directly anymore...

[manasdk]

My 2c aka another bad suggestion -

For JSON

{"value": null}

And the following rule:

action:
    ref: "mypack.myaction"
    parameters:
        parameter: "{{ trigger.value | use_none }}"

The use_none filter could generate a magic string __NONE__ of something weird enough which we can interpret specially at render time.

This was user call out what could be None and we can provide a way to deal with the issue.

The real answer is to use something like YAQL which is type preserving. At this point all options look terrible to me and I hoped we would not have to worry about this problem until we switch to YAQL

[Kami]

@manasdk Yeah, that's one of the approaches I thought about, but the thing is you really want to use the same None behavior everywhere so user would probably just end up sprinkling this filter all over the place which is not ideal.

In any case, it's a trade off and if others are OK with it, I'm fine with it as well.

[jjm]

For my use case of needing to pick one of two options for a webhook I think use_none will work fine.

However it being something like YAQL which will perverse type in the long term would be very nice.

[Kami]

This has been resolved (worked-around) with use_none filter in #2623. Closing.