[ui/core] Add Validators paradigm

Author: nicolas-lambert-tc

meshroom/common/PySignal.py

@@ -158,6 +158,10 @@ class ClassSignal:
     """
     _map = {}
 
+    def __init__(self, *args, **kwargs):
+        self._args = args
+        self._kwargs = kwargs
+
     def __get__(self, instance, owner):
         if instance is None:
             # When we access ClassSignal element on the class object without any instance,

meshroom/core/attribute.py

@@ -12,6 +12,7 @@
 from collections.abc import Iterable, Sequence
 from string import Template
 from meshroom.common import BaseObject, Property, Variant, Signal, ListModel, DictModel, Slot
+from meshroom.core.desc.validators import NotEmptyValidator
 from meshroom.core import desc, hashValue
 from meshroom.core.keyValues import KeyValues
 from meshroom.core.exception import InvalidEdgeError
@@ -84,6 +85,7 @@ def __init__(self, node, attributeDesc: desc.Attribute, isOutput: bool, root=Non
         self._enabled: bool = True
         self._depth: int = root.depth + 1 if root is not None else 0
         self._exposed: bool = root.exposed if root is not None else attributeDesc.exposed
+        self._description: str = attributeDesc.description
         self._invalidate = False if self._isOutput else attributeDesc.invalidate
         self._invalidationValue = ""  # invalidation value for output attributes
         self._value = None
@@ -235,6 +237,15 @@ def _setValue(self, value):
             self.requestNodeUpdate()
         self.valueChanged.emit()
 
+    def _get_description(self):
+        return self._description
+
+    def _set_description(self, desc):
+        if self._description == desc:
+            return
+        self._description = desc
+        self.descriptionChanged.emit()
+
     def _getKeyValues(self):
         """
         Return the per-key values object of the attribute or of the linked attribute.
@@ -401,21 +412,6 @@ def _isDefault(self):
         else:
             return self._getValue() == self.getDefaultValue()
 
-    def _isValid(self):
-        """
-        Check attribute description validValue:
-            - If it is a function, execute it and return the result
-            - Otherwise, simply return true
-        """
-        if callable(self._desc.validValue):
-            try:
-                return self._desc.validValue(self.node)
-            except Exception as exc:
-                if not self.node.isCompatibilityNode:
-                    logging.warning(f"Failed to evaluate 'isValid' (node lambda) for attribute '{self.fullName}': {exc}")
-                return True
-        return True
-
     def _is2dDisplayable(self) -> bool:
         """
         Return True if the current attribute is considered as a displayable 2D file.
@@ -487,6 +483,43 @@ def updateInternals(self):
         # Emit if the enable status has changed
         self._setEnabled(self._getEnabled())
 
+    def getErrorMessages(self) -> list[str]:
+        """ Execute the validators and aggregate the eventual error messages"""
+
+        result = []
+
+        for validator in self.desc.validators:
+            isValid, errorMessages = validator(self.node, self)
+
+            if isValid:
+                continue
+
+            for errorMessage in errorMessages:
+                result.append(errorMessage)
+
+        return result
+
+    def _isValid(self) -> bool:
+        """ Check the validation and return False if any validator return (False, erorrs)
+        """
+
+        for validator in self.desc.validators:
+            isValid, _ = validator(self.node, self)
+
+            if not isValid:
+                return False
+
+        return True
+
+    def _isMandatory(self) -> bool:
+        """ An attribute is considered as mandatory it contain a NotEmptyValidator """
+
+        for validator in self.desc.validators:
+            if isinstance(validator, NotEmptyValidator):
+                return True
+
+        return False
+
     def _getEnabled(self) -> bool:
         if callable(self._desc.enabled):
             try:
@@ -742,6 +775,9 @@ def validateIncomingConnection(self, connectingAttribute: Attribute) -> bool:
 
     expressionApplied = Signal()
 
+    errorMessageChanged = Signal()
+    errorMessages = Property(Variant, lambda self: self.getErrorMessages(), notify=errorMessageChanged)
+    isMandatory = Property(bool, _isMandatory, constant=True )    
 
 def raiseIfLink(func):
     """

meshroom/core/desc/attribute.py

