Workflow Engine & API Changes

[sfmskywalker]

For V3, I am thinking of making a number of changes.

These are just thoughts and I am looking for feedback. Nothing is set in stone as of yet.

Activity Data & Handlers

Currently, an activity is both a DTO as well as a service - it contains input & output properties and methods that implement behavior.

If we separate activity data from behavior, things will become easier:

• Serialization of activities simplified
• Ability to construct workflows by composing actual activity objects (this ties into the next section)
• Easier testing.

It would look something like this:

public class WriteLine : Activity
{
   public IActivityInput<string> Text { get; set; }
}

public class WriteLineHandler : IActivityHandler<WriteLine>
{
   public override void ExecuteActivity(WriteLine activity)
   {
      Console.WriteLine(activity.Text);
   }
}

A workflow would be constructed as follows:

public class MyWorkflow : Workflow
{
   public MyWorkflow()
   {
      Activities.Add(
         new Sequence ( // Support for container activities with natural scope.
            new WriteLine { Text = "Line 1" }, // Set a string literal. Implicitly cast to ActivityInput<string>.
            new WriteLine { Text = new CodeInput<string>(context => context.Input) }, // Set a dynamic value using a C# lambda.
            new WriteLine { Text = new LiquidInput<string>("Hello {{ Input }}") } // Set a value using a liquid expression.
         )
      );
   }
}

Connecting Activities

With V2, all activities are connected to one another via connections. Although this is convenient, it also poses a challenge to implement nested scopes and self-scheduling of child activities such as If, ForEach, Switch and others.

It also makes setting up coded workflows inconvenient. Although the Workflow Builder API does a nice job using Then<> and When, the following workflow would be even easier to setup:

