DataModel: allow valid values in non-strict mode for TypeOfList and TypeOfDict

platformio/datamodel.py

@@ -74,8 +74,8 @@ class DataField(object):
         self.validate_factory = validate_factory
         self.title = title
 
-        self.parent = None
+        self._parent = None
-        self.name = None
+        self._name = None
         self.value = None
 
     def __repr__(self):
@@ -84,13 +84,24 @@ class DataField(object):
             self.default if self.value is None else self.value,
         )
 
-    def validate(
+    @property
-        self, parent, name, value
+    def parent(self):
-    ):  # pylint: disable=too-many-return-statements
+        return self._parent
-        self.parent = parent
-        self.name = name
-        self.title = self.title or name.title()
 
+    @parent.setter
+    def parent(self, value):
+        self._parent = value
+
+    @property
+    def name(self):
+        return self._name
+
+    @name.setter
+    def name(self, value):
+        self._name = value
+        self.title = self.title or value.title()
+
+    def validate(self, value):
         try:
             if self.required and value is None:
                 raise ValueError("Missed value")
@@ -99,38 +110,19 @@ class DataField(object):
             if value is None:
                 return self.default
             if inspect.isclass(self.type) and issubclass(self.type, DataModel):
-                return self.type(**self._ensure_value_is_dict(value))
+                return self.type(**self.ensure_value_is_dict(value))
-            if isinstance(self.type, ListOfType):
+            if inspect.isclass(self.type) and issubclass(self.type, (str, bool)):
-                return self._validate_list_of_type(self.type.type, value)
-            if isinstance(self.type, DictOfType):
-                return self._validate_dict_of_type(self.type.type, value)
-            if issubclass(self.type, (str, bool)):
                 return getattr(self, "_validate_%s_value" % self.type.__name__)(value)
         except ValueError as e:
             raise DataFieldException(self, str(e))
         return value
 
     @staticmethod
-    def _ensure_value_is_dict(value):
+    def ensure_value_is_dict(value):
         if not isinstance(value, dict):
             raise ValueError("Value should be type of dict, not `%s`" % type(value))
         return value
 
-    def _validate_list_of_type(self, list_of_type, value):
-        if not isinstance(value, list):
-            raise ValueError("Value should be a list")
-        if isinstance(list_of_type, DataField):
-            return [list_of_type.validate(self.parent, self.name, v) for v in value]
-        assert issubclass(list_of_type, DataModel)
-        return [list_of_type(**self._ensure_value_is_dict(v)) for v in value]
-
-    def _validate_dict_of_type(self, dict_of_type, value):
-        assert issubclass(dict_of_type, DataModel)
-        value = self._ensure_value_is_dict(value)
-        return {
-            k: dict_of_type(**self._ensure_value_is_dict(v)) for k, v in value.items()
-        }
-
     def _validate_str_value(self, value):
         if not isinstance(value, string_types):
             value = str(value)
@@ -162,21 +154,94 @@ class DataModel(object):
 
     def __init__(self, **kwargs):
         self._field_names = []
-        self._exceptions = []
+        self._exceptions = set()
         for name, field in get_class_attributes(self).items():
             if not isinstance(field, DataField):
                 continue
+            field.parent = self
+            field.name = name
             self._field_names.append(name)
+
+            raw_value = kwargs.get(name)
             value = None
             try:
-                value = field.validate(self, name, kwargs.get(name))
+                if isinstance(field.type, ListOfType):
+                    value = self._validate_list_of_type(field, name, raw_value)
+                elif isinstance(field.type, DictOfType):
+                    value = self._validate_dict_of_type(field, name, raw_value)
+                else:
+                    value = field.validate(raw_value)
             except DataFieldException as e:
-                self._exceptions.append(e)
+                self._exceptions.add(e)
                 if isinstance(self, StrictDataModel):
                     raise e
             finally:
                 setattr(self, name, value)
 
