Support functionRef in dataConditions

[taonic]

What would you like to be added:

Support functionRef syntax in dataConditions for more consistent function reference and explicit argument passing.

Why is this needed:

Currently, dataConditions can reference reusable expression functions as described here. Copy pasted below for reader's convenience:

Expression function:

{
  "functions": [
    {
      "name": "isAdult",
      "operation": ".applicant | .age >= 18",
      "type": "expression"
    }
  ]
}

Function reference in dataConditions:

{
  "states":[
    {
       "name":"CheckApplicant",
       "type":"switch",
       "dataConditions": [
          {
            "name": "Applicant is adult",
            "condition": "${ fn:isAdult }",
            "transition": "ApproveApplication"
          }
       ],
       "defaultCondition": {
          "transition": "RejectApplication"
       }
    }
  ]
}

However, the function, e.g. isAdult is expected to evaluate against the flow state, which creates coupling and hinders reusability. It would be great if the specification can support the existing functionRef object with explicit argument passing. It may look like:

"dataConditions": [
  {
    "name": "Applicant is adult",
    "functionRef": {
      "refName": "isAdult",
      "arguments": {
        "age": "${ $applicant.age }"
      }
    }
    "transition": "ApproveApplication"
  }
]

[tsurdilo]

Thanks for opening issue, wanted to propose maybe something similar but bit different, let me know what you think:

With jq when implementing expression func would most likely do:

    def isAdult($d): $d.applicant | .age >= 18;

With this if you had state data:

    {
       "applicant": {
        "name": "Joe",
         "age": 22
       }
     }

you could do

    isAdult(.)

or even pass custom json to isAdult for example:

    isAdult({"applicant": {"name": "Joe","age": 22}})

So I am not sure we really need support for functionRef in this case but we could allow for input to be passed to the function referenced by "fn".

Imo would follow the way jq does this, so for example:

 "condition": "${ fn:isAdult(INPUT) }",

Where INPUT can be "." or "{...}" just like you can do with straight jq. Can also be references to $SECRETS and other things as well.
This way we could even get rid of "fn" completely.

WDYT

[taonic]

Thanks for the prompt reply @tsurdilo.

I like your proposal, especially its simplicity.

I'd also like to confirm that dataConditions is meant to support either inline jq expression or jq functions but not the other type of functions, e.g. rest and graphql. I'm asking while thinking about the limitations in jq and how to work around it for the more complex conditions.

[cdavernas]

@taoza I confirm! Other functions cannot be used as conditions, at least for the time being

[ricardozanini]

We must consider this possibility carefully since it can open a can of worms and add complexity to the model. jq expressions are simple yet powerful. If data is needed, one can add a state before the Switch to grab it, prepare it in the state content using stateFilter, and then proceed to the conditionals. It clears defines the intention.

[tsurdilo]

State data filers can be applied on the switch state itself if you need to pre-process stuff, not sure why adding extra state is needed but could be some use case i dont see atm.

If we allow arbitrary args to be passed to functions think it would be ok to pass this way for expression functions too, understand that you can do lots of weird stuff with jq but think you can do same weirdness with any expression type property as well currently. Wondering what scenarios or edge cases you have in mind that we should consider?

[ricardozanini]

State data filers can be applied on the switch state itself if you need to pre-process stuff, not sure why adding extra state is needed but could be some use case i dont see atm.

If you need to call an external service, like the OP stated in this description. In that case, another state should be able to call the external function.

[taonic]

If you need to call an external service, like the OP stated in this description. In that case, another state should be able to call the external function.

@ricardozanini I guess you are referring to this scenario: #732 (comment)

I figured it might help to elaborate on my use case. I'm thinking of building a visual workflow builder that produces SW DSL. Therefore, I'd like to have a more UI-friendly way to structure conditions, such as explicit arguments and logical operators (AND, OR), as well as reusing the externalised condition implementation while minimising scripts, such as JQ.

Below is an example from Step Functions Workflow Studio that might help to explain what I mean:

Editor:

Produces:

I'm fully aware that this direction may not align well with the project but I'm keen to hear your thoughts. Thanks folks!

[ricardozanini]

