Skip to content

Commit 45ef451

Browse files
committed
[Fix #1414] Gonzalos comments
Signed-off-by: fjtirado <ftirados@ibm.com>
1 parent df68b6f commit 45ef451

2 files changed

Lines changed: 30 additions & 28 deletions

File tree

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

Lines changed: 13 additions & 17 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 =
7587
TaskExecutorHelper.createExecutorList(position, task.getTry(), definition, "try");
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.copy(), 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-
}
9288
}
9389

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

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

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,13 @@
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;
3031
import io.serverlessworkflow.impl.lifecycle.WorkflowExecutionListener;
3132
import java.io.IOException;
@@ -65,17 +66,21 @@ void tearDown() throws IOException {
6566
private class RetryListener implements WorkflowExecutionListener {
6667

6768
private Map<String, Short> taskRetried = new ConcurrentHashMap<>();
68-
private Map<String, Short> tryTaskCompleted = new ConcurrentHashMap<>();
69+
private Map<String, Short> taskCompleted = new ConcurrentHashMap<>();
6970

71+
@Override
7072
public void onTaskRetried(TaskRetriedEvent ev) {
71-
taskRetried.put(ev.taskContext().position().jsonPointer(), ev.taskContext().retryAttempt());
73+
add2Map(taskRetried, ev);
7274
}
7375

76+
@Override
7477
public void onTaskCompleted(TaskCompletedEvent ev) {
75-
if (ev.taskContext().task() instanceof TryTask) {
76-
tryTaskCompleted.put(
77-
ev.taskContext().position().jsonPointer(), ev.taskContext().retryAttempt());
78-
}
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());
7984
}
8085
}
8186

@@ -103,7 +108,6 @@ void testRetry(String path) throws IOException {
103108
.until(() -> future.join().as(JsonNode.class).orElseThrow().equals(result));
104109
assertThat(retryListener.taskRetried).hasSize(1);
105110
assertThat(retryListener.taskRetried.get("do/0/tryGetPet/try/0/getPet")).isEqualTo((short) 2);
106-
assertThat(retryListener.tryTaskCompleted.values()).containsOnly((short) 0);
107111
}
108112

109113
@Test
@@ -136,9 +140,9 @@ void testNestedRetry() throws IOException {
136140
retryListener.taskRetried.get(
137141
"do/0/tryServerError/try/0/tryCommunication/try/0/getPet"))
138142
.isEqualTo((short) 5);
139-
assertThat(retryListener.tryTaskCompleted.get("do/0/tryServerError/try/0/tryCommunication/try"))
143+
assertThat(retryListener.taskCompleted.get("do/0/tryServerError/try/0/tryCommunication/try"))
140144
.isEqualTo((short) 2);
141-
assertThat(retryListener.tryTaskCompleted.get("do/0/tryServerError/try")).isEqualTo((short) 0);
145+
assertThat(retryListener.taskCompleted.get("do/0/tryServerError/try")).isEqualTo((short) 0);
142146
}
143147

144148
@Test
@@ -158,7 +162,9 @@ void testRetryDo() throws IOException {
158162
.orElseThrow()
159163
.equals(Map.of("setAfterFailingTask", "No problem")));
160164

161-
assertThat(retryListener.tryTaskCompleted.get("do/0/attemptTask/try")).isEqualTo((short) 0);
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");
162168
}
163169

164170
@Test

0 commit comments

Comments
 (0)