public class MyWorkflow : Workflow
{
   public MyWorkflow()
   {
      Activities.Add(
         new Sequence(
            new HttpEndpoint { Path = "/demo", Methods = new[] { "GET" } },
            new If { 
               Condition = JavaScriptInput<bool>("queryString('answer') == '42'"),
               True = new WriteLine { Text = "Correct!" },
               False = new WriteLine { Text = "Wrong..." }
            }
        );
   }
}

This also makes connecting multiple sources to the same destination much more natural:

public class MyWorkflow : Workflow
{
   public MyWorkflow()
   {
      var exit = new WriteLine { Text = "Bye!" };

      Activities.Add(
         new Sequence ( // Support for container activities with natural scope.
            new HttpEndpoint { Path = "/demo", Methods = new[] { "GET" } },
            new If { 
               Condition = JavaScriptInput<bool>("queryString('answer') == '42'"),
               True = new Sequence(WriteLine { Text = "Correct!" }, exit), // Connect to exit node.
               False = new Sequence(new WriteLine { Text = "Wrong..." }, exit) // Connect to same exit node.
            }
         )
      );
   }
}

With this API, containment would be an intrinsic concept, which removes all of the unintuitive quirks we currently have with Composite Activities in V2.

A consequence of this is that we lose the concept of "outcomes". Instead, an activity now defines "ports". The If activity for example would look something like this:

public class If : Activity
{
   [Port] public IActivity True { get; set; }
   [Port] public IActivity False { get; set; }
}

public class IfHandler : IActivityHandler<If>
{
   public async ValueTask ExecuteActivityAsync(If activity, ActivityExecutionContext context)
   {
      var result = await context.EvaluateAsync(activity.Condition);
   
      if (result == true)
         context.ScheduleActivity(activity.True);
      else
         context.ScheduleActivity(activity.False);
   }
}

Being able to assign activities to activity properties requires the workflow designer to receive a number of changes:

• When connecting e.g. some WriteLine activity to the True property of an If activity, we want to see a connection drawn. The designer needs to be able to serialize that into an hierarchical object model, instead of maintaining a list of connections between activities.
• When designing a workflow, the user should be able to freely connect one activity to another. The model however should use a Sequence container activity when the user connects activities that don't have explicit ports like the If activity.

Unwinding

V2 currently relies on "scopes" to enable outcomes such as "Iterate" to automatically re-schedule its owning activity such as ForEach, which then determines if the loop has finished or not. If finished, it schedules the "Done" outcome.

This implementation however is prone to error and is hard to apply to custom activities because there are multiple moving parts that need to cooperate to make this work.

With the proposed API changes, this will become a thing of the past, because it now becomes easy to determine if a child activity (an activity set to a port or within a sequence (which itself is a container activity) has completed. Activities such as ForEach will have two port properties: Loop and Complete, both of which are of type IActivity.

Pseudo code for ForEach:

public class ForEach : Activity
{
   public IActivityInput<bool> Condition { get; set; }
   [Port] public IActivity Loop { get; set; }
   [Port] public IActivity Complete { get; set; }
}

public class ForEachHandler : IActivityHandler<ForEach>
{
   public async ValueTask ExecuteActivityAsync(ForEach activity, ActivityExecutionContext context)
   {
      var result = await context.EvaluateAsync(activity.Condition);
   
      if (result == true)
      {
          context.ScheduleActivity(activity.Loop)
      }
   }

   // This method is invoked by the workflow engine once an activity completes that was scheduled by this activity.
   public void ScheduledActivityCompleted(ForEach activity, ActivityExecutionContext context, IActivity completedActivity)
   {
      if(completedActivity != Loop)
         return;

        var result = await context.EvaluateAsync(activity.Condition);
     
        if (result == true)
        {
            context.ScheduleActivity(activity.Loop)
        }
        else
        {
            context.ScheduleActivity(activity.Complete)
        }
   }
}

TODO: Describe workflow bookmarks for resuming suspended workflows.

[tomy2105]

What you've described and the benefits of it seems sound.

However I have one huge fear about this, and it is called backward compatibility. How easy will it be for people who wrote their own activities (god bless lunatics who are maybe using ActivityTypeProviders :)) to move from 2.x code to 3.0 code?

[tomy2105]

Another questions I would have is how will this change affect other possible Elsa 3.0 tasks like for example multi-tenancy.

I ask because, although it has number of benefits as you've described, this doesn't bring anything new functionality wise (we already have activities, outcomes, etc.....). Hence I'd rather see stuff that is "obvious" functionality gap included in next Elsa releases (before this particular change), like:

• multi-tenancy
• numerous designer improvements (copy/past/undo/redo/custom activity dialog implementation, etc.....)
• documentation (both user side and programmer side), and yes I'm painfully aware how boring and dull such tasks are :(

[jimitndiaye]

Your proposal sounds alot like WF4 activities. May be worth looking at the activity composition model there for inspiration.

[sfmskywalker]

Very observant :) It is indeed inspired on WF’s API.

[jimitndiaye]

Is there a specific reason why you've separated activities from their handlers? Generally I picture activities as self-contained bits of functionality including inputs, variables and an functional implementation. Separating them by reducing the activity to just a message containing the inputs to be passed on to separate handler seems counterintuitive to me.

[jdevillard]

Hello, some thoughts here

Seems very interesting, indeed, as I work a lot on WF4 Activities, I can see the inspiration ;).

I like the concept of sequence and new conception of connection (scheduling). I think it could help to create easily things like Try/Catch Container (things missing for me) and composite Activity.

as @tomy2105 said, the fear is about the compatibility with 2.x version. This version is quite well and I'll work hard to include this version on my current software. Maybe with the good adoption of 2.x version, a migration tools will be useful but not sure it will work for custom activity)

For me if Core Engine is changed for better containerization of Activity, I think it could be really useful to think if this new Arch is better in a distributed environment (where we can choose with granularity that some Activity execute in specific container ( ex here is XSLT 3.0 with custom library not available on NetCore so Activity can be executed in a specific Container or Environment that use NetFramework 4.7.+ or maybe mix Cloud/OnPrem).

About Multi-Tenancy, I think it's really useful (And a lot of people need it) but it could be a parallel work as It should not impact (or a little) the Core Engine. And at last, Designer is really cool but need also some elements as said by @tomy2105 that could be done before especially if designer need a lot of rework due to new engine. And for me it's important that all could be done via Designer.

Have you already some thoughts about the support of the version 2.X if you start to work on 3 ? (this could reassure people that starting to work on the current version)

Otherwise, thanks for the good work , I really like the solution and I'm happy to be able to work on it ;)

[sfmskywalker]

@tomy2105 @jdevillard

However I have one huge fear about this, and it is called backward compatibility. How easy will it be for people who wrote their own activities (god bless lunatics who are maybe using ActivityTypeProviders :)) to move from 2.x code to 3.0 code?

