From 36acdd77976cddf24b8d88b6f1262744af5586e6 Mon Sep 17 00:00:00 2001 From: Ivan Kravets Date: Fri, 4 Oct 2019 18:30:48 +0300 Subject: [PATCH] DataModel: allow valid values in non-strict mode for TypeOfList and TypeOfDict --- platformio/datamodel.py | 145 ++++++++++++++++++++------- platformio/package/manifest/model.py | 8 +- tests/test_pkgmanifest.py | 145 ++++++++++++++++++--------- 3 files changed, 213 insertions(+), 85 deletions(-) diff --git a/platformio/datamodel.py b/platformio/datamodel.py index 3dc4111f..53b99c29 100644 --- a/platformio/datamodel.py +++ b/platformio/datamodel.py @@ -74,8 +74,8 @@ class DataField(object): self.validate_factory = validate_factory self.title = title - self.parent = None - self.name = None + self._parent = 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( - self, parent, name, value - ): # pylint: disable=too-many-return-statements - self.parent = parent - self.name = name - self.title = self.title or name.title() + @property + def parent(self): + return self._parent + @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)) - if isinstance(self.type, ListOfType): - 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 self.type(**self.ensure_value_is_dict(value)) + if inspect.isclass(self.type) and 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 = {} diff --git a/platformio/package/manifest/model.py b/platformio/package/manifest/model.py index f881a370..23900777 100644 --- a/platformio/package/manifest/model.py +++ b/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) diff --git a/tests/test_pkgmanifest.py b/tests/test_pkgmanifest.py index 027733a1..3bca641b 100644 --- a/tests/test_pkgmanifest.py +++ b/tests/test_pkgmanifest.py @@ -14,9 +14,8 @@ import pytest -from platformio.datamodel import DataFieldException -from platformio.package.manifest import parser -from platformio.package.manifest.model import ManifestModel, StrictManifestModel +from platformio import datamodel +from platformio.package.manifest import model, parser def test_library_json_parser(): @@ -139,31 +138,39 @@ sentence=This is Arduino library ) # Platforms ALL - mp = parser.LibraryPropertiesManifestParser("architectures=*\n" + contents) - assert mp.as_dict()["platforms"] == ["*"] + data = parser.LibraryPropertiesManifestParser( + "architectures=*\n" + contents + ).as_dict() + assert data["platforms"] == ["*"] # Platforms specific - mp = parser.LibraryPropertiesManifestParser("architectures=avr, esp32\n" + contents) - assert mp.as_dict()["platforms"] == ["atmelavr", "espressif32"] + data = parser.LibraryPropertiesManifestParser( + "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" ), - ) - assert mp.as_dict()["export"] == { + ).as_dict() + 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 - ) - assert mp.as_dict()["homepage"] is None - assert mp.as_dict()["repository"] == { + ).as_dict() + assert data["homepage"] is None + 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 - ) - model = StrictManifestModel(**mp.as_dict()) - assert model.repository.url == "https://github.com/bblanchon/ArduinoJson.git" - assert model.examples[1].base == "examples/JsonHttpClient" - assert model.examples[1].files == ["JsonHttpClient.ino"] - assert model == StrictManifestModel( + ).as_dict() + m = model.StrictManifestModel(**data) + assert m.repository.url == "https://github.com/bblanchon/ArduinoJson.git" + assert m.examples[1].base == "examples/JsonHttpClient" + assert m.examples[1].files == ["JsonHttpClient.ino"] + 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 - ) - model = StrictManifestModel(**mp.as_dict()) - assert not model.get_exceptions() - assert model == StrictManifestModel( + ).as_dict() + m = model.StrictManifestModel(**data) + assert not m.get_exceptions() + 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 - ) - data = mp.as_dict() + ).as_dict() data["frameworks"] = sorted(data["frameworks"]) - model = ManifestModel(**mp.as_dict()) - assert model.frameworks == ["arduino", "simba"] - assert model == ManifestModel( + m = model.ManifestModel(**data) + assert m.frameworks == ["arduino", "simba"] + 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 - ) - model = ManifestModel(**mp.as_dict()) - assert model.system is None - assert model.homepage == "http://www.scons.org" - assert model == ManifestModel( + ).as_dict() + m = model.ManifestModel(**data) + assert m.system is None + assert m.homepage == "http://www.scons.org" + 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) - assert model.examples[3].name == "PlatformIO/hello" - assert model == ManifestModel( + m = model.ManifestModel(**data) + assert m.examples[3].name == "PlatformIO/hello" + 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 ManifestModel(name="MyPackage", version="broken_version").version is None + assert len(model.ManifestModel(name="MyPackage").get_exceptions()) == 4 + 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: - assert StrictManifestModel(name="MyPackage") + with pytest.raises(datamodel.DataFieldException) as excinfo: + 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"): - assert StrictManifestModel( + with pytest.raises( + datamodel.DataFieldException, + match=("Value `should be dict here` should be type of dictionary"), + ): + assert model.StrictManifestModel( name="MyPackage", description="MyDescription", keywords=["a", "b"],