feat: support to subflows on the Java DSL#1213
feat: support to subflows on the Java DSL#1213matheusandre1 wants to merge 3 commits intoserverlessworkflow:mainfrom
Conversation
ac6bdf9 to
98a4bf7
Compare
3f75faa to
8a790b2
Compare
mcruzdev
left a comment
There was a problem hiding this comment.
It looks great @matheusandre1, could you add a test running a workflow with your changes?
Signed-off-by: Matheus André <[email protected]>
e118bae to
c9388bf
Compare
fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/dsl/WorkflowSpec.java
Show resolved
Hide resolved
ricardozanini
left a comment
There was a problem hiding this comment.
Looks solid, we just need to add the Function to the input or it's useless for the FuncDSL (unless jq scripting).
@ricardozanini The Function should be located at the experimental module, I think it can be another PR. |
ricardozanini
left a comment
There was a problem hiding this comment.
Indeed, the Function can be added to a new PR - or directly to this one. Not sure why not. I do the DSL changes in both sides all the time since one influence the other.
|
@mcruzdev aproove? |
|
Hi @matheusandre1, when a said test I would like something like Executing the new DSL method with a WorkflowApplication: try (WorkflowApplication app = WorkflowApplication.builder().build()) {
WorkflowDefinition def = app.workflowDefinition(workflow);
WorkflowModel model = def.instance(10L).start().join();
Number number = model.asNumber().orElseThrow();
softly.assertThat(number.longValue()).isEqualTo(25L);
}The test you added is so good, but I think we need to test executing too. |
|
I'm going to do it. |
90e72b7 to
37add85
Compare
| Workflow workflow = | ||
| FuncWorkflowBuilder.workflow("enrichOutputWithTaskOutputTest") | ||
| .tasks( | ||
| function( |
There was a problem hiding this comment.
Hi @matheusandre1, it is close. You need to test what you added here. Instead use the enrichOutputWithTaskTest workflow use that one you added on testDoTaskRunWorkflow test:
Workflow wf =
WorkflowBuilder.workflow("parentFlow")
.tasks(
d ->
d.workflow(
"runChild",
w ->
w.namespace("org.acme")
.name("childFlow")
.version("1.0.0")
.input(Map.of("id", 42, "region", "us-east"))
.await(false)
.returnType(RunTaskConfiguration.ProcessReturnType.NONE)))
.build();Signed-off-by: Matheus André Galvão Da Silva <[email protected]>
37add85 to
b607231
Compare
| Long.class) | ||
| .outputAs( | ||
| (object, workflowContext, taskContextData) -> { | ||
| Long taskOutput = output(taskContextData, Long.class); |
There was a problem hiding this comment.
actually the object passed to the task is the task ouput, so you should write (Long taskOutput, worlflowContext, taskContextData) and avoid the call to output(taskContextData, Long.class);`
@mcruzdev please make sure the usage docs CLEARLY reflec this
There was a problem hiding this comment.
The documentation update makes sense, we can do it in another pull request.
I think we need to remove this test, it is duplicated, there is another test that do the same thing in the codebase (@matheusandre1 please remove it).
| (object, workflowContext, taskContextData) -> { | ||
| Long taskOutput = output(taskContextData, Long.class); | ||
| softly.assertThat(taskOutput).isEqualTo(15L); | ||
| Long input = input(workflowContext, Long.class); |
There was a problem hiding this comment.
This is fine, althoug kind of weird, but I guess make sense in a test
Many thanks for submitting your Pull Request ❤️!
What this PR does / why we need it:
Closes: #1151
Special notes for reviewers:
Additional information (if needed):