What we might consider is to run both engines side-by-side within the same package. What I mean by that is that we keep the V2 APIs for activities, but also allow V3 activities and workflows.

Although we would clearly mark V2 aspects as deprecated, users can migrate their code and workflows at their own pace for at least a couple of versions, if not more depending on feedback.

Perhaps the V2 engine becomes just another type of "workflow" for V3 (which allows any activity to handle child activities as they see fit in order to allow types like Sequence and Flowchart.

Some APIs might break at compile time, like the ActivityTypeProvider (I don't know what crazy people will even implement them, but surely there are a few ;) ), but that shouldn't have to change too drastically I don't think.

how will this change affect other possible Elsa 3.0 tasks like for example multi-tenancy.

We'll be revising the 3.0 milestone. As @jdevillard suggested, this is a separate feature that can be done separately. Probably it will be done as part of 2.2 or 2.3, depending on demand, bandwidth and interested contributors and such.

And at last, Designer is really cool but need also some elements as said by @tomy2105 that could be done before especially if designer need a lot of rework due to new engine. And for me it's important that all could be done via Designer.

Agreed.

[sfmskywalker]

@jimitndiaye

Is there a specific reason why you've separated activities from their handlers? Generally I picture activities as self-contained bits of functionality including inputs, variables and an functional implementation. Separating them by reducing the activity to just a message containing the inputs to be passed on to separate handler seems counterintuitive to me.

I'm glad you asked. My initial response was very similar to that of yours when @craigfowler first proposed this change actually. Like you, I regard activities as self-contained bits of functionality. But it's hard to consider the benefits of separating data from logic, such as:

• Activities as data make it easy to serialize and deserialize (no DI, no constructors other than the default one without service dependencies)
• Activities without non-default constructors are easy to instantiate when writing coded workflows: just do e.g. new MyActivity(). If on the other hand MyActivity depends on e.g. IMyCustomerService, you would now rely on a service provider to create an instance.
• Activity handlers on the other hand are pure logic that can take on as many dependencies as they need.

To get the benefits mentioned above, we might do what WF does, which is providing access to services from the activity execution context, and simply not allowing (or at least recommending) constructor injection. Property DI on the other hand might be fine, although the serializers need to be aware of this (i.e. ignore these properties).

A sample activity that is a single unit that is easily constructed while also depending on some service could look like this:

public class MyActivity
{
   public IActivityExecutionResult OnExecuteAsync(ActivityExecutionContext context)
   {
      var service = context.GetService<IMyService>();
   }
}

A downside of that approach is that it "hides" the dependencies of MyActivity. When doing it as a separate handler however, the dependencies are immediately clear when using constructor injection.

In any case, I haven't decided yet. I am leaning towards keeping the "single unit" approach, but I want to explore the "handler" approach as well and hopefully land on something elegant.

[craigfowler]

I certainly like separation of activity model/state from activity logic.

Additionally, I'm a big fan of not-requiring user/consumer-crafted extensions to do anything more than implement an interface. IE: No subclassing if I can avoid it (even abstract classes).

What makes an activity?

What we seem to have is:

• Activity boilerplate state - that's the properties which are present on the Activity base class such as Name, DisplayName and such. This is common to every activity.
• Activity-specific state - this is properties which would be added to custom activity classes and which are not common
• The logic for the activity, mainly the ExecuteAsync method but also CanExecuteAsync and OnResumeAsync
  • There are some convenience overloads of these for things like a non-async API
• Some convenience functions for frequently-used results like Done or Outcomes(/* ... */) etc
• Get State & Set State functions

The state models

That activity boilerplate state, the more I look at it, the less I can think of any good reasons to put it together with the activity-specific state. This is essentially activity metadata or an activity descriptor. The first example in this issue suggests having the activity-specific state subclass the activity descriptor state but is there anything practical gained from this? If they were completely split, then the activity-specific state could be a POCO with no coupling or constraints of any kind.

Unlike my gut feeling about workflow-specific models, I think activity-specific models are really useful and important. It would allow us to do away with the GetState/SetState concepts from the activity class altogether, which IMO are unnecessary complications. An activity class is self-contained and should not be depended-upon by other activities. This means that within the execution/configuration of an activity, a strongly-typed model is really good.

The logic

I also wonder if it's worth breaking-apart the interfaces for the activity logic. I'm using the name IActivity here because I don't think the model needs a base class. Conceptually you could call it the "activity model" though.

// Used for activities which need no model.  The nongeneric impl of ActivityExecutionContext has no model property!
public interface IActivity
{
    Task<IActivityExecutionResult> OnExecuteAsync(ActivityExecutionContext context, CancellationToken token = default);
}

// The generic ActivityExecutionContext subclasses the nongeneric one but also has a model property of the appropriate generic type
public interface IActivity<TModel>
{
    Task<IActivityExecutionResult> OnExecuteAsync(ActivityExecutionContext<TModel> context, CancellationToken token = default);
}

// These two interfaces follow the same principle as above, one with a model one without
public interface ICanConditionallyExecute : IActivity
{
    Task<bool> CanExecuteAsync(ActivityCanExecuteContext context, CancellationToken token = default);
}

public interface ICanConditionallyExecute<TModel> : IActivity<TModel>
{
    Task<bool> CanExecuteAsync(ActivityCanExecuteContext<TModel> context, CancellationToken token = default);
}


// Once again, the same principle as above, one with a model one without
public interface ICanResume : IActivity
{
    Task<IActivityExecutionResult> OnResumeAsync(ActivityResumingContext context, CancellationToken token = default);
}

public interface ICanResume<TModel> : IActivity<TModel>
{
    Task<IActivityExecutionResult> OnResumeAsync(ActivityResumingContext<TModel> context, CancellationToken token = default);
}

I specified different context types for "can execute" and "resuming" in case these scenarios offer different contextual information. If it's not appropriate or needed then feel free to share the same context type.

I think this is at least worth considering because of "The I of SOLID": ISP. If an activity (logic) class has no logic to define for CanExecuteAsync or OnResumeAsync then just don't bother implementing those interfaces. The host class which is "driving" the activity sees it doesn't have that interface and assumes a no-op. There's no need to code the no-op in a base class.

Suggest dropping the convenience overloads for the logic functions

Within the base Activity class there are overloads like OnExecute for a non-async API and also parameterless convenience overloads. This is a bit of a "landmine in the code" IMO and whilst it does have value, I think that the value is not worth the cost. For example, a developer could incorrectly override more than one of those related methods without understanding how the default implementations chain-into one another, and that by overriding more than one, they might break that chain.

Also, I feel like this might be trying to hold a developer's hand too tightly. All we are saving the developers from by writing these overloads is typing: Task.FromResult(/* ... */) or from accepting-but-then-not-using a parameter. What we traded-in for that benefit though, was a more confusing list of available virtual methods when they type override into their IDE and intellisense activates. If you've got a list of length one, then you can't choose the wrong thing ;)

The convenience functions for commonly-used outcomes

Apart from the get/set state functions (which we can do away with by switching activity state to a model), the only other things left on the base class are those conveniences for common outcomes. I do agree that we should provide them, but could I suggest instead putting them on a static class and then recommending developers leverage using static?

public static class CommonOutcomes
{
    public static OutcomeResult Done() => /* ... */;

    public static OutcomeResult Outcomes(/* ... */) => /* ... */;
}

Then in an activity logic class the developer can use return CommonOutcomes.Done(); or even more terse, if they have using static Elsa.CommonOutcomes; at the top of their source file, they only need write return Done(); in the same way they do at the moment.

Given we're currently expecting the developer to derive from a base class (strongly-coupled) at the moment, switching to a static convenience class is no worse as I see it. All that's important is that we never put anything non-trivial in that 'convenience class' of common outcomes. Because it's static, we will never be able to DI into it and we will never be able to substitute impl, so it really must be trivial/convenience functions and no more.

What would we have between all of this?

If we went with every part of this (and I'm happy for this idea to be sliced up and taken in part or modified) is that:

• An activity logic class is a C# class which only needs implement one interface and have one method OnExecuteAsync. It can also implement up to two more interfaces indicating it has CanExecuteAsync and OnResumeAsync methods, where desired.
  • There's no need or even motivation to derive from a base class, only interfaces.
• Activities may implement a generic version of those interfaces or a non-generic version; where it implements the generic version then its functions gain access to a strongly-typed activity model of the same generic type
• The activity model where used may be any type at all (custom ones would be a C# POCO with no constraints).
  • I almost suggested adding a new() generic type constraint (in order to create instances of the model for the first time) but actually if we need to create one and it has no public parameterless constructor we could try resolving it from DI
  • Then, all the guidance we need to give developers is "if you want to use a model which can't be created by Activator.CreateInstance, then make sure it can be resolved from DI, such as with a transient registration"
  • Oh, and of course, "make sure that your model is compatible with "
  • Perhaps for models which are not natively compatible, we could offer an interface for the developer to implement their own per-model-type custom serialization? That's probably going a bit beyond the scope of this issue though.
• The boilerplate activity state (activity descriptor) goes in a class of its own which is managed by Elsa and developers of custom activities don't need to care about it at all, it'll just be there when you create the activity and it'll be available from any appropriate context classes

[craigfowler]

Re-reading some of the other info here, I think that the activity should still be "logic first", because that's the thing which every activity needs at the very minimum, the OnExecuteAsync function. Whether or not an activity has a custom state/model is optional.

What I expected when building a workflow from code is an interface like:

public interface IBuildsWorkflow
{
    // Then, StartsWith, FinishWith - they're all semantically the same
    // except I guess when finishing we don't need to return a builder
    IBuildsWorkflow Then<TActivity>(Func<ActivityDescriptor> descriptorFactory = null) where TActivity : IActivity;

    IBuildsWorkflow Then<TActivity>(Func<ActivityDescriptor<TModel>> descriptorFactory = null) where TActivity : IActivity<TModel>;
}

public class ActivityDescriptor
{
    public string Name { get; set; }
    public string Description { get; set; }
    /* ... */
}

public class ActivityDescriptor<TModel> : ActivityDescriptor
{
    public TModel Model { get; set; }
}

The usage might look something like:

builder.Then<MyFunkyActivity>(() => new ActivityDescriptor { Name = "ActivityOne", Model = new MyFunkyModel { TurnItUp = 11 } });

Although here I'm specifying/implying that those objects are created anew in the usage of the activity. They don't have to be, Elsa could control their creation and then just provide Action<> instead of Func<> so that they are just mutated rather than created. There are pros and cons to either (if we can only mutate then it's less easy to write a simple initialisation as a one-liner), so I'd suggest picking whichever looks nicer to you.

[sfmskywalker]

Check out this sneak-peek of a POC I'm working on for the Elsa 3 designer with embedded activities (as discussed during last community call)! There's still a lot of work to be done.

For example, for simple activities, we could keep it embedded as you see now, but for more complex activities such as Sequence and Flowchart, we should probably only render a symbol and a display name, but open a new "tab" with a designer to edit the contents.

I'm also not sure about rendering the embedded activities as-is. Perhaps it needs a slightly different representation. Different color, just icon, no icon at all, etc. Feedback welcome!

[jimitndiaye]

@sfmskywalker are you considering a State Machine activity in addition to Sequence and FlowChart, like in WF4?

[sfmskywalker]

Yep, that’s the plan.

[jimitndiaye]

Looking foward to it. At the moment I'm experimenting with creating a blazor-designer for workflow activities based on CoreWF (a .NET Core Port of WF4) - has most of the goodness of the WF4 minus the rehosted designer. May be worth looking into.

[sfmskywalker]

Here's an updated preview that takes inspiration from WF & ADF:

[tomy2105]

As always in life, I love some things and don't like so much some others.....

I love the second preview with tabs :). I love new way of adding variables and assigning Activity Outputs (CurrentItem of While) to a variable (hope it will work for all outputs).

What I don't like (and this has been a debate for some time), is If activity behaving like this. Must be a force of habit or something but I've never gotten used of If activity having Done output (and this true and false being embedded into it). For me if (and this comes from using designer only, not code for activities) if you want your branches to "unite back" you just do it in workflow by connecting them to same activity at some point..... :). Strangely I don't have this problem with For(Each)/While.... Come to think of it I've written many workflows which don't unite after branching (or unite after 100+ activities).... Therefore I would definitely like to see:

1. If activity which is just a branching which (no embedded activities, no done output)
2. (Optionally) an option on all activities with possible embedding whether to have it displayed embedded or not.

Yes, this makes things complicated but also more powerful and maybe easier to understand for people who are not programmers.

[sfmskywalker]

I'm on the same page with you there actually. I'm thinking to do it like this:

• The current If activity is a regular activity that "embeds" the Then and Else activity (which themselves could be any kind of activity. This is similar to the If activity in Windows WF.
• There should be a new activity called e.g. Condition. This would be a "flow node" activity that has two outcomes: True and False. This is similar to the FlowDecision activity in Windows WF.

In Windows WF, there is a clear distinction between sequential activities and flowchart activities. In Elsa 3, there's no such clear distinction, which on the one hand is very convenient, but can potentially cause some confusion.

The reason we don't need a flowchart-style version for activities like For, ForEach and While is because here the control flow wouldn't make much sense if the user would be allowed to merge from the "Body" activity (called "Iterate" outcome in V2) to some other arbitrary activity in the workflow. Instead, you typically want this bit of execution to be contained within the looping construct.

Although we could let the user control the display mode of workflows between "flowchart" and "embedded", this is very complicated because we would need to recursively transform child activities into container activities to assign them to e.g. the Then port of the If activity.

Let me remind you that in Elsa 3, the If activity looks like this:

public class If : Activity
{
   public IActivity Then { get; set; }
   public IActivity Else { get; set; }
}

Notice that the ports (Then and Else) only accept a single destination activity. This means that if you have a flowchart where the then outcome is connected to Writeline1, which in turn is connected to Writeline2, there's no simple way to bring that into the Then property without wrapping these two into their own Flowchart or Sequence activity.

I started down this route, and although it was gonna work, I don't think it's worth the complexity.

Instead, I think there's a better way where we do it similar to Windows WF, which has different kinds of activities:

1. Control-flow activities
2. Flowchart activities

In WF, the user has to make a choice between a Flowchart workflow and a Sequence workflow (both of which are just activities). In Elsa, we too have these two types of workflows, however, we only have one type of designer for now: the Flowchart designer.

An interesting feature we have now though is that in Elsa, any activity can be added to a flowchart, and the designer supports this.

So, what we could do is introduce 2 new "flowchart" activities (aka "flow nodes"):

• FlowDecision
• FlowSwitch (to solve the same issue with Switch activity which also supports embedding).

These activities would be available from a new "Flow" category, and would behave slightly different from their sequential If and Switch twins:

• The FlowDecision has two outcomes called True and False
• The user can connect to these outcomes using Connections (in the designer, the user would visually draw these connections)

Currently, the Flowchart activity in V3 only knows about a "Done" outcome, but I will change this to support multiple outcomes like we have in V2.

Although one might think having different flavors of similar activities is confusing, I think it's a lesser evil compared to trying to transform/transpose flowchart-style diagrams into sequential/embedded views and the other way around. It's very complex and maybe not worth it. Especially since the potential confusion could be easily mitigated by simply allowing the application to restrict the kinds of activities available to a kind of workflow (e.g. Flowcharts only support FlowNode, FlowDecision and FlowSwitch).

Which we might need to do also for future workflow types like State Machine and BPMN.

[tomy2105]

Having two kinds of If and Switch activities (one with and one without embedding and done output) would work perfectly IMHO and probably easier to implement and maintain without introducing complexity and errors.