[core] Check that the description of a node is correct before loading it

At Meshroom's launch, check that every node we attempt to load has a
valid description, i.e. that every parameter has a default value that
matches its parameter's type.

If there is at least one parameter with an incorrect default value,
the node is not loaded and a corresponding message will be displayed.

This prevents the user from loading erroneous nodes that may lead to
unexpected behaviours (such as a change of a node's UID between the
moment when it is written and the moment it is loaded).

2 files changed, 102 insertions, 1 deletions

diff --git a/meshroom/core/__init__.py b/meshroom/core/__init__.py
index 56d6351d..3c484637 100644
--- a/meshroom/core/__init__.py
+++ b/meshroom/core/__init__.py
@@ -80,10 +80,20 @@ def loadPlugins(folder, packageName, classType):
                            and issubclass(plugin, classType)]
                 if not plugins:
                     logging.warning("No class defined in plugin: {}".format(pluginModuleName))
+
+                importPlugin = True
                 for p in plugins:
+                    if classType == desc.Node:
+                        nodeErrors = validateNodeDesc(p)
+                        if nodeErrors:
+                            errors.append("  * {}: The following parameters do not have valid default values: {}"
+                                          .format(pluginName, ", ".join(nodeErrors)))
+                            importPlugin = False
+                            break
                     p.packageName = packageName
                     p.packageVersion = packageVersion
-                pluginTypes.extend(plugins)
+                if importPlugin:
+                    pluginTypes.extend(plugins)
             except Exception as e:
                 errors.append('  * {}: {}'.format(pluginName, str(e)))
 
@@ -94,6 +104,38 @@ def loadPlugins(folder, packageName, classType):
     return pluginTypes
 
 
+def validateNodeDesc(nodeDesc):
+    """
+    Check that the node has a valid description before being loaded. For the description
+    to be valid, the default value of every parameter needs to correspond to the type
+    of the parameter.
+    An empty returned list means that every parameter is valid, and so is the node's description.
+    If it is not valid, the returned list contains the names of the invalid parameters. In case
+    of nested parameters (parameters in groups or lists, for example), the name of the parameter
+    follows the name of the parent attributes. For example, if the attribute "x", contained in group
+    "group", is invalid, then it will be added to the list as "group:x".
+
+    Args:
+        nodeDesc (desc.Node): description of the node
+
+    Returns:
+        errors (list): the list of invalid parameters if there are any, empty list otherwise
+    """
+    errors = []
+
+    for param in nodeDesc.inputs:
+        err = param.checkValueTypes()
+        if err:
+            errors.append(err)
+
+    for param in nodeDesc.outputs:
+        err = param.checkValueTypes()
+        if err:
+            errors.append(err)
+
+    return errors
+
+
 class Version(object):
     """
     Version provides convenient properties and methods to manipulate and compare versions.
diff --git a/meshroom/core/desc.py b/meshroom/core/desc.py
index e66eb053..74a62c9d 100755
--- a/meshroom/core/desc.py
+++ b/meshroom/core/desc.py
@@ -44,6 +44,14 @@ class Attribute(BaseObject):
         """
         raise NotImplementedError("Attribute.validateValue is an abstract function that should be implemented in the derived class.")
 
+    def checkValueTypes(self):
+        """ Returns the attribute's name if the default value's type is invalid, empty string otherwise.
+
+        Returns:
+            string: contains the attribute's name if the type is invalid, empty string otherwise
+        """
+        raise NotImplementedError("Attribute.checkValueTypes is an abstract function that should be implemented in the derived class.")
+
     def matchDescription(self, value, strict=True):
         """ Returns whether the value perfectly match attribute's description.
 
@@ -85,6 +93,9 @@ class ListAttribute(Attribute):
             raise ValueError('ListAttribute only supports list/tuple input values (param:{}, value:{}, type:{})'.format(self.name, value, type(value)))
         return value
 
+    def checkValueTypes(self):
+        return self.elementDesc.checkValueTypes()
+
     def matchDescription(self, value, strict=True):
         """ Check that 'value' content matches ListAttribute's element description. """
         if not super(ListAttribute, self).matchDescription(value, strict):
@@ -133,6 +144,23 @@ class GroupAttribute(Attribute):
 
         return value
 
+    def checkValueTypes(self):
+        """ Check the type of every attribute contained in the group (including nested attributes).
+        Returns an empty string if all the attributes' types are valid, or concatenates the names of the attributes in
+        the group with invalid types.
+        """
+        invalidParams = []
+        for attr in self.groupDesc:
+            name = attr.checkValueTypes()
+            if name:
+                invalidParams.append(name)
+        if invalidParams:
+            # In group "group", if parameters "x" and "y" (in nested group "subgroup") are invalid, the returned
+            # string will be: "group:x, group:subgroup:y"
+            return self.name + ":" + str(", " + self.name + ":").join(invalidParams)
+        return ""
+
+
     def matchDescription(self, value, strict=True):
         """
         Check that 'value' contains the exact same set of keys as GroupAttribute's group description
@@ -185,6 +213,13 @@ class File(Attribute):
             raise ValueError('File only supports string input  (param:{}, value:{}, type:{})'.format(self.name, value, type(value)))
         return os.path.normpath(value).replace('\\', '/') if value else ''
 
+    def checkValueTypes(self):
+        # Some File values are functions generating a string: check whether the value is a string or if it
+        # is a function (but there is no way to check that the function's output is indeed a string)
+        if not isinstance(self.value, pyCompatibility.basestring) and not callable(self.value):
+            return self.name
+        return ""
+
 
 class BoolParam(Param):
     """
@@ -201,6 +236,11 @@ class BoolParam(Param):
         except:
             raise ValueError('BoolParam only supports bool value (param:{}, value:{}, type:{})'.format(self.name, value, type(value)))
 
+    def checkValueTypes(self):
+        if not isinstance(self.value, bool):
+            return self.name
+        return ""
+
 
 class IntParam(Param):
     """
@@ -218,6 +258,11 @@ class IntParam(Param):
         except:
             raise ValueError('IntParam only supports int value (param:{}, value:{}, type:{})'.format(self.name, value, type(value)))
 
+    def checkValueTypes(self):
+        if not isinstance(self.value, int):
+            return self.name
+        return ""
+
     range = Property(VariantList, lambda self: self._range, constant=True)
 
 
@@ -234,6 +279,11 @@ class FloatParam(Param):
         except:
             raise ValueError('FloatParam only supports float value (param:{}, value:{}, type:{})'.format(self.name, value, type(value)))
 
+    def checkValueTypes(self):
+        if not isinstance(self.value, float):
+            return self.name
+        return ""
+
     range = Property(VariantList, lambda self: self._range, constant=True)
 
 
@@ -263,6 +313,10 @@ class ChoiceParam(Param):
             raise ValueError('Non exclusive ChoiceParam value should be iterable (param:{}, value:{}, type:{})'.format(self.name, value, type(value)))
         return [self.conformValue(v) for v in value]
 
+    def checkValueTypes(self):
+        # nothing to validate
+        return ""
+
     values = Property(VariantList, lambda self: self._values, constant=True)
     exclusive = Property(bool, lambda self: self._exclusive, constant=True)
     joinChar = Property(str, lambda self: self._joinChar, constant=True)
@@ -279,6 +333,11 @@ class StringParam(Param):
             raise ValueError('StringParam value should be a string (param:{}, value:{}, type:{})'.format(self.name, value, type(value)))
         return value
 
+    def checkValueTypes(self):
+        if not isinstance(self.value, pyCompatibility.basestring):
+            return self.name
+        return ""
+
 
 class Level(Enum):
     NONE = 0