I see. Your use case makes sense. But what I meant is for use cases such as this:

{
  {
  "functions": [
    {
      "name": "isAdult",
      "operation": "myopenapi.json#isadult",
      "type": "rest"
    }
  ]
},
  "states":[
    {
       "name":"CheckApplicant",
       "type":"switch",
       "dataConditions": [
          {
            "name": "Applicant is adult",
            "condition": {
                "functionref": "isAdult",
                "arguments":  { "age": "${  .user.age  }"}
             },
            "transition": "ApproveApplication"
          }
       ],
       "defaultCondition": {
          "transition": "RejectApplication"
       }
    }
  ]
}

Where the function can be an arbitrary function defined in the workflow rather than a simple jq. In situations like this, we have no clue about the function's response interface to infer the datacondition. Additionally, we should add support for stateDataFilter too, so the workflow's author can filter the response somehow since the service might not offer the exactly boolean parameter we are expecting here.

So, suppose you need to rely on an output from a function like this. In that case, I'd say to have a state before the switch to populate the workflow data context with the information and have the datacondition a simple jq expression to infer over the structure.

I agree that support to explicitly declare the parameter would be an excellent addition to the spec, but not calling any arbitrary function from the conditional statement. The jq function can't implement your scenario? For example:

 (.order.total > 10 and .order.country == "CA") or  (.order.total > 10 and .order.country == "US")

[tsurdilo]

Just fyi in early stages of the project we did have very similar switch state boolean operators, however found that it is very limiting and that expression languages can do this much easier and are more powerful

[tsurdilo]

I think you can do these types of UIs just as easy with SW. Instead of building your json structure from these fields you would build your jq expression instead.
This would be have to be done by runtime anyways :)

[tsurdilo]

| In that case, I'd say to have a state before the switch to populate the workflow data context with the information and have the datacondition a simple jq expression to infer over the structure.

@ricardozanini i think this is one way, but with state data filter "input" expression type prop: https://github.com/serverlessworkflow/specification/blob/main/schema/workflow.json#L1847
you dont really need to, you can filter the state data directly with it and it would be filtered before any conditions are evaluated (its the first thing that happens when wf state becomes active)

[ricardozanini]

yes! but if you need complementary data to perform the evaluation, you would need to fetch this data from somewhere, right? So, in that case one would need to use an operation state. Or am I missing something?

[taonic]

I think you can do these types of UIs just as easy with SW. Instead of building your json structure from these fields you would build your jq expression instead.

I agree @tsurdilo, I'm also thinking about exploring this path.

but if you need complementary data to perform the evaluation, you would need to fetch this data from somewhere, right?

@ricardozanini Or perhaps delegate the conditional evaluation to an external service when you can't afford to fetch the data back. For example, when checking the uniqueness of an email address.

[ricardozanini]

@taoza, that again is the problem I was talking about. If you have an external service that, let's say receives an email as input and return a boolean value you can't possibly know the interface/data structure by reading the workflow DSL. You can just assume. That's what I was trying to explain in my previous comment. You can rather call the service in another state, include the response to the data context, and validate it in the conditionals in the switch state.

[taonic]

let's say receives an email as input and return a boolean value you can't possibly know the interface/data structure by reading the workflow DSL. You can just assume.

@ricardozanini For RESTful APIs, could we gain the certainty from the mandated OpenAPI spec?

Also, could an inline expression do the trick instead of adding the response to the flow state?

Assume the JSON response is:

{
  "data": {
     "unique": true
  }
}

Inside dataConditions

"condition": "${ fn:isEmailUnique | .data.unique }",

[ricardozanini]

@ricardozanini For RESTful APIs, could we gain the certainty from the mandated OpenAPI spec?

We can, but users would have to go to the OpenAPI to see the interface; it won't be evident in the DSL.

Yes, your example is valid and clearly states the interface in this case. My concern is that this is not always true, unfortunately. I also understand that adding one more state to the workflow to fetch the information can be too lengthy.

I'd like to hear from other community members about it. Personally, I won't add this feature to the DSL. However, I won't stand against it.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[cdavernas]

Closed as resolved by 1.0.0-alpha1, and therefore as part of #843