+    def _validate_list_of_type(self, field, name, value):
+        data_type = field.type.type
+        # check if ListOfType is not required
+        value = field.validate(value)
+        if not value:
+            return None
+        if not isinstance(value, list):
+            raise DataFieldException(field, "Value should be a list")
+
+        if isinstance(data_type, DataField):
+            result = []
+            data_type.parent = self
+            data_type.name = name
+            for v in value:
+                try:
+                    result.append(data_type.validate(v))
+                except DataFieldException as e:
+                    self._exceptions.add(e)
+                    if isinstance(self, StrictDataModel):
+                        raise e
+            return result
+
+        assert issubclass(data_type, DataModel)
+
+        result = []
+        for v in value:
+            try:
+                if not isinstance(v, dict):
+                    raise DataFieldException(
+                        field, "Value `%s` should be type of dictionary" % v
+                    )
+                result.append(data_type(**v))
+            except DataFieldException as e:
+                self._exceptions.add(e)
+                if isinstance(self, StrictDataModel):
+                    raise e
+        return result
+
+    def _validate_dict_of_type(self, field, _, value):
+        data_type = field.type.type
+        assert issubclass(data_type, DataModel)
+
+        # check if DictOfType is not required
+        value = field.validate(value)
+        if not value:
+            return None
+        if not isinstance(value, dict):
+            raise DataFieldException(
+                field, "Value `%s` should be type of dictionary" % value
+            )
+        result = {}
+        for k, v in value.items():
+            try:
+                if not isinstance(v, dict):
+                    raise DataFieldException(
+                        field, "Value `%s` should be type of dictionary" % v
+                    )
+                result[k] = data_type(**v)
+            except DataFieldException as e:
+                self._exceptions.add(e)
+                if isinstance(self, StrictDataModel):
+                    raise e
+        return result
+
     def __eq__(self, other):
         assert isinstance(other, DataModel)
         if self.get_field_names() != other.get_field_names():
@@ -195,7 +260,19 @@ class DataModel(object):
         return self._field_names
 
     def get_exceptions(self):
-        return self._exceptions
+        result = list(self._exceptions)
+        for name in self._field_names:
+            value = getattr(self, name)
+            if isinstance(value, DataModel):
+                result.extend(value.get_exceptions())
+                continue
+            if not isinstance(value, (dict, list)):
+                continue
+            for v in value.values() if isinstance(value, dict) else value:
+                if not isinstance(v, DataModel):
+                    continue
+                result.extend(v.get_exceptions())
+        return result
 
     def as_dict(self):
         result = {}

platformio/package/manifest/model.py

@@ -30,6 +30,10 @@ class AuthorModel(DataModel):
     url = DataField(max_length=255)
 
 
+class StrictAuthorModel(AuthorModel, StrictDataModel):
+    pass
+
+
 class RepositoryModel(DataModel):
     type = DataField(max_length=3, required=True)
     url = DataField(max_length=255, required=True)
@@ -44,7 +48,7 @@ class ExportModel(DataModel):
 class ExampleModel(DataModel):
     name = DataField(max_length=100, regex=r"^[a-zA-Z\d\-\_/]+$", required=True)
     base = DataField(required=True)
-    files = DataField(type=ListOfType(DataField()))
+    files = DataField(type=ListOfType(DataField()), required=True)
 
 
 class ManifestModel(DataModel):
@@ -84,4 +88,4 @@ class ManifestModel(DataModel):
 
 
 class StrictManifestModel(ManifestModel, StrictDataModel):
-    pass
+    authors = DataField(type=ListOfType(StrictAuthorModel), required=True)

tests/test_pkgmanifest.py

@@ -14,9 +14,8 @@
 
 import pytest
 
-from platformio.datamodel import DataFieldException
+from platformio import datamodel
-from platformio.package.manifest import parser
+from platformio.package.manifest import model, parser
-from platformio.package.manifest.model import ManifestModel, StrictManifestModel
 
 
 def test_library_json_parser():
@@ -139,31 +138,39 @@ sentence=This is Arduino library
     )
 
     # Platforms ALL
-    mp = parser.LibraryPropertiesManifestParser("architectures=*\n" + contents)
+    data = parser.LibraryPropertiesManifestParser(
-    assert mp.as_dict()["platforms"] == ["*"]
+        "architectures=*\n" + contents
+    ).as_dict()
+    assert data["platforms"] == ["*"]
     # Platforms specific
