[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).
This commit is contained in:
Candice Bentéjac 2022-09-27 16:12:40 +02:00
parent 5b45182bcb
commit 545f3a7218
2 changed files with 102 additions and 1 deletions

View file

@ -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.

View file

@ -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