fix #1252: Add predictable naming to tasks instead of UUIDs#1253
fix #1252: Add predictable naming to tasks instead of UUIDs#1253fjtirado merged 3 commits intoserverlessworkflow:mainfrom
Conversation
…of UUIDs Signed-off-by: Ricardo Zanini <ricardozanini@gmail.com>
There was a problem hiding this comment.
Pull request overview
Updates the Java fluent DSL to generate deterministic task names when callers pass null/blank names (and when using name-less overloads), replacing previously random UUID-based naming to improve debuggability and testability (Issue #1252).
Changes:
- Replace UUID defaults in fluent SPI convenience overloads with
nullso builders can apply deterministic naming. - Implement deterministic auto-naming in task list builders via a
taskType-indexscheme. - Add a new JUnit test suite covering auto-naming across top-level and nested task builders (try/forEach/fork).
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| fluent/spec/src/test/java/io/serverlessworkflow/fluent/spec/TaskItemDefaultNamingTest.java | Adds unit tests asserting deterministic default naming for unnamed tasks (including nested builders). |
| fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/spi/TryCatchFluent.java | Changes default overload to pass null (enabling deterministic naming downstream). |
| fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/spi/SwitchTaskFluent.java | Changes default overload to pass null for switch item names. |
| fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/spi/SwitchFluent.java | Changes default overload to pass null (enabling deterministic naming downstream). |
| fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/spi/SetFluent.java | Changes default overloads to pass null (enabling deterministic naming downstream). |
| fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/spi/RaiseFluent.java | Changes default overload to pass null (enabling deterministic naming downstream). |
| fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/spi/ListenFluent.java | Changes default overload to pass null (enabling deterministic naming downstream). |
| fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/spi/ForkFluent.java | Changes default overload to pass null (enabling deterministic naming downstream). |
| fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/spi/ForEachFluent.java | Changes default overload to pass null (enabling deterministic naming downstream). |
| fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/spi/EmitFluent.java | Changes default overload to pass null (enabling deterministic naming downstream). |
| fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/spi/CallOpenAPIFluent.java | Changes default overload to pass null (enabling deterministic naming downstream). |
| fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/spi/CallHttpFluent.java | Changes default overload to pass null (enabling deterministic naming downstream). |
| fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/TaskItemListBuilder.java | Plumbs task type into default-name helper for each task kind. |
| fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/BaseTaskItemListBuilder.java | Introduces deterministic naming helper (taskType + "-" + list.size()) and task type constants. |
| experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/spi/CallFnFluent.java | Changes default overload to pass null (enabling deterministic naming downstream). |
| experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/FuncTaskItemListBuilder.java | Updates func DSL list builder to use deterministic naming helper (adds TYPE_FUNCTION). |
| experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/FuncSwitchTaskBuilder.java | Adds deterministic naming for onPredicate(...) when name is null/blank. |
| experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/FuncForkTaskBuilder.java | Adds deterministic naming for unnamed fork branches. |
| experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/FuncForTaskBuilder.java | Adds deterministic naming for unnamed loop tasks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fluent/spec/src/test/java/io/serverlessworkflow/fluent/spec/TaskItemDefaultNamingTest.java
Outdated
Show resolved
Hide resolved
fluent/spec/src/test/java/io/serverlessworkflow/fluent/spec/TaskItemDefaultNamingTest.java
Outdated
Show resolved
Hide resolved
...ental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/FuncSwitchTaskBuilder.java
Outdated
Show resolved
Hide resolved
fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/spi/SwitchTaskFluent.java
Show resolved
Hide resolved
fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/BaseTaskItemListBuilder.java
Outdated
Show resolved
Hide resolved
fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/BaseTaskItemListBuilder.java
Show resolved
Hide resolved
Signed-off-by: Ricardo Zanini <ricardozanini@gmail.com>
Signed-off-by: Ricardo Zanini <ricardozanini@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/FuncForkTaskBuilder.java:66
- Deterministic default naming for
branch(...)is new behavior (branch-<index>when name is null/blank) but there are no tests asserting this. Adding a small unit test in the func DSL test suite would help prevent regressions and verify the intended numbering/format, especially when mixingbranch(...)andbranches(...)calls.
public <T, V> FuncForkTaskBuilder branch(
String name, Function<T, V> function, Class<T> argParam) {
if (name == null || name.isBlank()) {
name = "branch-" + this.items.size();
}
this.items.add(
new TaskItem(
name,
new Task().withCallTask(new CallTaskJava(CallJava.function(function, argParam)))));
return this;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fluent/spec/src/test/java/io/serverlessworkflow/fluent/spec/TaskItemDefaultNamingTest.java
Show resolved
Hide resolved
...rimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/FuncForTaskBuilder.java
Show resolved
Hide resolved
| protected final String defaultNameAndRequireConfig( | ||
| String name, Consumer<?> cfg, String taskType) { | ||
| Objects.requireNonNull(cfg, "Configurer must not be null"); | ||
|
|
||
| if (name == null || name.isBlank()) { | ||
| name = UUID.randomUUID().toString(); | ||
| return taskType + "-" + this.list.size(); | ||
| } | ||
| Objects.requireNonNull(cfg, "Configurer must not be null"); | ||
|
|
||
| return name; |
There was a problem hiding this comment.
The auto-generated name uses this.list.size() from the current list builder instance. Since WorkflowBuilder.tasks(...) can be invoked multiple times and each call creates a new DoTaskBuilder / TaskItemListBuilder with an empty list, auto-naming will restart at *-0 on each invocation. That can produce duplicate task names in the final workflow, which will break downstream execution/visualization logic that keys by task name (e.g., maps keyed by item.getName()). Consider deriving the index from the merged workflow-level task list (or maintaining a workflow-scoped counter / offset) so names remain unique across multiple .tasks(...) appends.
Many thanks for submitting your Pull Request ❤️!
What this PR does / why we need it:
Fixes #1252
Special notes for reviewers:
Additional information (if needed):