Keep in the model the information of which edges are reverted

This is required to be able to position shape

Also removed generated Ids from shape, there is no need to have generated ids as BPMN file already have id.

java/src/main/java/io/process/analytics/tools/bpmn/generator/algo/ShapeSorter.java

@@ -15,6 +15,8 @@
  */
 package io.process.analytics.tools.bpmn.generator.algo;
 
+import static io.process.analytics.tools.bpmn.generator.model.Edge.revertedEdge;
+
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -45,40 +47,46 @@ public class ShapeSorter {
      */
     public SortedDiagram sort(Diagram diagram) {
         Set<Shape> nodesToSort = new HashSet<>(diagram.getShapes());
-        Set<Edge> edges = new HashSet<>(diagram.getEdges());
-        List<Join> joins = findAllJoins(nodesToSort, edges);
+        Set<Edge> remainingEdges = new HashSet<>(diagram.getEdges());
+        Set<Edge> finalEdges = new HashSet<>(diagram.getEdges());
+        List<Join> joins = findAllJoins(nodesToSort, remainingEdges);
 
         SortedDiagram.SortedDiagramBuilder sortedDiagram = SortedDiagram.builder();
 
         while (!nodesToSort.isEmpty()) {
-            Set<Shape> startShapes = getStartNodes(nodesToSort, edges);
+            Set<Shape> startShapes = getStartNodes(nodesToSort, remainingEdges);
             if (!startShapes.isEmpty()) {
                 for (Shape startShape : startShapes) {
                     nodesToSort.remove(startShape);
                     sortedDiagram.shape(startShape);
-                    edges = removeEdgesStartingWithNode(edges, startShape);
-                    joins.stream().filter(j -> j.isJoining(startShape)).forEach(Join::markProcessed);
+                    remainingEdges = removeEdgesStartingWithNode(remainingEdges, startShape);
+                    joins.stream().filter(j -> j.isJoining(startShape)).forEach(j -> j.markProcessed(startShape));
                 }
             } else {
                 //retrieve a join that we "entered", i.e. we processed an element that has an edge going to this join.
                 Join join = getAJoinThatWasProcessed(joins);
                 //revert all edges from this join to remove the cycle
-                edges = edges.stream().map(edge -> {
-                    if (join.contains(edge)) {
-                        //revert the edge
-                        return new Edge(edge.getId(), edge.getTo(), edge.getFrom());
-                    } else {
-                        return edge;
-                    }
-                }).collect(Collectors.toSet());
+                remainingEdges = revertNonProcessedEdgeOfTheJoin(remainingEdges, join);
+                finalEdges = revertNonProcessedEdgeOfTheJoin(finalEdges, join);
             }
         }
-        return sortedDiagram.edges(diagram.getEdges()).build();
+        return sortedDiagram.edges(finalEdges).build();
+    }
+
+    private Set<Edge> revertNonProcessedEdgeOfTheJoin(Set<Edge> remainingEdges, Join join) {
+        return remainingEdges.stream().map(edge -> {
+            if (join.contains(edge) && !join.wasProcessed(edge.getFrom())) {
+                //revert the edge
+                return revertedEdge(edge);
+            } else {
+                return edge;
+            }
+        }).collect(Collectors.toSet());
     }
 
     private Join getAJoinThatWasProcessed(List<Join> joins) {
-        // we should always have a join that was processed when we don't have a start node
-        return joins.stream().filter(j -> j.wasProcessed).findFirst().get();
+        // we should always have a join that was processed at least once when we don't have a start node
+        return joins.stream().filter(j -> !j.getProcessedEdges().isEmpty()).findFirst().get();
     }
 
     private Set<Edge> removeEdgesStartingWithNode(Set<Edge> edges, Shape startShape) {
@@ -113,14 +121,19 @@ private static class Join {
         @Singular
         private Set<String> incomings;
         private final Shape to;
-        private boolean wasProcessed;
+        @Builder.Default
+        private Set<String> processedEdges = new HashSet<>();
 
         boolean isJoining(Shape shape) {
             return incomings.contains(shape.getId());
         }
 
-        void markProcessed() {
-            setWasProcessed(true);
+        void markProcessed(Shape shape) {
+            processedEdges.add(shape.getId());
+        }
+
+        boolean wasProcessed(String shapeId) {
+            return processedEdges.contains(shapeId);
         }
 
         boolean contains(Edge edge) {

java/src/main/java/io/process/analytics/tools/bpmn/generator/converter/BpmnToAlgoModelConverter.java

@@ -39,7 +39,7 @@ public Diagram toAlgoModel(TDefinitions definitions) {
                     .forEach(diagram::shape);
 
             bpmnElements.getSequenceFlows().stream()
-                    .map(seqFlow -> new Edge(seqFlow.getId(), getId(seqFlow.getSourceRef()), getId(seqFlow.getTargetRef())))
+                    .map(seqFlow -> Edge.edge(seqFlow.getId(), getId(seqFlow.getSourceRef()), getId(seqFlow.getTargetRef())))
                     .forEach(diagram::edge);
         }
 

java/src/main/java/io/process/analytics/tools/bpmn/generator/model/Edge.java

@@ -17,17 +17,34 @@
 
 import static io.process.analytics.tools.bpmn.generator.internal.IdUtils.generateRandomId;
 
+import lombok.Builder;
 import lombok.Data;
 
 @Data
+@Builder( toBuilder = true)
 public class Edge {
 
     private final String id; // the bpmnElement id
     private final String from;
     private final String to;
+    /**
+     * This property helps to break cycles
+     *
+     * `true` when the original edge is in the opposite direction
+     *
+     * When drown, it should be drown in the opposite direction.
+     * When positioning shapes, it should be kept reverted
+     */
+    private final boolean reverted;
 
+    public static Edge edge(String id, String from, String to) {
+        return new Edge(id, from, to, false);
+    }
     public static Edge edge(Shape from, Shape to) {
-        return new Edge(generateRandomId(), from.getId(), to.getId());
+        return new Edge(generateRandomId(), from.getId(), to.getId(), false);
+    }
+    public static Edge revertedEdge(Edge original) {
+        return original.toBuilder().from(original.getTo()).to(original.getFrom()).reverted(true).build();
     }
 
 }

java/src/main/java/io/process/analytics/tools/bpmn/generator/model/Shape.java

@@ -15,8 +15,6 @@
  */
 package io.process.analytics.tools.bpmn.generator.model;
 
-import static io.process.analytics.tools.bpmn.generator.internal.IdUtils.generateRandomId;
-
 import lombok.Builder;
 import lombok.Data;
 import lombok.RequiredArgsConstructor;
@@ -30,7 +28,7 @@ public class Shape {
     private final String name;
 
     public Shape(String name) {
-        this(generateRandomId(), name);
+        this(name, name);
     }
 
     public static Shape shape(String name) {

java/src/test/java/io/process/analytics/tools/bpmn/generator/algo/ShapeSorterTest.java

@@ -1,5 +1,7 @@
 package io.process.analytics.tools.bpmn.generator.algo;
 
+import static io.process.analytics.tools.bpmn.generator.model.Edge.edge;
+import static io.process.analytics.tools.bpmn.generator.model.Edge.revertedEdge;
 import static io.process.analytics.tools.bpmn.generator.model.Edge.edge;
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -19,6 +21,7 @@ class ShapeSorterTest {
     Shape step3 = new Shape("step3");
     Shape step4 = new Shape("step4");
     Shape step5 = new Shape("step5");
+    Shape end = new Shape("end");
 
     @Test
     void should_sort_2_shapes() {
@@ -78,7 +81,7 @@ void should_sort_shapes_with_split_and_join() {
     }
 
     @Test
-    void should_sort_shapes_with_loop() {
+    void should_sort_shapes_with_cycle() {
         // loop between step2, step3, step4
         //
         //                      -------------------------
@@ -106,4 +109,34 @@ void should_sort_shapes_with_loop() {
         assertThat(sorted.getShapes()).containsExactly(start, step1, step2, step3, step4, step5);
     }
 
+    @Test
+    public void should_revert_one_edge_of_a_cycle() {
+        Edge t1 = Edge.builder().id("t1").from(start.getId()).to(step1.getId()).build();
+        Edge t2 = Edge.builder().id("t2").from(step1.getId()).to(step2.getId()).build();
+        Edge t3 = Edge.builder().id("t3").from(step2.getId()).to(step3.getId()).build();
+        Edge t4 = Edge.builder().id("t4").from(step3.getId()).to(step1.getId()).build();
+        Edge t5 = Edge.builder().id("t5").from(step2.getId()).to(end.getId()).build();
+        Diagram diagram = Diagram.builder()
+                .shape(start)
+                .shape(step1)
+                .shape(step2)
+                .shape(step3)
+                .shape(end)
+                .edge(t1)
+                .edge(t2)
+                .edge(t3)
+                .edge(t4)
+                .edge(t5)
+                .build();
+
+        SortedDiagram sorted = shapeSorter.sort(diagram);
+
+        assertThat(sorted.getEdges()).containsOnly(
+                t1,
+                t2,
+                t3,
+                revertedEdge(t4),
+                t5);
+    }
+
 }

java/src/test/java/io/process/analytics/tools/bpmn/generator/converter/BpmnToAlgoModelConverterTest.java

@@ -12,16 +12,15 @@
  */
 package io.process.analytics.tools.bpmn.generator.converter;
 
+
 import io.process.analytics.tools.bpmn.generator.internal.generated.model.TDefinitions;
 import io.process.analytics.tools.bpmn.generator.model.Diagram;
-import io.process.analytics.tools.bpmn.generator.model.Edge;
 import io.process.analytics.tools.bpmn.generator.model.Shape;
-import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.Test;
 
 import static io.process.analytics.tools.bpmn.generator.internal.SemanticTest.definitionsFromBpmnFile;
+import static io.process.analytics.tools.bpmn.generator.model.Edge.edge;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.tuple;
 
 class BpmnToAlgoModelConverterTest {
 
@@ -49,8 +48,8 @@ public void convertBpmnFlowNodesIntoDiagram() {
                 new Shape("task_1", "Task 1"),
                 new Shape("endEvent_1", "End Event"));
         assertThat(diagram.getEdges()).containsOnly(
-                new Edge("sequenceFlow_1","startEvent_1", "task_1"),
-                new Edge("sequenceFlow_2","task_1", "endEvent_1"));
+                edge("sequenceFlow_1", "startEvent_1", "task_1"),
+                edge("sequenceFlow_2", "task_1", "endEvent_1"));
     }
 
 }