elsa-workflows / elsa-core

A .NET workflows library
https://v3.elsaworkflows.io/
MIT License
6.24k stars 1.14k forks source link

[BUG] `Variable` and `Input` Do Not Work for Custom `IWorkflowProvider` Implementation #5898

Open cali-llama opened 1 month ago

cali-llama commented 1 month ago

Description

When implementing a custom IWorkflowProvider, the JSON representation is serialized and persisted successfully, but upon execution of the workflow, it is not possible to access any workflow Input or to set any Variable value using the known conventions.

For an activity like WriteLine which accesses some Input or a Variable, then there will be no error, but the console line output will be blank. If the activity requires the input property then there will be an error that the same is required such as...

warn: Elsa.Workflows.Middleware.Activities.ExceptionHandlingMiddleware[0]
      An exception was caught from a downstream middleware component
      System.Exception: InCmxContext is required.
         at Elsa.Extensions.InputExtensions.Get[T](Input`1 input, ActivityExecutionContext context, String inputName)
         at Services.Workflow.Workflow.TestActivity.ExecuteAsync(ActivityExecutionContext context)

Steps to Reproduce

Following is an example of an IWorkflowProvider implementation that doesn't do anything useful beyond returning a static MaterializedWorkflow. Once registered with services.AddWorkflowDefinitionProvider<CustomWorkflowProvider>(); then it will be called on startup of the Elsa.Web project and will successfully serialize and persist the workflow to the WorkflowDefinitions table in the Elsa database.

using Elsa.Extensions;
using Elsa.Workflows.Activities;
using Elsa.Workflows.Activities.Flowchart.Activities;
using Elsa.Workflows.Activities.Flowchart.Models;
using Elsa.Workflows.Contracts;
using Elsa.Workflows.Management.Materializers;
using Elsa.Workflows.Memory;
using Elsa.Workflows.Models;
using Elsa.Workflows.Runtime.Contracts;
using Elsa.Workflows.Runtime.Models;

namespace Services.Workflow.Workflow;

public class CustomWorkflowProvider : IWorkflowProvider
{
    public const string CustomVarName = "customVar";
    public const string CustomWorkflowDefId = "CustomWorklowDefinitionId";
    public const string CustomWorkflowId = "abcdefg1234567890";

    public string Name { get; } = "CmxDesignerJsonMaterializer";

    public async ValueTask<IEnumerable<MaterializedWorkflow>> GetWorkflowsAsync(
        CancellationToken cancellationToken = new CancellationToken())
    {
        return await Task.FromResult(new List<MaterializedWorkflow> { MaterializeWorkflow() });
    }

    private MaterializedWorkflow MaterializeWorkflow()
    {
        var identity = new WorkflowIdentity(CustomWorkflowDefId, 1, CustomWorkflowId);

        var workflowMetadata = new WorkflowMetadata("CustomWorkflow", "Custom WF Description", DateTimeOffset.UtcNow);

        var varCustomVariable = new Variable<string>(CustomVarName, default)
        {
            Id = "Workflow1:variable-1", StorageDriverType = typeof(Elsa.Workflows.Services.WorkflowStorageDriver)
        };

        var elsaWf = new Elsa.Workflows.Activities.Workflow
        {
            Id = CustomWorkflowId,
            Name = CustomWorkflowDefId,
            Variables = [varCustomVariable],
            Identity = identity,
            WorkflowMetadata = workflowMetadata,
            Root = MapDesignerToElsaWorkflow(varCustomVariable)
        };

        return new MaterializedWorkflow(elsaWf, Name, JsonWorkflowMaterializer.MaterializerName);
    }

    private IActivity MapDesignerToElsaWorkflow(Variable<string> varCustomVar)
    {
        var setVarAct =
            new SetVariable<string>(varCustomVar, context => context.GetInput<string>(CustomVarName) ?? "FAIL");
        var writeLine = new WriteLine(ctx => $"CustomVariable ==> {varCustomVar.Get(ctx)}");
        var root = new Flowchart
        {
            Activities = [setVarAct, writeLine],
            Connections = [new Connection(setVarAct, writeLine)]
        };

        return root;
    }
}

In the above example, the problem arises when you try to access context.GetInput<string>(CustomVarName) inside the SetVariable activity (see MapDesignerToElsaWorkflow() method). Similarly, the output of the subsequent WriteLine activity is a blank line in the console output.

The serialized workflow json looks good (to me) and if compared to the same workflow but defined as regular programmatic workflow, using the Build() overload, and loaded via the default ClrWorkflowProvider, it is virtually identical. For that reason, I have included links to two diff reports for exactly this comparison.

Following diff report is for the workflow json as it's saved in the database... workflow json diff

...and this is for the data column on the same workflow... workflow data diff

We're attempting to use Elsa as a workflow orchestrator, but we have our own UX experience for building complex end client defined workflows, and was suggested by @sfmskywalker that using IWorkflowProvider would be a good way to translate our own bespoke JSON workflow definition into something that Elsa can execute. However, I'm a bit worried that the state management doesn't seem to be working, given how foundational that is for workflow execution.

If there's something I'm missing, please advise. I've spent many hours pouring through the source, but there's so many layers of abstraction that I feel like it's going to take quite a while before I can find the issue myself, as a relative noob to Elsa.

Expected Behavior

Be able to resolve workflow input and variables within individual activities

Actual Behavior

All references to input and variables are null within an individual activity

Environment

cali-llama commented 4 weeks ago

After digging deeper, I believe that I have found the root cause of this issue...

Context

With a normal programmatic workflow, when Elsa is booting up, the ClrWorkflowProvider materializes the workflow using the WorkflowBuilder which in turn calls the IWorkflow.BuildAsync() for the workflow it's materializing. The resulting workflow object is then serialized into JSON and persisted into WorkflowDefinitions.

One would assume that when the same workflow is executed, that the WorkflowHostFactory (when using DefaultWorkflowRuntime) would use the same serialized JSON from above to execute the workflow, but instead, it re-materializes the workflow all over again using the WorkflowBuilder. (this can be seen by following from WorkflowHostFactory.CreateAsync()). So essentially, we're not eating our own dogfood...the serialized JSON is never consumed for workflow execution and so not validated.

Root Cause

What I've found is that the serialized JSON is incomplete.

For instance, for the dynamically generated workflow mentioned in the original post, when we deserialize the JSON, the Value and Text properties for the SetVariable and WriteLine activities, respectively, are null! If I compare this to a programmatic workflow that uses the exact same activities, parameters, expressions, then you can see (in the following debugger screenshots) that instead of being null the Value and Text properties for the activities is set to an Input<string> with a Func<MemoryBlockReference>. I believe that this is causing the issue b/c the dynamically generated workflow is unable to resolve the input because it's reference is null.

For the following screenshots of activity state, it's helpful to remember that, as seen from the diff reports in the original post, that the serialized JSON of both workflows is virtually identical. Again, the difference results from the fact that the programmatic workflow is rebuilt at time of execution from the WorkflowBuilder (so its JSON is discarded)

SetVariable.Value property on dynamically generated workflow at time of execution... image

SetVariable.Value property on programmatic workflow at time of execution... image

WriteLine.Text property on dynamically generated workflow at time of execution... image

WriteLine.Text property on programmatic workflow at time of execution... image

For reference, and to prove that the programmatic workflow is functionally the same as the dynamic one from the original post, here is the that workflow...

public class TestWorkflow : WorkflowBase
{
    protected override void Build(IWorkflowBuilder builder)
    {
        var varCustomVar = builder.WithVariable<string>(CustomWorkflowProvider.CustomVarName, default);

        var setVarAct = new SetVariable<string>(varCustomVar, context => context.GetInput<string>(CustomWorkflowProvider.CustomVarName)?? "FAIL");
        var writeLine = new WriteLine(ctx => $"CustomVariable ==> {varCustomVar.Get(ctx)}");

        builder.Root = new Flowchart
        {
            Activities =
            {
                setVarAct, writeLine
            },
            Connections = [new Connection(setVarAct, writeLine)]
        };
    }
}

Finally, one other piece of evidence to support my claim...

When first materializing my dynamically generated workflow on Elsa startup before JSON serialization, you can see that both of the SetVariable.Value and WriteLine.Text properties are correctly set (as opposed to the null value that we see post deserialization)...

SetVariable.Value property on dynamically generated workflow at time of initial materialization... image

SetVariable.Text property on dynamically generated workflow at time of initial materialization... image

Resolution

I'm still unsure which way to go from here...is it possible to beef up the JSON serialization such that a deserialized workflow can be executed as is, or do we need to find some way to re-hydrate a workflow that uses the JsonMaterializer? Any input here would be greatly appreciated!

Thank you!

sfmskywalker commented 4 weeks ago

As you correctly observed, all workflow definitions are stored in the database, regardless of the workflow provider type. The data in these database records is meant to be interpreted by the corresponding workflow materializer.

For the ClrWorkflowMaterializer, the stored JSON is essentially useless because we can't (currently, at least) serialize or deserialize elements like C# lambda expressions within the JSON. As a result, the ClrWorkflowMaterializer is designed to invoke the IWorkflowBuilder type again to materialize the workflow definition from the database, effectively ignoring the associated JSON.

However, this behavior may differ depending on your workflow provider. For instance, if your workflow provider generates the same JSON structure as the designer, you might associate the materialized workflow with the JsonWorkflowMaterializer. Alternatively, you could create a custom materializer that reprocesses your custom DSL and disregards the content in the database record, similar to how the ClrWorkflowMaterializer operates.

Looking at your issue, particularly with your custom workflow provider, you're constructing a workflow programmatically, similar to ClrWorkflowProvider and ClrWorkflowMaterializer. Since you're using lambda expressions, such as new WriteLine(ctx => $"CustomVariable ==> {varCustomVar.Get(ctx)}");, the serialized JSON will be irrelevant. Therefore, it would be best to create a custom IWorkflowMaterializer, perhaps named CustomWorkflowMaterializer, to align with the naming convention of your custom workflow provider called CustomWorkflowMaterializer in your example.

This will reliably repeat the mapping from your DSL to a Workflow object every time a Workflow is to be materialized for execution.

cali-llama commented 3 weeks ago

@sfmskywalker, thanks for the input. Once I realized that the problem lay in the serialization, it made much more sense. In the long term, it may be necessary for me to create a custom workflow materializer, but for now for this prototype I've found it that if I use JavaScript expressions (the C# expressions didn't work for me via RunCSharp activity) and avoid using lambdas and Func in the building of my activities, then the state management is working well using my CustomWorkflowProvider coupled with the JsonWorkflowMaterializer.