Update ListAttributes identically when removing edges or nodes

The behaviour regarding `ListAttributes` was different depending on
whether an edge or a node was removed:
- if an edge was removed, the `ListAttribute` that was the destination
of that edge had its corresponding element completely removed;
- if a node was removed, the `ListAttribute` that was the destination
of one of the node's edges had its corresponding element reset, but not
removed.

With this behaviour, a user had different UIDs depending on whether
a single edge or the whole node had been removed where the UID should
always be identical.

meshroom/core/graph.py

@@ -645,18 +645,38 @@ def nodeOutEdges(self, node):
     @changeTopology
     def removeNode(self, nodeName):
         """
-        Remove the node identified by 'nodeName' from the graph
-        and return in and out edges removed by this operation in two dicts {dstAttr.getFullNameToNode(), srcAttr.getFullNameToNode()}
+        Remove the node identified by 'nodeName' from the graph.
+        Returns:
+            - a dictionary containing the incoming edges removed by this operation: {dstAttr.getFullNameToNode(), srcAttr.getFullNameToNode()}
+            - a dictionary containing the outgoing edges removed by this operation: {dstAttr.getFullNameToNode(), srcAttr.getFullNameToNode()}
+            - a dictionary containing the values, indices and keys of attributes that were connected to a ListAttribute prior to the removal of all edges:
+                {dstAttr.getFullNameToNode(), (dstAttr.root.getFullNameToNode(), dstAttr.index, dstAttr.value)}
         """
         node = self.node(nodeName)
         inEdges = {}
         outEdges = {}
+        outListAttributes = {}
 
         # Remove all edges arriving to and starting from this node
         with GraphModification(self):
+            # Two iterations over the outgoing edges are necessary:
+            # - the first one is used to collect all the information about the edges while they are all there (overall context)
+            # - once we have collected all the information, the edges (and perhaps the entries in ListAttributes) can actually be removed
             for edge in self.nodeOutEdges(node):
-                self.removeEdge(edge.dst)
                 outEdges[edge.dst.getFullNameToNode()] = edge.src.getFullNameToNode()
+
+                if isinstance(edge.dst.root, ListAttribute):
+                    index = edge.dst.root.index(edge.dst)
+                    outListAttributes[edge.dst.getFullNameToNode()] = (edge.dst.root.getFullNameToNode(), index, edge.dst.value if edge.dst.value else None)
+
+            for edge in self.nodeOutEdges(node):
+                self.removeEdge(edge.dst)
+
+                # Remove the corresponding attributes from the ListAttributes instead of just emptying their values
+                if isinstance(edge.dst.root, ListAttribute):
+                    index = edge.dst.root.index(edge.dst)
+                    edge.dst.root.remove(index)
+
             for edge in self.nodeInEdges(node):
                 self.removeEdge(edge.dst)
                 inEdges[edge.dst.getFullNameToNode()] = edge.src.getFullNameToNode()
@@ -667,7 +687,7 @@ def removeNode(self, nodeName):
                 self._importedNodes.remove(node)
             self.update()
 
-        return inEdges, outEdges
+        return inEdges, outEdges, outListAttributes
 
     def addNewNode(self, nodeType, name=None, position=None, **kwargs):
         """
@@ -707,22 +727,35 @@ def upgradeNode(self, nodeName):
             nodeName (str): the name of the CompatibilityNode to upgrade
 
         Returns:
-            the list of deleted input/output edges
+            - the upgraded (newly created) node
+            - a dictionary containing the incoming edges removed by this operation: {dstAttr.getFullNameToNode(), srcAttr.getFullNameToNode()}
+            - a dictionary containing the outgoing edges removed by this operation: {dstAttr.getFullNameToNode(), srcAttr.getFullNameToNode()}
+            - a dictionary containing the values, indices and keys of attributes that were connected to a ListAttribute prior to the removal of all edges:
+                {dstAttr.getFullNameToNode(), (dstAttr.root.getFullNameToNode(), dstAttr.index, dstAttr.value)}
         """
         node = self.node(nodeName)
         if not isinstance(node, CompatibilityNode):
             raise ValueError("Upgrade is only available on CompatibilityNode instances.")
         upgradedNode = node.upgrade()
         with GraphModification(self):
-            inEdges, outEdges = self.removeNode(nodeName)
+            inEdges, outEdges, outListAttributes = self.removeNode(nodeName)
             self.addNode(upgradedNode, nodeName)
             for dst, src in outEdges.items():
