fix(core): flowable failed to display a graph (#2120)

due to jackson
kestra-io · Sep 15, 2023 · 5669018 · 5669018
1 parent 5fc293e
commit 5669018
Show file tree

Hide file tree

Showing 7 changed files with 32 additions and 16 deletions.
diff --git a/core/src/main/java/io/kestra/core/models/hierarchies/AbstractGraph.java b/core/src/main/java/io/kestra/core/models/hierarchies/AbstractGraph.java
@@ -33,4 +33,12 @@ public AbstractGraph(String uid) {
     public String getLabel() {
         return this.getUid();
     }
+
+    public void updateUidWithChildren(String uid) {
+        this.uid = uid;
+    }
+
+    public void updateErrorWithChildren(boolean error) {
+        this.error = error;
+    }
 }
diff --git a/core/src/main/java/io/kestra/core/models/hierarchies/GraphCluster.java b/core/src/main/java/io/kestra/core/models/hierarchies/GraphCluster.java
@@ -69,7 +69,7 @@ public void addNode(AbstractGraph node) {
 
     public void addNode(AbstractGraph node, boolean withClusterUidPrefix) {
         if (withClusterUidPrefix) {
-            node.setUid(prefixedUid(node.uid));
+            node.updateUidWithChildren(prefixedUid(node.uid));
         }
         this.getGraph().addNode(node);
     }
@@ -100,25 +100,23 @@ public Map<GraphCluster, List<AbstractGraph>> allNodesByParent() {
 
     @Override
     public String getUid() {
-        return "cluster_" + super.getUid();
+        return "cluster_" + super.getUid().replace("cluster_", "");
     }
 
     @Override
-    @JsonIgnore
-    public void setUid(String uid) {
+    public void updateUidWithChildren(String uid) {
         graph.nodes().stream().filter(node ->
                 // filter other clusters' root & end to prevent setting uid multiple times
                 // this is because we need other clusters' root & end to have edges over them, but they are already managed by their own cluster
                 (!(node instanceof GraphClusterRoot) && !(node instanceof GraphClusterEnd))
                 || node.equals(this.root) || node.equals(this.end))
-            .forEach(node -> node.setUid(uid + node.uid.substring(this.uid.length())));
+            .forEach(node -> node.updateUidWithChildren(uid + node.uid.substring(this.uid.length())));
 
-        super.setUid(uid);
+        super.updateUidWithChildren(uid);
     }
 
     @Override
-    @JsonIgnore
-    public void setError(boolean error) {
+    public void updateErrorWithChildren(boolean error) {
         this.error = error;
 
         this.taskNode.error = error;

diff --git a/core/src/main/java/io/kestra/core/services/GraphService.java b/core/src/main/java/io/kestra/core/services/GraphService.java
@@ -95,7 +95,7 @@ public void replace() {
             parentCluster.getGraph().removeNode(taskToReplace);
 
             if (taskToReplace.isError()) {
-                clusterForReplacement.setError(true);
+                clusterForReplacement.updateErrorWithChildren(true);
             }
         }
     }

diff --git a/core/src/main/java/io/kestra/core/utils/GraphUtils.java b/core/src/main/java/io/kestra/core/utils/GraphUtils.java
@@ -280,7 +280,7 @@ private static void fillGraph(
                 graph.addNode(currentGraph);
 
                 if (relationType == RelationType.ERROR) {
-                    currentGraph.setError(true);
+                    currentGraph.updateErrorWithChildren(true);
                     if (isFirst) {
                         previous = graph.getRoot();
                     }

diff --git a/core/src/test/java/io/kestra/core/models/hierarchies/FlowGraphTest.java b/core/src/test/java/io/kestra/core/models/hierarchies/FlowGraphTest.java
@@ -157,9 +157,9 @@ void allFlowable() throws IllegalVariableEvaluationException {
         Flow flow = this.parse("flows/valids/all-flowable.yaml");
         FlowGraph flowGraph = GraphUtils.flowGraph(flow, null);
 
-        assertThat(flowGraph.getNodes().size(), is(34));
-        assertThat(flowGraph.getEdges().size(), is(37));
-        assertThat(flowGraph.getClusters().size(), is(6));
+        assertThat(flowGraph.getNodes().size(), is(38));
+        assertThat(flowGraph.getEdges().size(), is(42));
+        assertThat(flowGraph.getClusters().size(), is(7));
     }
 
     @Test

diff --git a/core/src/test/resources/flows/valids/all-flowable.yaml b/core/src/test/resources/flows/valids/all-flowable.yaml
@@ -17,6 +17,13 @@ tasks:
           - id: 1-each-2-1
             type: io.kestra.core.tasks.debugs.Return
             format: "{{task.id}} > {{taskrun.value}}"
+    errors:
+      - id: 1-each-seq-error
+        type: io.kestra.core.tasks.flows.Sequential
+        tasks:
+          - id: 1-each-seq-error-1
+            type: io.kestra.core.tasks.debugs.Return
+            format: "error {{task.id}} > {{taskrun.value}}"
 
 
   # Simple task

diff --git a/webserver/src/test/java/io/kestra/webserver/controllers/FlowControllerTest.java b/webserver/src/test/java/io/kestra/webserver/controllers/FlowControllerTest.java
@@ -106,9 +106,12 @@ void graph() {
         FlowGraph result = client.toBlocking().retrieve(HttpRequest.GET("/api/v1/flows/io.kestra" +
             ".tests/all-flowable/graph"), FlowGraph.class);
 
-        assertThat(result.getNodes().size(), is(34));
-        assertThat(result.getEdges().size(), is(37));
-        assertThat(result.getClusters().size(), is(6));
+        assertThat(result.getNodes().size(), is(38));
+        assertThat(result.getEdges().size(), is(42));
+        assertThat(result.getClusters().size(), is(7));
+        assertThat(result.getClusters().stream().map(FlowGraph.Cluster::getCluster).toList(), Matchers.everyItem(
+            Matchers.hasProperty("uid", Matchers.not(Matchers.startsWith("cluster_cluster_")))
+        ));
     }
 
     @Test