Fix bug on workflow having join activity after a FormRepliedEvent

finos · Oct 6, 2022 · c8205c0 · c8205c0
1 parent 0cb1134
commit c8205c0
Show file tree

Hide file tree

Showing 5 changed files with 102 additions and 19 deletions.
diff --git a/...ot-app/src/main/java/com/symphony/bdk/workflow/engine/camunda/bpmn/BpmnBuilderHelper.java b/...ot-app/src/main/java/com/symphony/bdk/workflow/engine/camunda/bpmn/BpmnBuilderHelper.java
@@ -3,13 +3,15 @@
 import com.symphony.bdk.workflow.engine.WorkflowDirectGraph;
 import com.symphony.bdk.workflow.engine.WorkflowNodeType;
 
+import lombok.experimental.UtilityClass;
 import org.camunda.bpm.model.bpmn.builder.AbstractFlowNodeBuilder;
 import org.camunda.bpm.model.bpmn.builder.AbstractGatewayBuilder;
 import org.camunda.bpm.model.bpmn.builder.SubProcessBuilder;
 
 /**
  * Helper class on checks or common actions
  */
+@UtilityClass
 public class BpmnBuilderHelper {
 
   public static AbstractFlowNodeBuilder<?, ?> endEventSubProcess(BuildProcessContext context,

diff --git a/...t-app/src/main/java/com/symphony/bdk/workflow/engine/camunda/bpmn/CamundaBpmnBuilder.java b/...t-app/src/main/java/com/symphony/bdk/workflow/engine/camunda/bpmn/CamundaBpmnBuilder.java
@@ -216,11 +216,25 @@ private void computeChildren(WorkflowNode currentNode, AbstractFlowNodeBuilder<?
     return builder;
   }
 
+  @SuppressWarnings("checkstyle:JavadocTagContinuationIndentation")
   private void leafNode(String currentNodeId, AbstractFlowNodeBuilder<?, ?> camundaBuilder,
       BuildProcessContext context) {
-    camundaBuilder = camundaBuilder.endEvent();
-    context.addNodeBuilder(currentNodeId, camundaBuilder);
-    context.addLastNodeBuilder(camundaBuilder);
+    // if builder is an instance of sub process builder, the node should be already ended {@see SignalNodeBuilder#31},
+    // skip the ending
+    /*
+     * on:
+     *   one-of:
+     *     - form-replied:
+     *         form-id: init
+     *         exclusive: true
+     *     - message-received:
+     *         content: hey
+     */
+    if (!(camundaBuilder instanceof SubProcessBuilder)) {
+      camundaBuilder = camundaBuilder.endEvent();
+      context.addNodeBuilder(currentNodeId, camundaBuilder);
+      context.addLastNodeBuilder(camundaBuilder);
+    }
   }
 
   private boolean hasFormRepliedEvent(BuildProcessContext context, NodeChildren currentNodeChildren) {

diff --git a/...n/java/com/symphony/bdk/workflow/engine/camunda/bpmn/builder/AbstractNodeBpmnBuilder.java b/...n/java/com/symphony/bdk/workflow/engine/camunda/bpmn/builder/AbstractNodeBpmnBuilder.java
@@ -4,13 +4,14 @@
 import static com.symphony.bdk.workflow.engine.camunda.bpmn.CamundaBpmnBuilder.EXCLUSIVE_GATEWAY_SUFFIX;
 
 import com.symphony.bdk.workflow.engine.WorkflowNode;
-import com.symphony.bdk.workflow.engine.camunda.bpmn.BpmnBuilderHelper;
 import com.symphony.bdk.workflow.engine.camunda.bpmn.BuildProcessContext;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
 import org.camunda.bpm.model.bpmn.builder.AbstractFlowNodeBuilder;
 import org.camunda.bpm.model.bpmn.builder.AbstractGatewayBuilder;
-import org.camunda.bpm.model.bpmn.builder.SubProcessBuilder;
+
+import java.util.List;
+import java.util.stream.Collectors;
 
 public abstract class AbstractNodeBpmnBuilder implements WorkflowNodeBpmnBuilder {
 
@@ -34,6 +35,11 @@ public abstract class AbstractNodeBpmnBuilder implements WorkflowNodeBpmnBuilder
       // then connect the activity
       if (context.hasEventSubProcess() && context.getParents(element.getId()).size() > 1) {
         builder = endEventSubProcess(context, builder);
+        // since the sub process is ended here, and the child node is going to be connected by this ended sub process,
+        // therefore, all other branches remove their same child and are going to be ended inside the sub process.
+        List<String> parents =
+            context.getParents(element.getId()).stream().filter(k -> !k.equals(parentId)).collect(Collectors.toList());
+        parents.forEach(parent -> context.getChildren(parent).removeChild(element.getId()));
       }
       return build(element, parentId, builder, context);
     }
@@ -58,22 +64,9 @@ public abstract class AbstractNodeBpmnBuilder implements WorkflowNodeBpmnBuilder
   }
 
   protected void connectToExistingNode(String nodeId, AbstractFlowNodeBuilder<?, ?> builder) {
-    // if builder is an instance of sub process builder, the node should be already connected, skip the connection
-    /*
-     * on:
-     *   one-of:
-     *     - form-replied:
-     *         form-id: init
-     *         exclusive: true
-     *     - message-received:
-     *         content: hey
-     */
-    if (!(builder instanceof SubProcessBuilder)) {
-      builder.connectTo(nodeId);
-    }
+    builder.connectTo(nodeId);
   }
 
   protected abstract AbstractFlowNodeBuilder<?, ?> build(WorkflowNode element, String parentId,
       AbstractFlowNodeBuilder<?, ?> builder, BuildProcessContext context) throws JsonProcessingException;
-
 }
diff --git a/workflow-bot-app/src/test/java/com/symphony/bdk/workflow/FormReplyIntegrationTest.java b/workflow-bot-app/src/test/java/com/symphony/bdk/workflow/FormReplyIntegrationTest.java
@@ -23,6 +23,9 @@
 import org.apache.commons.lang3.StringEscapeUtils;
 import org.junit.jupiter.api.Test;
 import org.mockito.ArgumentCaptor;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.ValueSource;
 
 import java.util.Collections;
 import java.util.concurrent.TimeUnit;
@@ -405,4 +408,28 @@ void formRepliedSendMessageOnConditionElse() throws Exception {
 
     assertThat(workflow).executed(workflow, "testForm", "resMenu", "finish");
   }
+
+  @ParameterizedTest
+  @CsvSource(value = {"GOOG,response0", "GOOGLE,response1"})
+  void formReplied_fork_condition_join_activity(String tickerValue, String expectedActivity) throws Exception {
+    Workflow workflow =
+        SwadlParser.fromYaml(getClass().getResourceAsStream("/form/send-form-reply-join-activity.swadl.yaml"));
+
+    when(messageService.send(anyString(), any(Message.class))).thenReturn(message("msgId"));
+
+    engine.deploy(workflow);
+
+    // trigger workflow execution
+    engine.onEvent(messageReceived("/go"));
+    verify(messageService, timeout(2000)).send(anyString(), contains("form"));
+    clearInvocations(messageService);
+
+    await().atMost(5, TimeUnit.SECONDS).ignoreExceptions().until(() -> {
+      engine.onEvent(form("msgId", "sendForm", Collections.singletonMap("ticker", tickerValue)));
+      return true;
+    });
+    verify(messageService, timeout(2000)).send(anyString(), contains("END"));
+
+    assertThat(workflow).executed(workflow, "sendForm", expectedActivity, "response2");
+  }
 }
