Possible bug on subtasks ?

[cvgaviao]

Hello,

I've started to study this tool which got my attention this week.

I took one of your tests and tried to add another child in the parent workflow.

But first I would like to say that having two methods workflow for different purpose in the DLS is not much intuitive :(

Anyway, the idea here was to experiment how to deal with multiple complex subtask that will be mixed together later. Next step is do the same with parallel subtasks.

My expectation was that since I'm just changing the counter on the second child the greeting provided by the child1 would be kept.

But the test failed with a null value for greeting.

java.lang.AssertionError: 
Expected: is "helloWorld"
     but: was null
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:6)
	at com.natixis.matrisk.api.management.releases.flows.SubWorkflowTest.runSubWorkflowFromDslTest(SubWorkflowTest.java:48)

Am I missing something here, or its a bug?

    @Test
    void runSubWorkflowFromDslTest() throws Exception {
        Workflow child1 = WorkflowBuilder.workflow("childFlow1", "org.acme", "1.0.0")
                .tasks(d -> d.set("update1", s -> s.put("counter", 1).put("greeting", "helloWorld")))
                .build();
        Workflow child2 = WorkflowBuilder.workflow("childFlow2", "org.acme", "1.0.0")
                .tasks(d -> d.set("update2", s -> s.put("counter", 2)))
                .build();

        Workflow parent = WorkflowBuilder.workflow("parentFlow", "org.acme", "1.0.0")
                .tasks(
                        // child1
                        DSL.workflow(DSL.workflow("org.acme", "childFlow1", "1.0.0")
                                .input(Map.of("id", 42, "region", "us-east"))),

                        // child2
                        DSL.workflow(DSL.workflow("org.acme", "childFlow2", "1.0.0")
                                .input(Map.of("id", 42, "region", "us-east"))))
                .build();

        try (WorkflowApplication app = WorkflowApplication.builder().build()) {
            app.workflowDefinition(child1);
            app.workflowDefinition(child2);
            Map<String, Object> result = app.workflowDefinition(parent)
                    .instance(Map.of())
                    .start()
                    .join()
                    .asMap()
                    .orElseThrow();
            assertThat(result.get("counter"), is(equalTo(2)));
            assertThat(result.get("greeting"), is(equalTo("helloWorld")));
        }
    }

[cvgaviao]

Hey @fjtirado, could you please take a look on this ? :)

[fjtirado]

We are currently changing the Fluent DSL to enhance the way subflows are defined, which I agree is cumbersome.
In any case, there is not bug, the workflow is behaving as expected according to my understanding of set keyword on the cnfc spec. That understanding is that the set do not merge the specified with the previous input, but override it. To discuss that understanding, you can open a thread here (actually I encourage you to do, because after carefully reading the spec, that is not 100% clear)
With that understanding, since you are calling two subflows in sequence, the first one sets the output to {counter=1, greeting=helloword}, the seconds one overrides it and sets the output to {counter:2}, which is the output of the workflow and why the test fail.
You can make it work by changing the example to this

Workflow child1 =
     WorkflowBuilder.workflow("childFlow1", "org.acme", "1.0.0")
         .tasks(d -> d.set("update1", s -> s.put("counter", 1).put("greeting", "helloWorld")))
         .build();
 Workflow child2 =
     WorkflowBuilder.workflow("childFlow2", "org.acme", "1.0.0")
         .tasks(d -> d.set("update2", s -> s.expr("$input+{counter: .counter+1}")))
         .build();

 Workflow parent =
     WorkflowBuilder.workflow("parentFlow", "org.acme", "1.0.0")
         .tasks(
             // child1
             DSL.subflow(
                 DSL.workflow("org.acme", "childFlow1", "1.0.0")
                     .input(Map.of())),

             // child2
             DSL.subflow(
                 DSL.workflow("org.acme", "childFlow2", "1.0.0")
                     .input(Map.of())))
         .build();

[fjtirado]

There is a bug though, input should not be mandatory for subflow, so you can actually access the model from the parent, I worked around that by adding input(map.of))

This definition

    Workflow child1 =
        WorkflowBuilder.workflow("childFlow1", "org.acme", "1.0.0")
            .tasks(d -> d.set("update1", s -> s.put("counter", 1).put("greeting", "helloWorld")))
            .build();
    Workflow child2 =
        WorkflowBuilder.workflow("childFlow2", "org.acme", "1.0.0")
            .tasks(d -> d.set("update2", s -> s.expr("$input+{counter:.counter+1}")))
            .build();

    Workflow parent =
        WorkflowBuilder.workflow("parentFlow", "org.acme", "1.0.0")
            .tasks(
                // child1
                DSL.subflow(
                    DSL.workflow("org.acme", "childFlow1", "1.0.0")),

                // child2
                DSL.subflow(
                    DSL.workflow("org.acme", "childFlow2", "1.0.0")))
            .build();

should work but is throwing a null pointer exception.

Ill fix that with #1316