@@ -0,0 +1,66 @@
+from typing import TYPE_CHECKING
+
+if TYPE_CHECKING:
+    from meshroom.core.attribute import Attribute
+    from meshroom.core.node import Node
+
+
+def success() -> tuple[bool, list[str]]:
+    return (True, [])
+
+def error(*messages: str) -> tuple[bool, list[str]]:
+    return (False, list(messages))
+
+class AttributeValidator(object):
+    """ Interface for an attribute validation
+        You can inherit from this class and override the __call__ methods to implement your own attribute validation logic
+
+        Because it's a callable class, you can also create your own validators on the fly
+
+        .. code-block: python
+
+            lambda node, attribute: success() if attribute.value and attribute.value != "" else error("attribute have no value")
+    """
+
+    def __call__(self, node: "Node", attribute: "Attribute") -> tuple[bool, list[str]]:
+        """
+            Override this method to implement your custom validation logic.
+            You can use the success() and error() helpers that encapsulate the returning response.
+
+            :param node: The node that holds the attribute to validate
+            :param attribute: The atribute to validate
+
+            :returns: The validtion response: (True, []) if it's valid, (False, [errorMessage1, errorMessage2, ...]) if error exists
+
+        """
+        raise NotImplementedError()
+
+
+class NotEmptyValidator(AttributeValidator):
+    """ The attribute value should not be empty
+        This class is used to determine if an attribute label should be considered as mandatory/required
+    """
+
+    def __call__(self, node: "Node", attribute: "Attribute") -> tuple[bool, list[str]]:
+
+        if attribute.value is None or attribute.value == "":
+            return error("Empty value is not allowed")
+        
+        return success()
+
+
+class RangeValidator(AttributeValidator):
+    """ Check if the attribute value is in the given range
+    """
+
+    def __init__(self, min, max):
+        self._min = min
+        self._max = max
+
+    def __call__(self, node:"Node", attribute: "Attribute") -> tuple[bool, list[str]]:
+
+        if attribute.value < self._min or attribute.value > self._max:
+            return error(f"Value should be greater than {self._min} and less than {self._max}", 
+                            f"({self._min} < {attribute.value} < {self._max})")
+
+        return success()

meshroom/core/desc/validators.py

@@ -1710,6 +1710,7 @@ def setVerbose(self, v):
     statusUpdated = Signal()
     canComputeLeavesChanged = Signal()
     canComputeLeaves = Property(bool, lambda self: self._canComputeLeaves, notify=canComputeLeavesChanged)
+    attributeValueChanged = Signal(Attribute)
 
 
 def loadGraph(filepath, strictCompatibility: bool = False) -> Graph:

meshroom/core/graph.py

@@ -1522,6 +1522,8 @@ def _onAttributeChanged(self, attr: Attribute):
         if callback:
             callback(self)
 
+        self.hasInvalidAttributeChanged.emit()
+
         if self.graph:
             # If we are in a graph, propagate the notification to the connected output attributes
             for edge in self.graph.outEdges(attr):
@@ -1792,7 +1794,7 @@ def loadOutputAttr(self):
         # This does not apply to non dynamic output
         if not self.nodeDesc.hasDynamicOutputAttribute:
             return
-        
+
         # Check existence of values.json file
         valuesFile = self.valuesFile
         if not os.path.exists(valuesFile):
@@ -2157,6 +2159,12 @@ def hasTextOutputAttribute(self) -> bool:
         """
         return next((attr for attr in self._attributes if attr.enabled and attr.isOutput and attr.isTextDisplayable), None) is not None
 
+    def _hasInvalidAttribute(self):
+        for attribute in self._attributes:
+            if len(attribute.errorMessages) > 0:
+                return True
+        return False
+
     def _hasDisplayableShape(self):
         """
         Return True if at least one attribute is a ShapeAttribute, a ShapeListAttribute or a shape File.
@@ -2236,6 +2244,9 @@ def _hasDisplayableShape(self):
     # Whether the node contains a ShapeAttribute, a ShapeListAttribute or a shape File.
     hasDisplayableShape = Property(bool, _hasDisplayableShape, constant=True)
 