diff --git a/workflow-bot-app/src/test/resources/form/send-form-reply-join-activity.swadl.yaml b/workflow-bot-app/src/test/resources/form/send-form-reply-join-activity.swadl.yaml
@@ -0,0 +1,47 @@
+id: condition-form-reply-join
+activities:
+  - send-message:
+      id: sendForm
+      on:
+        message-received:
+          content: /go
+      content: |
+        <messageML>
+          <form id="sendForm">
+            <text-field name="ticker" placeholder="Please enter the Stock Ticker here"></text-field>
+            <textarea name="content" placeholder="Please enter your Research Content here and then Submit form."></textarea>
+            <button name="send-answers" type="action">Send</button>
+          </form>
+        </messageML>
+  - send-message:
+      id: response0
+      on:
+        form-replied:
+          form-id: sendForm
+      if: ${sendForm.ticker == 'GOOG'}
+      content: |
+        <messageML>
+          First reply (if): ${sendForm.content}
+        </messageML>
+  - send-message:
+      id: response1
+      on:
+        form-replied:
+          form-id: sendForm
+      else: {}
+      content: |
+        <messageML>
+          First reply (else): ${sendForm.content}
+        </messageML>
+  - send-message:
+      id: response2
+      on:
+        one-of:
+          - activity-completed:
+              activity-id: response0
+          - activity-completed:
+              activity-id: response1
+      content: |
+        <messageML>
+          END
+        </messageML>