-    mp = parser.LibraryPropertiesManifestParser("architectures=avr, esp32\n" + contents)
+    data = parser.LibraryPropertiesManifestParser(
-    assert mp.as_dict()["platforms"] == ["atmelavr", "espressif32"]
+        "architectures=avr, esp32\n" + contents
+    ).as_dict()
+    assert data["platforms"] == ["atmelavr", "espressif32"]
 
     # Remote URL
-    mp = parser.LibraryPropertiesManifestParser(
+    data = parser.LibraryPropertiesManifestParser(
         contents,
         remote_url=(
             "https://raw.githubusercontent.com/username/reponame/master/"
             "libraries/TestPackage/library.properties"
         ),
-    )
+    ).as_dict()
-    assert mp.as_dict()["export"] == {
+    assert data["export"] == {
         "exclude": ["extras", "docs", "tests", "test", "*.doxyfile", "*.pdf"],
         "include": "libraries/TestPackage",
     }
+    assert data["repository"] == {
+        "url": "https://github.com/username/reponame",
+        "type": "git",
+    }
 
     # Hope page
-    mp = parser.LibraryPropertiesManifestParser(
+    data = parser.LibraryPropertiesManifestParser(
         "url=https://github.com/username/reponame.git\n" + contents
-    )
+    ).as_dict()
-    assert mp.as_dict()["homepage"] is None
+    assert data["homepage"] is None
-    assert mp.as_dict()["repository"] == {
+    assert data["repository"] == {
         "type": "git",
         "url": "https://github.com/username/reponame.git",
     }
@@ -208,14 +215,14 @@ def test_library_json_model():
   ]
 }
 """
-    mp = parser.ManifestParserFactory.new(
+    data = parser.ManifestParserFactory.new(
         contents, parser.ManifestFileType.LIBRARY_JSON
-    )
+    ).as_dict()
-    model = StrictManifestModel(**mp.as_dict())
+    m = model.StrictManifestModel(**data)
-    assert model.repository.url == "https://github.com/bblanchon/ArduinoJson.git"
+    assert m.repository.url == "https://github.com/bblanchon/ArduinoJson.git"
-    assert model.examples[1].base == "examples/JsonHttpClient"
+    assert m.examples[1].base == "examples/JsonHttpClient"
-    assert model.examples[1].files == ["JsonHttpClient.ino"]
+    assert m.examples[1].files == ["JsonHttpClient.ino"]
-    assert model == StrictManifestModel(
+    assert m == model.StrictManifestModel(
         **{
             "name": "ArduinoJson",
             "keywords": ["json", "rest", "http", "web"],
@@ -270,12 +277,12 @@ category=Display
 url=https://github.com/olikraus/u8glib
 architectures=avr,sam
 """