+    hasInvalidAttributeChanged = Signal()
+    hasInvalidAttribute = Property(bool, _hasInvalidAttribute, notify=hasInvalidAttributeChanged)
+
 
 class Node(BaseNode):
     """

meshroom/core/node.py

@@ -314,16 +314,21 @@ def redoImpl(self):
         if self.value == self.oldValue:
             return False
         if self.graph.attribute(self.attrName) is not None:
-            self.graph.attribute(self.attrName).value = self.value
+            attribute = self.graph.attribute(self.attrName)
         else:
-            self.graph.internalAttribute(self.attrName).value = self.value
+            attribute = self.graph.internalAttribute(self.attrName)
+
+        attribute.value = self.value        
+
         return True
 
     def undoImpl(self):
         if self.graph.attribute(self.attrName) is not None:
-            self.graph.attribute(self.attrName).value = self.oldValue
+            attribute = self.graph.attribute(self.attrName)
         else:
-            self.graph.internalAttribute(self.attrName).value = self.oldValue
+            attribute = self.graph.internalAttribute(self.attrName)
+
+        attribute.value = self.oldValue
 
 class AddAttributeKeyValueCommand(GraphCommand):
     def __init__(self, graph, attribute, key, value, parent=None):

meshroom/ui/commands.py

@@ -39,7 +39,7 @@ Page {
         property alias showImageGallery: imageGalleryVisibilityCB.checked
         property alias showTextViewer: textViewerVisibilityCB.checked
     }
-    
+
     Settings {
         id: nodeActionsSettings
         category: "NodeActions"
@@ -135,6 +135,14 @@ Page {
         return true;
     }
 
+    function getAllNodes() {
+        const nodes = []
+        for(let i=0; i<graphEditor.graph.nodes.count; i++) {
+            nodes.push(graphEditor.graph.nodes.at(i))
+        }
+        return nodes
+    }
+
     // File dialogs
     Platform.FileDialog {
         id: saveFileDialog
@@ -267,14 +275,24 @@ Page {
         function submit(nodes) {
             if (!canSubmit) {
                 unsavedSubmitDialog.open()
-            } else {
+            }
+            else {
                 try {
-                    _currentScene.submit(nodes)
+                    if(nodes == null) {
+                        nodes = getAllNodes()
+                    }
+                    if ( nodes && nodes.find(node => node.hasInvalidAttribute) ) {
+                        submitWithWarningDialog.nodes = nodes
+                        submitWithWarningDialog.open()
+                    } else {
+                        _currentScene.submit(nodes)
+                    }
                 }
-                catch (error) {
+                 catch (error) {
                     const data = ErrorHandler.analyseError(error)
-                    if (data.context === "SUBMITTING")
+                    if (data.context === "SUBMITTING") {
                         computeSubmitErrorDialog.openError(data.type, data.msg, nodes)
+                    }
                 }
             }
         }
@@ -400,6 +418,26 @@ Page {
             onAccepted: saveAsAction.trigger()
         }
 
+        MessageDialog {
+            id: submitWithWarningDialog
+
+            canCopy: false
+            icon.text: MaterialIcons.warning
+            parent: Overlay.overlay
+            preset: "Warning"
+            title: "Nodes Containing Warnings"
+            text: "Some nodes contain warnings. Are you sure you want to submit?"
+            helperText: "Submit even if some nodes have warnings"
+            standardButtons: Dialog.Cancel | Dialog.Yes
+
+            property var nodes: []
+
+            onDiscarded: close()
+            onAccepted: {
+                _currentScene.submit(nodes)
+            }
+        }
+
         MessageDialog {
             id: fileModifiedDialog
 
@@ -733,7 +771,7 @@ Page {
                         model: MeshroomApp.recentProjectFiles
                         MenuItem {
                             enabled: modelData["status"] != 0
-                            
+
                             onTriggered: ensureSaved(function() {
                                 openRecentMenu.dismiss()
                                 if (_currentScene.load(modelData["path"])) {
@@ -887,7 +925,7 @@ Page {
                         id: nodeActionsSettingsMenu
                         title: "NodeActions Settings"
                         implicitWidth: 250
-                        
+
                         MenuItem {
                             id: nodeActionsConfirmDelete
                             checkable: true
@@ -1504,7 +1542,7 @@ Page {
                     SplitView.minimumWidth: 350
 
                     node: _currentScene ? _currentScene.selectedNode : null
-                    property bool computing: _currentScene ? _currentScene.computing : false       
+                    property bool computing: _currentScene ? _currentScene.computing : false
                     property var currentAttributes: []
 
                     // Make NodeEditor readOnly when computing