Providing a mechanism to pass a list of Works in SequentialFlow

[arunav-bhattacharya]

For an application supporting multiple complex workflows for different APIs with a considerable number of Works in each WorkFlow, it is kind of redundant to provide the individual Works/WorkFlows everytime for building a new WorkFlow. Everytime if they have to add or delete a worker from a WorkFlow, they have to make a change in the code where the workflow is being built. Ideally, I would want to abstract the building and execution of a particular type of WorkFlow from the multiple developers working on a large codebase and would just want them to write only the specific Work to support or enhance the Business process.

For example, all the Works part of a particular WorkFlow can be defined under a single package and at runtime just the package can be passed to a WorkFlow builder, which can internally create a List of Works under that particular package and pass it to the easy-flows SequentialFlow. With the current implementation however this is not possible as I have to use the builder pattern for building individual SequentialFlows by providing the Works/WorkFlows explicitly.
With a minor enhancement to the SequentialFLow Builder, we will be able to accomplish this.
Currently the execute() method accepts Work, but if we provide an overridden method of execute to accept List, then that will provide the flexibility to the users on how they want to use it -
public SequentialFlow.Builder execute(List works) {
this.works.addAll(works);
return this;
}
The developers can simply write their individual Works in a particular package and then an abstracted SequentialWorkFlowBuilder class can scan through all the Works in that particular package and create the list of Works at runtime. This list can be passed to the execute(List works). We may also consider about the then() method, which can also be overridden to accept List.

In this context I would like to highlight that, the ParallelFlow already has this option -
List works = workerList; //passed at runtime
Work[] workList = works.stream().toArray(Work[]::new);
ParallelFlow flow = aNewParallelFlow(Executors.newFixedThreadPool(2))
.execute(workList)
.build();

Please let me know your views on this.

Thanks,
Arunav

[arunav-bhattacharya]

I have forked and pushed the changes here -
arunav-bhattacharya@bf47f38

[fmbenhassine]

Thank you for opening this issue. I can understand overloading the next method to accept a list of work units, but not execute. Because this will make it possible to write something like:

SequentialFlow sequentialFlow = SequentialFlow.Builder.aNewSequentialFlow()
        .execute(work1, work2)
        .execute(work3)
        .then(work4)
        ...

which does not make sense to me. execute method is expected to be called only once for the initial work unit in the sequence, and as discussed in gitter, this will be enforced by using the step builder pattern in a future release (I opened #18 for that). Once this will be enforced, the previous snippet would not make sense because its ambiguous: what should be executed first, work1 or work3? I hope this makes it clear.

In this context I would like to highlight that, the ParallelFlow already has this option

That's different. For a parallel flow, it does not make sense to have a method execute with a single work (execute a single work in parallel?). That's why this method accepts a list of work units (those which needs to be executed in parallel).

I have forked and pushed the changes here -

LGTM. If you remove the overloading of the execute method and keep only the one for next, you can open a PR and it will be good to merge.

[arunav-bhattacharya]

Thank you for your feedback !

which does not make sense to me. execute method is expected to be called only once for the initial work unit in the sequence, and as discussed in gitter, this will be enforced by using the step builder pattern in a future release (I opened #18 for that).

The sole reason for wanting to have an additional execute method that accepts a list of Work is that in case the user wants to create a sequentialFlow only using a list in the order in which they created the list at runtime (as I explained in my earlier post above) they don't have to use the other methods in the builder pattern. Enforcing to not have a second 'execute' can be done when the step builder is in place, then there wont be any ambiguity in the order of execution.

Once this will be enforced, the previous snippet would not make sense because its ambiguous: what should be executed first, work1 or work3? I hope this makes it clear.

Currently the order of execution of the sequentialFlow is defined by the order in which it is being built. For example, I can build something like this and the flow will be executed in the order it is being built irrespective of the method -

SequentialFlow sequentialFlow = SequentialFlow.Builder.aNewSequentialFlow()
        .then(work1)
        .execute(work3)
        .then(work4)
        ...

We can plan to fix this pattern as well as a part of the step builder pattern fix.

The suggestion is just to ease the building of the sequentialFlow by passing a list dynamically, instead of having to write the same code aNewSequentialFlow().execute(work1).then(work2)...then(workn).build() multiple times for different sizes of lists of Work.

Let me know if you agree with this.

[fmbenhassine]

@arunav-b #18 is now resolved. By using the step builder pattern, it is now impossible to write semantically incorrect workflow definitions. The builder will guide the user to call methods in the right order like:

SequentialFlow sequentialFlow = SequentialFlow.Builder.aNewSequentialFlow()
        .execute(work1)
        .then(work2) // this is optional
        .then(work3) // this is optional
        .build();

We can now add overloaded execute and then methods that accept a varargs of work units, such that it is possible to write something like:

SequentialFlow sequentialFlow = SequentialFlow.Builder.aNewSequentialFlow()
        .execute(work1, work2)
        .then(work3, work4) // this is optional
        .build();

Feel free to open a PR if you want to contribute, otherwise let me know and I will introduce the changes.