[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).

Author: yann-lty

meshroom/core/desc/node.py

@@ -67,6 +67,11 @@ def __init__(self):
     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.

meshroom/core/graph.py

@@ -4,7 +4,7 @@
 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 @@ def initFromTemplate(self, filepath: PathLike, publishOutputs: bool = False):
         """
         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,25 +626,35 @@ def removeNode(self, nodeName):
 
         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.
         """
         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.

meshroom/core/node.py

@@ -1467,33 +1467,6 @@ def __init__(self, nodeType, position=None, parent=None, uid=None, **kwargs):
             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():

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