From 90acb93ea84ea4f59faf7516e6a08d4e8a0d7190 Mon Sep 17 00:00:00 2001 From: Yann Lanthony Date: Fri, 14 Feb 2025 11:42:22 +0100 Subject: [PATCH] [core] Trigger node desc creation callback only for explicit node creation The desc.Node.onCreated callback is a hook in the Node API for developers to write custom behavior on a node first initialization. By being called in the core.Node constructor, this was triggered in several situations that don't match this idea and with unpredictable side effects (graph loading, node re-creation on undo...). * Make `onNodeCreated` callback an explicit method on desc.Node. * Remove the call to node descriptor's `onNodeCreated` callback outside core.Node constructor. * Trigger this callback on explicit node creation (adding new node to graph, init a graph from a template). --- meshroom/core/desc/node.py | 5 +++ meshroom/core/graph.py | 33 +++++++++++++----- meshroom/core/node.py | 27 --------------- tests/test_nodeCallbacks.py | 68 +++++++++++++++++++++++++++++++++++++ 4 files changed, 97 insertions(+), 36 deletions(-) create mode 100644 tests/test_nodeCallbacks.py diff --git a/meshroom/core/desc/node.py b/meshroom/core/desc/node.py index b105d2f9..2272c0ba 100644 --- a/meshroom/core/desc/node.py +++ b/meshroom/core/desc/node.py @@ -67,6 +67,11 @@ class Node(object): def upgradeAttributeValues(self, attrValues, fromVersion): return attrValues + @classmethod + def onNodeCreated(cls, node): + """Called after a node instance had been created from this node descriptor and added to a Graph.""" + pass + @classmethod def update(cls, node): """ Method call before node's internal update on invalidation. diff --git a/meshroom/core/graph.py b/meshroom/core/graph.py index 61666dd2..ce39d0f3 100644 --- a/meshroom/core/graph.py +++ b/meshroom/core/graph.py @@ -4,7 +4,7 @@ import json import logging import os import re -from typing import Any, Optional +from typing import Any, Iterable, Optional import weakref from collections import defaultdict, OrderedDict from contextlib import contextmanager @@ -257,6 +257,11 @@ class Graph(BaseObject): """ self._deserialize(Graph._loadGraphData(filepath)) + # Creating nodes from a template is conceptually similar to explicit node creation, + # therefore the nodes descriptors' "onNodeCreated" callback is triggered for each + # node instance created by this process. + self._triggerNodeCreatedCallback(self.nodes) + if not publishOutputs: with GraphModification(self): for node in [node for node in self.nodes if node.nodeType == "Publish"]: @@ -621,15 +626,17 @@ class Graph(BaseObject): return inEdges, outEdges, outListAttributes - def addNewNode(self, nodeType, name=None, position=None, **kwargs): + def addNewNode( + self, nodeType: str, name: Optional[str] = None, position: Optional[str] = None, **kwargs + ) -> Node: """ Create and add a new node to the graph. Args: - nodeType (str): the node type name. - name (str): if specified, the desired name for this node. If not unique, will be prefixed (_N). - position (Position): (optional) the position of the node - **kwargs: keyword arguments to initialize node's attributes + nodeType: the node type name. + name: if specified, the desired name for this node. If not unique, will be prefixed (_N). + position: the position of the node. + **kwargs: keyword arguments to initialize the created node's attributes. Returns: The newly created node. @@ -637,9 +644,17 @@ class Graph(BaseObject): if name and name in self._nodes.keys(): name = self._createUniqueNodeName(name) - n = self.addNode(Node(nodeType, position=position, **kwargs), uniqueName=name) - n.updateInternals() - return n + node = self.addNode(Node(nodeType, position=position, **kwargs), uniqueName=name) + node.updateInternals() + self._triggerNodeCreatedCallback([node]) + return node + + def _triggerNodeCreatedCallback(self, nodes: Iterable[Node]): + """Trigger the `onNodeCreated` node descriptor callback for each node instance in `nodes`.""" + with GraphModification(self): + for node in nodes: + if node.nodeDesc: + node.nodeDesc.onNodeCreated(node) def _createUniqueNodeName(self, inputName: str, existingNames: Optional[set[str]] = None): """Create a unique node name based on the input name. diff --git a/meshroom/core/node.py b/meshroom/core/node.py index 8d671135..bdf36976 100644 --- a/meshroom/core/node.py +++ b/meshroom/core/node.py @@ -1467,33 +1467,6 @@ class Node(BaseNode): if attr.invalidate: self.invalidatingAttributes.add(attr) - self.optionalCallOnDescriptor("onNodeCreated") - - def optionalCallOnDescriptor(self, methodName, *args, **kwargs): - """ Call of optional method defined in the descriptor. - Available method names are: - - onNodeCreated - """ - if hasattr(self.nodeDesc, methodName): - m = getattr(self.nodeDesc, methodName) - if callable(m): - try: - m(self, *args, **kwargs) - except Exception: - import traceback - # Format error strings with all the provided arguments - argsStr = ", ".join(str(arg) for arg in args) - kwargsStr = ", ".join(str(key) + "=" + str(value) for key, value in kwargs.items()) - finalErrStr = argsStr - if kwargsStr: - if argsStr: - finalErrStr += ", " - finalErrStr += kwargsStr - - logging.error("Error on call to '{}' (with args: '{}') for node type {}". - format(methodName, finalErrStr, self.nodeType)) - logging.error(traceback.format_exc()) - def setAttributeValues(self, values): # initialize attribute values for k, v in values.items(): diff --git a/tests/test_nodeCallbacks.py b/tests/test_nodeCallbacks.py new file mode 100644 index 00000000..149151e8 --- /dev/null +++ b/tests/test_nodeCallbacks.py @@ -0,0 +1,68 @@ +from meshroom.core import desc, registerNodeType, unregisterNodeType +from meshroom.core.node import Node +from meshroom.core.graph import Graph, loadGraph + + +class NodeWithCreationCallback(desc.InputNode): + """Node defining an 'onNodeCreated' callback, triggered a new node is added to a Graph.""" + + inputs = [ + desc.BoolParam( + name="triggered", + label="Triggered", + description="Attribute impacted by the `onNodeCreated` callback", + value=False, + ), + ] + + @classmethod + def onNodeCreated(cls, node: Node): + """Triggered when a new node is created within a Graph.""" + node.triggered.value = True + + +class TestNodeCreationCallback: + @classmethod + def setup_class(cls): + registerNodeType(NodeWithCreationCallback) + + @classmethod + def teardown_class(cls): + unregisterNodeType(NodeWithCreationCallback) + + def test_notTriggeredOnNodeInstantiation(self): + node = Node(NodeWithCreationCallback.__name__) + assert node.triggered.value is False + + def test_triggeredOnNewNodeCreationInGraph(self): + graph = Graph("") + node = graph.addNewNode(NodeWithCreationCallback.__name__) + assert node.triggered.value is True + + def test_notTriggeredOnNodeDuplication(self): + graph = Graph("") + node = graph.addNewNode(NodeWithCreationCallback.__name__) + node.triggered.resetToDefaultValue() + + duplicates = graph.duplicateNodes([node]) + assert duplicates[node][0].triggered.value is False + + def test_notTriggeredOnGraphLoad(self, graphSavedOnDisk): + graph: Graph = graphSavedOnDisk + node = graph.addNewNode(NodeWithCreationCallback.__name__) + node.triggered.resetToDefaultValue() + graph.save() + + loadedGraph = loadGraph(graph.filepath) + assert loadedGraph.node(node.name).triggered.value is False + + def test_triggeredOnGraphInitializationFromTemplate(self, graphSavedOnDisk): + graph: Graph = graphSavedOnDisk + node = graph.addNewNode(NodeWithCreationCallback.__name__) + node.triggered.resetToDefaultValue() + graph.save(template=True) + + graphFromTemplate = Graph("") + graphFromTemplate.initFromTemplate(graph.filepath) + + assert graphFromTemplate.node(node.name).triggered.value is True