Merge pull request #633 from ICRAR/eagle-1046

Add check for re-used ids across nodes, edges and fields. Found a big…
ICRAR · Sep 6, 2023 · 0b90b84 · 0b90b84
2 parents 074b9ba + 0062516
commit 0b90b84
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 14 deletions.
diff --git a/src/Eagle.ts b/src/Eagle.ts
@@ -4509,21 +4509,29 @@ export class Eagle {
 
         this.logicalGraph().addNodeComplete(newNode);
 
-        // set new ids for any ports in this node
-        Utils.giveNodePortsNewIds(newNode);
+        // set new ids for any fields in this node
+        for (const field of newNode.getFields()){
+            field.setId(Utils.uuidv4());
+        }
 
         // set new keys for embedded applications within node, and new ids for ports within those embedded nodes
         if (newNode.hasInputApplication()){
             newNode.getInputApplication().setKey(Utils.newKey(this.logicalGraph().getNodes()));
             newNode.getInputApplication().setEmbedKey(newNode.getKey());
 
-            Utils.giveNodePortsNewIds(newNode.getInputApplication());
+            // set new ids for any fields in this node
+            for (const field of newNode.getInputApplication().getFields()){
+                field.setId(Utils.uuidv4());
+            }
         }
         if (newNode.hasOutputApplication()){
             newNode.getOutputApplication().setKey(Utils.newKey(this.logicalGraph().getNodes()));
             newNode.getOutputApplication().setEmbedKey(newNode.getKey());
 
-            Utils.giveNodePortsNewIds(newNode.getOutputApplication());
+            // set new ids for any fields in this node
+            for (const field of newNode.getOutputApplication().getFields()){
+                field.setId(Utils.uuidv4());
+            }
         }
 
         // flag that the logical graph has been modified

diff --git a/src/Edge.ts b/src/Edge.ts
@@ -61,6 +61,10 @@ export class Edge {
         return this._id;
     }
 
+    setId = (id: string) : void => {
+        this._id = id;
+    }
+
     getSrcNodeKey = () : number => {
         return this.srcNodeKey;
     }

diff --git a/src/Utils.ts b/src/Utils.ts
@@ -1312,16 +1312,6 @@ export class Utils {
         return result;
     }
 
-    static giveNodePortsNewIds(node: Node) : void {
-        // set new ids for any ports in this node
-        for (const port of node.getInputPorts()){
-            port.setId(Utils.uuidv4());
-        }
-        for (const port of node.getOutputPorts()){
-            port.setId(Utils.uuidv4());
-        }
-    }
-
     static determineFileType(data: any): Eagle.FileType {
         if (typeof data.modelData !== 'undefined'){
             if (typeof data.modelData.fileType !== 'undefined'){
@@ -1439,6 +1429,52 @@ export class Utils {
             Edge.isValid(eagle, edge.getId(), edge.getSrcNodeKey(), edge.getSrcPortId(), edge.getDestNodeKey(), edge.getDestPortId(), edge.getDataType(), edge.isLoopAware(), edge.isClosesLoop(), false, false, errorsWarnings);
         }
 
+        // check that all node, edge, field ids are unique
+        {
+            const ids : string[] = [];
+
+            // loop over graph nodes
+            for (const node of graph.getNodes()){
+                if (ids.includes(node.getId())){
+                    const issue: Errors.Issue = Errors.ShowFix(
+                        "Node (" + node.getName() + ") does not have a unique id",
+                        function(){Utils.showNode(eagle, Eagle.FileType.Graph, node.getId())},
+                        function(){Utils.newId(node)},
+                        "Assign node a new id"
+                    );
+                    errorsWarnings.errors.push(issue);
+                }
+                ids.push(node.getId());
+
+                for (const field of node.getFields()){
+                    if (ids.includes(field.getId())){
+                        const issue: Errors.Issue = Errors.ShowFix(
+                            "Field (" + field.getDisplayText() + ") on node (" + node.getName() + ") does not have a unique id",
+                            function(){Utils.showNode(eagle, Eagle.FileType.Graph, node.getId())},
+                            function(){Utils.newId(field)},
+                            "Assign field a new id"
+                        );
+                        errorsWarnings.errors.push(issue);
+                    }
+                    ids.push(field.getId());
+                }
+            }
+
+            // loop over graph edges
+            for (const edge of graph.getEdges()){
+                if (ids.includes(edge.getId())){
+                    const issue: Errors.Issue = Errors.ShowFix(
+                        "Edge (" + edge.getId() + ") does not have a unique id",
+                        function(){Utils.showEdge(eagle, edge.getId())},
+                        function(){Utils.newId(edge)},
+                        "Assign edge a new id"
+                    );
+                    errorsWarnings.errors.push(issue);
+                }
+                ids.push(edge.getId());
+            }
+        }
+
         return errorsWarnings;
     }
 
@@ -1912,6 +1948,10 @@ export class Utils {
         eagle.undo().pushSnapshot(eagle, "Fix");
     }
 
+    static newId(object: Node | Edge | Field): void {
+        object.setId(Utils.uuidv4());
+    }
+
     static showEdge(eagle: Eagle, edgeId: string): void {
         // close errors modal if visible
         $('#errorsModal').modal("hide");