+                # Re-create the entries in ListAttributes that were completely removed during the call to "removeNode"
+                # If they are not re-created first, adding their edges will lead to errors
+                # 0 = attribute name, 1 = attribute index, 2 = attribute value
+                if dst in outListAttributes.keys():
+                    listAttr = self.attribute(outListAttributes[dst][0])
+                    if isinstance(outListAttributes[dst][2], list):
+                        listAttr[outListAttributes[dst][1]:outListAttributes[dst][1]] = outListAttributes[dst][2]
+                    else:
+                        listAttr.insert(outListAttributes[dst][1], outListAttributes[dst][2])
                 try:
                     self.addEdge(self.attribute(src), self.attribute(dst))
                 except (KeyError, ValueError) as e:
                     logging.warning("Failed to restore edge {} -> {}: {}".format(src, dst, str(e)))
 
-        return upgradedNode, inEdges, outEdges
+        return upgradedNode, inEdges, outEdges, outListAttributes
 
     def upgradeAllNodes(self):
         """ Upgrade all upgradable CompatibilityNode instances in the graph. """

meshroom/ui/commands.py

@@ -156,10 +156,11 @@ def __init__(self, graph, node, parent=None):
         self.nodeName = node.getName()
         self.setText("Remove Node {}".format(self.nodeName))
         self.outEdges = {}
+        self.outListAttributes = {}  # maps attribute's key with a tuple containing the name of the list it is connected to and its value
 
     def redoImpl(self):
-        # only keep outEdges since inEdges are serialized in nodeDict
-        _, self.outEdges = self.graph.removeNode(self.nodeName)
+        # keep outEdges (inEdges are serialized in nodeDict so unneeded here) and outListAttributes to be able to recreate the deleted elements in ListAttributes
+        _, self.outEdges, self.outListAttributes = self.graph.removeNode(self.nodeName)
         return True
 
     def undoImpl(self):
@@ -169,6 +170,15 @@ def undoImpl(self):
             assert (node.getName() == self.nodeName)
             # recreate out edges deleted on node removal
             for dstAttr, srcAttr in self.outEdges.items():
+                # if edges were connected to ListAttributes, recreate their corresponding entry in said ListAttribute
+                # 0 = attribute name, 1 = attribute index, 2 = attribute value
+                if dstAttr in self.outListAttributes.keys():
+                    listAttr = self.graph.attribute(self.outListAttributes[dstAttr][0])
+                    if isinstance(self.outListAttributes[dstAttr][2], list):
+                        listAttr[self.outListAttributes[dstAttr][1]:self.outListAttributes[dstAttr][1]] = self.outListAttributes[dstAttr][2]
+                    else:
+                        listAttr.insert(self.outListAttributes[dstAttr][1], self.outListAttributes[dstAttr][2])
+
                 self.graph.addEdge(self.graph.attribute(srcAttr),
                                    self.graph.attribute(dstAttr))
 
@@ -410,12 +420,13 @@ def __init__(self, graph, node, parent=None):
         self.nodeDict = node.toDict()
         self.nodeName = node.getName()
         self.outEdges = {}
+        self.outListAttributes = {}
         self.setText("Upgrade Node {}".format(self.nodeName))
 
     def redoImpl(self):
         if not self.graph.node(self.nodeName).canUpgrade:
             return False
-        upgradedNode, inEdges, self.outEdges = self.graph.upgradeNode(self.nodeName)
+        upgradedNode, _, self.outEdges, self.outListAttributes = self.graph.upgradeNode(self.nodeName)
         return upgradedNode
 
     def undoImpl(self):
@@ -427,6 +438,15 @@ def undoImpl(self):
             self.graph.addNode(node, self.nodeName)
             # recreate out edges
             for dstAttr, srcAttr in self.outEdges.items():
+                # if edges were connected to ListAttributes, recreate their corresponding entry in said ListAttribute
+                # 0 = attribute name, 1 = attribute index, 2 = attribute value
+                if dstAttr in self.outListAttributes.keys():
+                    listAttr = self.graph.attribute(self.outListAttributes[dstAttr][0])
+                    if isinstance(self.outListAttributes[dstAttr][2], list):
+                        listAttr[self.outListAttributes[dstAttr][1]:self.outListAttributes[dstAttr][1]] = self.outListAttributes[dstAttr][2]
+                    else:
+                        listAttr.insert(self.outListAttributes[dstAttr][1], self.outListAttributes[dstAttr][2])
+
                 self.graph.addEdge(self.graph.attribute(srcAttr),
                                    self.graph.attribute(dstAttr))