-    mp = parser.ManifestParserFactory.new(
+    data = parser.ManifestParserFactory.new(
         contents, parser.ManifestFileType.LIBRARY_PROPERTIES
-    )
+    ).as_dict()
-    model = StrictManifestModel(**mp.as_dict())
+    m = model.StrictManifestModel(**data)
-    assert not model.get_exceptions()
+    assert not m.get_exceptions()
-    assert model == StrictManifestModel(
+    assert m == model.StrictManifestModel(
         **{
             "license": None,
             "description": (
@@ -359,14 +366,13 @@ def test_platform_json_model():
   }
 }
 """
-    mp = parser.ManifestParserFactory.new(
+    data = parser.ManifestParserFactory.new(
         contents, parser.ManifestFileType.PLATFORM_JSON
-    )
+    ).as_dict()
-    data = mp.as_dict()
     data["frameworks"] = sorted(data["frameworks"])
-    model = ManifestModel(**mp.as_dict())
+    m = model.ManifestModel(**data)
-    assert model.frameworks == ["arduino", "simba"]
+    assert m.frameworks == ["arduino", "simba"]
-    assert model == ManifestModel(
+    assert m == model.ManifestModel(
         **{
             "name": "atmelavr",
             "title": "Atmel AVR",
@@ -399,13 +405,13 @@ def test_package_json_model():
     "version": "3.30101.0"
 }
 """
-    mp = parser.ManifestParserFactory.new(
+    data = parser.ManifestParserFactory.new(
         contents, parser.ManifestFileType.PACKAGE_JSON
-    )
+    ).as_dict()
-    model = ManifestModel(**mp.as_dict())
+    m = model.ManifestModel(**data)
-    assert model.system is None
+    assert m.system is None
-    assert model.homepage == "http://www.scons.org"
+    assert m.homepage == "http://www.scons.org"
-    assert model == ManifestModel(
+    assert m == model.ManifestModel(
         **{
             "name": "tool-scons",
             "description": "SCons software construction tool",
@@ -491,9 +497,9 @@ def test_examples_from_dir(tmpdir_factory):
         return sorted(items, key=lambda item: item["name"])
 
     data["examples"] = _sort_examples(data["examples"])
-    model = ManifestModel(**data)
+    m = model.ManifestModel(**data)
-    assert model.examples[3].name == "PlatformIO/hello"
+    assert m.examples[3].name == "PlatformIO/hello"
-    assert model == ManifestModel(
+    assert m == model.ManifestModel(
         **{
             "version": "1.0.0",
             "name": "pkg",
@@ -541,26 +547,64 @@ def test_examples_from_dir(tmpdir_factory):
     )
 
 
+def test_dict_of_type():
+    class TestModel(datamodel.DataModel):
+        examples = datamodel.DataField(type=datamodel.DictOfType(model.ExampleModel))
+
+    class StrictTestModel(TestModel, datamodel.StrictDataModel):
+        pass
+
+    # valid
+    m = TestModel(
+        examples={
+            "valid": dict(name="Valid", base="valid", files=["valid.h"]),
+            "invalid": "test",
+        }
+    )
+    assert list(m.examples.keys()) == ["valid"]
+
+    # invalid
+    with pytest.raises(datamodel.DataFieldException):
+        StrictTestModel(examples=[dict(name="Valid", base="valid", files=["valid.h"])])
+
+    with pytest.raises(datamodel.DataFieldException):
+        StrictTestModel(
+            examples={
+                "valid": dict(name="Valid", base="valid", files=["valid.h"]),
+                "invalid": "test",
+            }
+        )
+
+
 def test_broken_models():
     # non-strict mode
-    assert len(ManifestModel(name="MyPackage").get_exceptions()) == 4
+    assert len(model.ManifestModel(name="MyPackage").get_exceptions()) == 4
-    assert ManifestModel(name="MyPackage", version="broken_version").version is None
+    assert (
+        model.ManifestModel(name="MyPackage", version="broken_version").version is None
+    )
+
+    # invalid keywords
+    m = model.ManifestModel(keywords=["kw1", "*^[]"])
+    assert any(
+        "Value `*^[]` does not match RegExp" in str(e) for e in m.get_exceptions()
+    )
+    assert m.keywords == ["kw1"]
 
     # strict mode
 
-    with pytest.raises(DataFieldException) as excinfo:
+    with pytest.raises(datamodel.DataFieldException) as excinfo:
-        assert StrictManifestModel(name="MyPackage")
+        assert model.StrictManifestModel(name="MyPackage")
     assert excinfo.match(r"Missed value for `StrictManifestModel.[a-z]+` field")
 
     # broken SemVer
     with pytest.raises(
-        DataFieldException,
+        datamodel.DataFieldException,
         match=(
             "Invalid semantic versioning format for "
             "`StrictManifestModel.version` field"
         ),
     ):
-        assert StrictManifestModel(
+        assert model.StrictManifestModel(
             name="MyPackage",
             description="MyDescription",
             keywords=["a", "b"],
@@ -569,8 +613,11 @@ def test_broken_models():
         )
 
     # broken value for DataModel
-    with pytest.raises(DataFieldException, match="Value should be type of dict"):
+    with pytest.raises(
-        assert StrictManifestModel(
+        datamodel.DataFieldException,
+        match=("Value `should be dict here` should be type of dictionary"),
+    ):
+        assert model.StrictManifestModel(
             name="MyPackage",
             description="MyDescription",
             keywords=["a", "b"],