Introduce taskInput, taskOutput and workflowInput methods#1197
Introduce taskInput, taskOutput and workflowInput methods#1197mcruzdev wants to merge 2 commits intoserverlessworkflow:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds DSL helpers to simplify enriching inputFrom(...) and outputAs(...) transformations with access to the root workflow input (WorkflowModel), addressing #1196 by reducing boilerplate when reading workflowContext.instanceData().input().
Changes:
- Added
FuncDSL.enriched(...)andFuncDSL.enrichedOutput(...)helper functions (including root-input-only variants). - Introduced
EnrichWithModelBiFunctionfunctional interface to support(typedValue, WorkflowModel)enrichment lambdas. - Added new integration-style tests and updated
impl/testMaven dependencies to compile/run them.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java |
Adds new enrichment helpers and their Javadocs. |
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/EnrichWithModelBiFunction.java |
New functional interface for enrichment with WorkflowModel. |
impl/test/src/test/java/io/serverlessworkflow/impl/test/FuncDSLEnrichWithTest.java |
New tests covering enriched input/output scenarios. |
impl/test/pom.xml |
Adds test-scope dependencies required by the new tests. |
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/Step.java |
Minor formatting-only changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
impl/test/src/test/java/io/serverlessworkflow/impl/test/FuncDSLEnrichWithTest.java
Outdated
Show resolved
Hide resolved
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
ricardozanini
left a comment
There was a problem hiding this comment.
Looks good, but please consider renaming to enrich instead and fwiw: https://www.enterpriseintegrationpatterns.com/patterns/messaging/DataEnricher.html
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
...uent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/EnrichWithModelBiFunction.java
Outdated
Show resolved
Hide resolved
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
impl/test/src/test/java/io/serverlessworkflow/impl/test/FuncDSLEnrichWithTest.java
Outdated
Show resolved
Hide resolved
impl/test/src/test/java/io/serverlessworkflow/impl/test/FuncDSLEnrichWithTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
impl/test/src/test/java/io/serverlessworkflow/impl/test/FuncDSLEnrichWithTest.java
Outdated
Show resolved
Hide resolved
|
See my comment on the issue, I think the PR needs to be refactor accordingly |
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
impl/test/src/test/java/io/serverlessworkflow/impl/test/FuncDSLEnrichWithTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
impl/test/src/test/java/io/serverlessworkflow/impl/test/FuncDSLEnrichWithTest.java
Outdated
Show resolved
Hide resolved
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Show resolved
Hide resolved
| class FuncDSLEnrichWithTest { | ||
|
|
||
| @Test | ||
| void test_input_with_input_from() { |
There was a problem hiding this comment.
Test method names in this module consistently use lower camelCase (e.g., testBasic, testConsoleLogWithArgs). These new tests use underscores (test_input_with_input_from, etc.) and one has inconsistent capitalization (test_enrich_Input...). Please rename them to match the existing convention for readability and consistency.
| void test_input_with_input_from() { | |
| void testInputWithInputFrom() { |
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Show resolved
Hide resolved
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Show resolved
Hide resolved
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Outdated
Show resolved
Hide resolved
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Show resolved
Hide resolved
| * @return the deserialized workflow input object of type T | ||
| */ | ||
| public static <T> T input(WorkflowContextData context, Class<T> inputClass) { | ||
| return context.instanceData().input().as(inputClass).orElseThrow(); |
There was a problem hiding this comment.
orElseThrow() here will throw a NoSuchElementException with no message, which is hard to debug from DSL usage. Consider throwing an IllegalStateException/IllegalArgumentException with a descriptive message that explains the workflow input is missing or cannot be deserialized into the requested type.
| return context.instanceData().input().as(inputClass).orElseThrow(); | |
| return context | |
| .instanceData() | |
| .input() | |
| .as(inputClass) | |
| .orElseThrow( | |
| () -> | |
| new IllegalStateException( | |
| "Workflow input is missing or cannot be deserialized into type " | |
| + inputClass.getName() | |
| + " when calling FuncDSL.input(WorkflowContextData, Class).")); |
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java
Show resolved
Hide resolved
| } | ||
|
|
||
| @Test | ||
| void test_enrich_Input_with_workflowInput_task() { |
There was a problem hiding this comment.
Test method name has inconsistent capitalization (test_enrich_Input...) compared to the other test methods in this class. Renaming it to use consistent lower_snake_case (or whatever naming pattern the file is following) will make the suite easier to scan and grep.
| void test_enrich_Input_with_workflowInput_task() { | |
| void test_enrich_input_with_workflow_input_task() { |
impl/test/src/test/java/io/serverlessworkflow/impl/test/FuncDSLEnrichWithTest.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Matheus Cruz <matheuscruz.dev@gmail.com>
Signed-off-by: Matheus Cruz <matheuscruz.dev@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| () -> | ||
| new IllegalStateException( | ||
| "Workflow output is missing or cannot be deserialized into type " | ||
| + outputClass.getName() | ||
| + " when calling FuncDSL.output(TaskContextData, Class<T>).")); |
There was a problem hiding this comment.
The exception message in FuncDSL.output(TaskContextData, …) says "Workflow output" but this helper deserializes a task output (TaskContextData.output()). Please change the message to "Task output" to avoid confusion in logs/stack traces.
ricardozanini
left a comment
There was a problem hiding this comment.
LGTM, there's just one minor comment from copilot that should be addressed.
This pull request add
inputandoutputmethods to DSL.Closes #1196