Skip to content

Commit e5e2147

Browse files
fjtiradofjtirado
authored
[Fix #1414] Use try rather than do as position name (#1432)
* [Fix #1414] Use try rather than do as position name Signed-off-by: fjtirado <ftirados@ibm.com> * [Fix #1414] Gonzalos comments Signed-off-by: fjtirado <ftirados@ibm.com> --------- Signed-off-by: fjtirado <ftirados@ibm.com>
1 parent 5b201fb commit e5e2147

4 files changed

Lines changed: 90 additions & 36 deletions

File tree

‎impl/core/src/main/java/io/serverlessworkflow/impl/executors/TaskExecutorHelper.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,16 @@ public static TaskExecutor<?> createExecutorList(
6161
WorkflowMutablePosition position,
6262
List<TaskItem> taskItems,
6363
WorkflowDefinition workflowDefinition) {
64+
return createExecutorList(position, taskItems, workflowDefinition, "do");
65+
}
66+
67+
public static TaskExecutor<?> createExecutorList(
68+
WorkflowMutablePosition position,
69+
List<TaskItem> taskItems,
70+
WorkflowDefinition workflowDefinition,
71+
String positionPrefix) {
6472
Map<String, TaskExecutorBuilder<?>> executors =
65-
createExecutorBuilderList(position, taskItems, workflowDefinition, "do");
73+
createExecutorBuilderList(position, taskItems, workflowDefinition, positionPrefix);
6674
executors.values().forEach(t -> t.connect(executors));
6775
Iterator<TaskExecutorBuilder<?>> iter = executors.values().iterator();
6876
TaskExecutor<?> first = iter.next().build();

‎impl/core/src/main/java/io/serverlessworkflow/impl/executors/TryExecutor.java‎

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import io.serverlessworkflow.impl.executors.retry.RetryExecutor;
4040
import io.serverlessworkflow.impl.executors.retry.RetryIntervalFunction;
4141
import java.util.List;
42+
import java.util.Objects;
4243
import java.util.Optional;
4344
import java.util.concurrent.CompletableFuture;
4445
import java.util.concurrent.CompletionException;
@@ -67,28 +68,23 @@ public static class TryExecutorBuilder extends RegularTaskExecutorBuilder<TryTas
6768
protected TryExecutorBuilder(
6869
WorkflowMutablePosition position, TryTask task, WorkflowDefinition definition) {
6970
super(position, task, definition);
70-
TryTaskCatch catchInfo = task.getCatch();
71+
TryTaskCatch catchInfo =
72+
Objects.requireNonNull(task.getCatch(), "Catch property is mandatory for Try task");
7173
this.errorFilter = buildErrorFilter(catchInfo.getErrors());
7274
this.whenFilter = WorkflowUtils.optionalPredicate(application, catchInfo.getWhen());
7375
this.exceptFilter = WorkflowUtils.optionalPredicate(application, catchInfo.getExceptWhen());
76+
this.errorVariable = catchInfo.getAs();
77+
List<TaskItem> catchTaskDo = catchInfo.getDo();
78+
this.catchTaskExecutor =
79+
catchTaskDo != null && !catchTaskDo.isEmpty()
80+
? Optional.of(
81+
TaskExecutorHelper.createExecutorList(
82+
position.copy().addProperty("catch"), catchTaskDo, definition))
83+
: Optional.empty();
84+
Retry retry = catchInfo.getRetry();
85+
this.retryIntervalExecutor = retry != null ? buildRetryInterval(retry) : Optional.empty();
7486
this.taskExecutor =
75-
TaskExecutorHelper.createExecutorList(position, task.getTry(), definition);
76-
TryTaskCatch catchTask = task.getCatch();
77-
if (catchTask != null) {
78-
this.errorVariable = catchTask.getAs();
79-
List<TaskItem> catchTaskDo = catchTask.getDo();
80-
this.catchTaskExecutor =
81-
catchTaskDo != null && !catchTaskDo.isEmpty()
82-
? Optional.of(
83-
TaskExecutorHelper.createExecutorList(position, catchTaskDo, definition))
84-
: Optional.empty();
85-
86-
Retry retry = catchTask.getRetry();
87-
this.retryIntervalExecutor = retry != null ? buildRetryInterval(retry) : Optional.empty();
88-
} else {
89-
this.catchTaskExecutor = Optional.empty();
90-
this.retryIntervalExecutor = Optional.empty();
91-
}
87+
TaskExecutorHelper.createExecutorList(position, task.getTry(), definition, "try");
9288
}
9389

9490
private Optional<RetryExecutor> buildRetryInterval(Retry retry) {

‎impl/test/src/test/java/io/serverlessworkflow/impl/test/RetryTimeoutTest.java‎

Lines changed: 45 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,18 @@
2020
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2121

2222
import com.fasterxml.jackson.databind.JsonNode;
23-
import io.serverlessworkflow.api.types.TryTask;
23+
import io.serverlessworkflow.impl.TaskContextData;
2424
import io.serverlessworkflow.impl.WorkflowApplication;
2525
import io.serverlessworkflow.impl.WorkflowException;
2626
import io.serverlessworkflow.impl.WorkflowModel;
2727
import io.serverlessworkflow.impl.jackson.JsonUtils;
2828
import io.serverlessworkflow.impl.lifecycle.TaskCompletedEvent;
29+
import io.serverlessworkflow.impl.lifecycle.TaskEvent;
2930
import io.serverlessworkflow.impl.lifecycle.TaskRetriedEvent;
30-
import io.serverlessworkflow.impl.lifecycle.TraceExecutionListener;
3131
import io.serverlessworkflow.impl.lifecycle.WorkflowExecutionListener;
3232
import java.io.IOException;
3333
import java.time.Duration;
3434
import java.util.Map;
35-
import java.util.Set;
3635
import java.util.concurrent.CompletableFuture;
3736
import java.util.concurrent.ConcurrentHashMap;
3837
import okhttp3.mockwebserver.MockResponse;
@@ -55,11 +54,7 @@ void setUp() throws IOException {
5554
apiServer = new MockWebServer();
5655
apiServer.start(9797);
5756
retryListener = new RetryListener();
58-
app =
59-
WorkflowApplication.builder()
60-
.withListener(retryListener)
61-
.withListener(new TraceExecutionListener())
62-
.build();
57+
app = WorkflowApplication.builder().withListener(retryListener).build();
6358
}
6459

6560
@AfterEach
@@ -71,16 +66,21 @@ void tearDown() throws IOException {
7166
private class RetryListener implements WorkflowExecutionListener {
7267

7368
private Map<String, Short> taskRetried = new ConcurrentHashMap<>();
74-
private Set<Short> contexts = ConcurrentHashMap.newKeySet();
69+
private Map<String, Short> taskCompleted = new ConcurrentHashMap<>();
7570

71+
@Override
7672
public void onTaskRetried(TaskRetriedEvent ev) {
77-
taskRetried.put(ev.taskContext().position().jsonPointer(), ev.taskContext().retryAttempt());
73+
add2Map(taskRetried, ev);
7874
}
7975

76+
@Override
8077
public void onTaskCompleted(TaskCompletedEvent ev) {
81-
if (ev.taskContext().task() instanceof TryTask) {
82-
contexts.add(ev.taskContext().retryAttempt());
83-
}
78+
add2Map(taskCompleted, ev);
79+
}
80+
81+
private static void add2Map(Map<String, Short> map, TaskEvent ev) {
82+
TaskContextData taskContext = ev.taskContext();
83+
map.put(taskContext.position().jsonPointer(), taskContext.retryAttempt());
8484
}
8585
}
8686

@@ -107,8 +107,7 @@ void testRetry(String path) throws IOException {
107107
.atMost(Duration.ofSeconds(1))
108108
.until(() -> future.join().as(JsonNode.class).orElseThrow().equals(result));
109109
assertThat(retryListener.taskRetried).hasSize(1);
110-
assertThat(retryListener.taskRetried.get("do/0/tryGetPet/do/0/getPet")).isEqualTo((short) 2);
111-
assertThat(retryListener.contexts).containsOnly((short) 0);
110+
assertThat(retryListener.taskRetried.get("do/0/tryGetPet/try/0/getPet")).isEqualTo((short) 2);
112111
}
113112

114113
@Test
@@ -135,8 +134,37 @@ void testNestedRetry() throws IOException {
135134
.atMost(Duration.ofSeconds(1))
136135
.until(() -> future.join().as(JsonNode.class).orElseThrow().equals(result));
137136
assertThat(retryListener.taskRetried).hasSize(2);
138-
assertThat(retryListener.taskRetried.values()).containsExactlyInAnyOrder((short) 5, (short) 2);
139-
assertThat(retryListener.contexts).containsExactlyInAnyOrder((short) 0, (short) 2);
137+
assertThat(retryListener.taskRetried.get("do/0/tryServerError/try/0/tryCommunication/try"))
138+
.isEqualTo((short) 2);
139+
assertThat(
140+
retryListener.taskRetried.get(
141+
"do/0/tryServerError/try/0/tryCommunication/try/0/getPet"))
142+
.isEqualTo((short) 5);
143+
assertThat(retryListener.taskCompleted.get("do/0/tryServerError/try/0/tryCommunication/try"))
144+
.isEqualTo((short) 2);
145+
assertThat(retryListener.taskCompleted.get("do/0/tryServerError/try")).isEqualTo((short) 0);
146+
}
147+
148+
@Test
149+
void testRetryDo() throws IOException {
150+
CompletableFuture<WorkflowModel> future =
151+
app.workflowDefinition(
152+
readWorkflowFromClasspath("workflows-samples/try-catch-with-do.yaml"))
153+
.instance(Map.of("delay", 0.01))
154+
.start();
155+
Awaitility.await()
156+
.atMost(Duration.ofSeconds(1))
157+
.until(
158+
() ->
159+
future
160+
.join()
161+
.asMap()
162+
.orElseThrow()
163+
.equals(Map.of("setAfterFailingTask", "No problem")));
164+
165+
assertThat(retryListener.taskCompleted.get("do/0/attemptTask/try")).isEqualTo((short) 0);
166+
assertThat(retryListener.taskCompleted)
167+
.containsKey("do/0/attemptTask/catch/do/0/executeAfterFailingTask");
140168
}
141169

142170
@Test

‎impl/test/src/test/resources/workflows-samples/try-catch-with-do.yaml‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
document:
2+
dsl: '1.0.0-alpha5'
3+
namespace: test
4+
name: try-catch-with-do
5+
version: '0.1.0'
6+
do:
7+
- attemptTask:
8+
try:
9+
- failingTask:
10+
raise:
11+
error:
12+
type: https://example.com/errors/runtime
13+
status: 500
14+
catch:
15+
errors:
16+
with:
17+
type: https://example.com/errors/runtime
18+
status: 500
19+
do:
20+
- executeAfterFailingTask:
21+
set:
22+
setAfterFailingTask: "No problem"

0 commit comments

Comments
 (0)