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.
This commit is contained in:
Candice Bentéjac 2024-02-02 15:23:44 +01:00
parent 49d808b98b
commit e9ee9f2315
2 changed files with 63 additions and 10 deletions

View file

@ -645,18 +645,38 @@ class Graph(BaseObject):
@changeTopology @changeTopology
def removeNode(self, nodeName): def removeNode(self, nodeName):
""" """
Remove the node identified by 'nodeName' from the graph 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()} 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) node = self.node(nodeName)
inEdges = {} inEdges = {}
outEdges = {} outEdges = {}
outListAttributes = {}
# Remove all edges arriving to and starting from this node # Remove all edges arriving to and starting from this node
with GraphModification(self): 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):
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): for edge in self.nodeOutEdges(node):
self.removeEdge(edge.dst) self.removeEdge(edge.dst)
outEdges[edge.dst.getFullNameToNode()] = edge.src.getFullNameToNode()
# 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): for edge in self.nodeInEdges(node):
self.removeEdge(edge.dst) self.removeEdge(edge.dst)
inEdges[edge.dst.getFullNameToNode()] = edge.src.getFullNameToNode() inEdges[edge.dst.getFullNameToNode()] = edge.src.getFullNameToNode()
@ -667,7 +687,7 @@ class Graph(BaseObject):
self._importedNodes.remove(node) self._importedNodes.remove(node)
self.update() self.update()
return inEdges, outEdges return inEdges, outEdges, outListAttributes
def addNewNode(self, nodeType, name=None, position=None, **kwargs): def addNewNode(self, nodeType, name=None, position=None, **kwargs):
""" """
@ -707,22 +727,35 @@ class Graph(BaseObject):
nodeName (str): the name of the CompatibilityNode to upgrade nodeName (str): the name of the CompatibilityNode to upgrade
Returns: 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) node = self.node(nodeName)
if not isinstance(node, CompatibilityNode): if not isinstance(node, CompatibilityNode):
raise ValueError("Upgrade is only available on CompatibilityNode instances.") raise ValueError("Upgrade is only available on CompatibilityNode instances.")
upgradedNode = node.upgrade() upgradedNode = node.upgrade()
with GraphModification(self): with GraphModification(self):
inEdges, outEdges = self.removeNode(nodeName) inEdges, outEdges, outListAttributes = self.removeNode(nodeName)
self.addNode(upgradedNode, nodeName) self.addNode(upgradedNode, nodeName)
for dst, src in outEdges.items(): 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: try:
self.addEdge(self.attribute(src), self.attribute(dst)) self.addEdge(self.attribute(src), self.attribute(dst))
except (KeyError, ValueError) as e: except (KeyError, ValueError) as e:
logging.warning("Failed to restore edge {} -> {}: {}".format(src, dst, str(e))) logging.warning("Failed to restore edge {} -> {}: {}".format(src, dst, str(e)))
return upgradedNode, inEdges, outEdges return upgradedNode, inEdges, outEdges, outListAttributes
def upgradeAllNodes(self): def upgradeAllNodes(self):
""" Upgrade all upgradable CompatibilityNode instances in the graph. """ """ Upgrade all upgradable CompatibilityNode instances in the graph. """

View file

@ -156,10 +156,11 @@ class RemoveNodeCommand(GraphCommand):
self.nodeName = node.getName() self.nodeName = node.getName()
self.setText("Remove Node {}".format(self.nodeName)) self.setText("Remove Node {}".format(self.nodeName))
self.outEdges = {} 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): def redoImpl(self):
# only keep outEdges since inEdges are serialized in nodeDict # 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.graph.removeNode(self.nodeName) _, self.outEdges, self.outListAttributes = self.graph.removeNode(self.nodeName)
return True return True
def undoImpl(self): def undoImpl(self):
@ -169,6 +170,15 @@ class RemoveNodeCommand(GraphCommand):
assert (node.getName() == self.nodeName) assert (node.getName() == self.nodeName)
# recreate out edges deleted on node removal # recreate out edges deleted on node removal
for dstAttr, srcAttr in self.outEdges.items(): 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.addEdge(self.graph.attribute(srcAttr),
self.graph.attribute(dstAttr)) self.graph.attribute(dstAttr))
@ -410,12 +420,13 @@ class UpgradeNodeCommand(GraphCommand):
self.nodeDict = node.toDict() self.nodeDict = node.toDict()
self.nodeName = node.getName() self.nodeName = node.getName()
self.outEdges = {} self.outEdges = {}
self.outListAttributes = {}
self.setText("Upgrade Node {}".format(self.nodeName)) self.setText("Upgrade Node {}".format(self.nodeName))
def redoImpl(self): def redoImpl(self):
if not self.graph.node(self.nodeName).canUpgrade: if not self.graph.node(self.nodeName).canUpgrade:
return False return False
upgradedNode, inEdges, self.outEdges = self.graph.upgradeNode(self.nodeName) upgradedNode, _, self.outEdges, self.outListAttributes = self.graph.upgradeNode(self.nodeName)
return upgradedNode return upgradedNode
def undoImpl(self): def undoImpl(self):
@ -427,6 +438,15 @@ class UpgradeNodeCommand(GraphCommand):
self.graph.addNode(node, self.nodeName) self.graph.addNode(node, self.nodeName)
# recreate out edges # recreate out edges
for dstAttr, srcAttr in self.outEdges.items(): 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.addEdge(self.graph.attribute(srcAttr),
self.graph.attribute(dstAttr)) self.graph.attribute(dstAttr))