Strict manifest validation when submitting to Registry, more tests for manifest model

platformio/commands/lib.py

@@ -25,7 +25,7 @@ from platformio import exception, util
 from platformio.commands import PlatformioCLI
 from platformio.compat import dump_json_to_unicode
 from platformio.managers.lib import LibraryManager, get_builtin_libs, is_builtin_lib
-from platformio.package.manifest.model import ManifestModel
+from platformio.package.manifest.model import StrictManifestModel
 from platformio.package.manifest.parser import ManifestFactory
 from platformio.proc import is_ci
 from platformio.project.config import ProjectConfig
@@ -494,8 +494,8 @@ def lib_register(config_url):
     if not config_url.startswith("http://") and not config_url.startswith("https://"):
         raise exception.InvalidLibConfURL(config_url)
 
-    model = ManifestModel(**ManifestFactory.new_from_url(config_url).as_dict())
+    # Validate manifest
-    assert set(["name", "version"]) & set(list(model.as_dict()))
+    StrictManifestModel(**ManifestFactory.new_from_url(config_url).as_dict())
 
     result = util.get_api_result("/lib/register", data=dict(config_url=config_url))
     if "message" in result and result["message"]:

platformio/datamodel.py

@@ -69,7 +69,7 @@ class DataField(object):
 
         try:
             if self.required and value is None:
-                raise ValueError("Required field `%s` is None" % name)
+                raise ValueError("Missed value")
             if self.validate_factory is not None:
                 return self.validate_factory(self, value) or self.default
             if value is None:
@@ -82,7 +82,7 @@ class DataField(object):
                 return getattr(self, "_validate_%s_value" % self.type.__name__)(value)
         except ValueError as e:
             raise DataModelException(
-                "%s for %s.%s" % (str(e), parent.__class__.__name__, name)
+                "%s for `%s.%s` field" % (str(e), parent.__class__.__name__, name)
             )
         return value
 
@@ -127,13 +127,11 @@ class DataModel(object):
             self._known_attributes.append(name)
             setattr(self, name, field.validate(self, name, kwargs.get(name)))
 
-    # def __repr__(self):
+    def __repr__(self):
-    #     attrs = []
+        fields = []
-    #     for name, value in get_class_attributes(self).items():
+        for name in self._known_attributes:
-    #         if name in self.__PRIVATE_ATTRIBUTES__:
+            fields.append('%s="%s"' % (name, getattr(self, name)))
-    #             continue
+        return "<%s %s>" % (self.__class__.__name__, " ".join(fields))
-    #         attrs.append('%s="%s"' % (name, value))
-    #     return "<%s %s>" % (self.__class__.__name__, " ".join(attrs))
 
     def as_dict(self):
         return {name: getattr(self, name) for name in self._known_attributes}

platformio/package/manifest/model.py

@@ -17,6 +17,12 @@ import semantic_version
 from platformio.datamodel import DataField, DataModel, ListOfType
 
 
+def validate_semver_field(_, value):
+    if "." not in value:
+        raise ValueError("Invalid semantic versioning format")
+    return value if semantic_version.Version.coerce(value) else None
+
+
 class AuthorModel(DataModel):
     name = DataField(max_length=50, required=True)
     email = DataField(max_length=50)
@@ -40,18 +46,14 @@ class ManifestModel(DataModel):
     # Required fields
     name = DataField(max_length=100, required=True)
     version = DataField(
-        max_length=50,
+        max_length=50, validate_factory=validate_semver_field, required=True
-        validate_factory=lambda field, value: value
-        if semantic_version.Version.coerce(value)
-        else None,
-        required=True,
     )
-    description = DataField(max_length=1000, required=True)
+
+    description = DataField(max_length=1000)
     keywords = DataField(
-        type=ListOfType(DataField(max_length=255, regex=r"^[a-z][a-z\d\- ]*[a-z]$")),
+        type=ListOfType(DataField(max_length=255, regex=r"^[a-z][a-z\d\- ]*[a-z]$"))
-        required=True,
     )
-    authors = DataField(type=ListOfType(AuthorModel), required=True)
+    authors = DataField(type=ListOfType(AuthorModel))
 
     homepage = DataField(max_length=255)
     license = DataField(max_length=255)
@@ -64,3 +66,10 @@ class ManifestModel(DataModel):
 
     repository = DataField(type=RepositoryModel)
     export = DataField(type=ExportModel)
+
+
+class StrictManifestModel(ManifestModel):
+    def __init__(self, *args, **kwargs):
+        for name in ("description", "keywords", "authors"):
+            getattr(self, name).required = True
+        super(StrictManifestModel, self).__init__(*args, **kwargs)

platformio/package/manifest/parser.py

@@ -232,15 +232,15 @@ class LibraryPropertiesManifestParser(BaseManifestParser):
         if repository and repository["url"] == homepage:
             homepage = None
         return dict(
-            name=properties.get("name"),
-            version=properties.get("version"),
-            description=properties.get("sentence"),
             frameworks=["arduino"],
-            platforms=self._process_platforms(properties) or ["*"],
-            keywords=self._parse_keywords(properties),
-            authors=self._parse_authors(properties) or None,
             homepage=homepage,
             repository=repository or None,
+            name=properties.get("name"),
+            version=properties.get("version"),
+            description=self._parse_description(properties),
+            platforms=self._parse_platforms(properties) or ["*"],
+            keywords=self._parse_keywords(properties),
+            authors=self._parse_authors(properties) or None,
             export=self._parse_export(),
         )
 
@@ -258,6 +258,16 @@ class LibraryPropertiesManifestParser(BaseManifestParser):
             data[key.strip()] = value.strip()
         return data
 
+    @staticmethod
+    def _parse_description(properties):
+        lines = []
+        for k in ("sentence", "paragraph"):
+            if k in properties and properties[k] not in lines:
+                lines.append(properties[k])
+        if len(lines) == 2 and not lines[0].endswith("."):
+            lines[0] += "."
+        return " ".join(lines)
+
     @staticmethod
     def _parse_keywords(properties):
         result = []
@@ -269,7 +279,7 @@ class LibraryPropertiesManifestParser(BaseManifestParser):
         return result
 
     @staticmethod
-    def _process_platforms(properties):
+    def _parse_platforms(properties):
         result = []
         platforms_map = {
             "avr": "atmelavr",

tests/test_pkgmanifest.py

@@ -16,7 +16,7 @@ import pytest
 
 from platformio.datamodel import DataModelException
 from platformio.package.manifest import parser
-from platformio.package.manifest.model import ManifestModel
+from platformio.package.manifest.model import ManifestModel, StrictManifestModel
 
 
 def test_library_json_parser():
@@ -207,3 +207,78 @@ def test_library_json_valid_model():
             "license": "MIT",
         }.items()
     )
+
+
+def test_library_properties_valid_model():
+    contents = """
+name=U8glib
+version=1.19.1
+author=oliver <olikraus@gmail.com>
+maintainer=oliver <olikraus@gmail.com>
+sentence=A library for monochrome TFTs and OLEDs
+paragraph=Supported display controller: SSD1306, SSD1309, SSD1322, SSD1325
+category=Display
+url=https://github.com/olikraus/u8glib
+architectures=avr,sam
+"""
+    data = parser.ManifestFactory.new(
+        contents, parser.ManifestFileType.LIBRARY_PROPERTIES
+    )
+    model = ManifestModel(**data.as_dict())
+    assert sorted(model.as_dict().items()) == sorted(
+        {
+            "license": None,
+            "description": (
+                "A library for monochrome TFTs and OLEDs. Supported display "
+                "controller: SSD1306, SSD1309, SSD1322, SSD1325"
+            ),
+            "repository": {
+                "url": "https://github.com/olikraus/u8glib",
+                "type": "git",
+                "branch": None,
+            },
+            "frameworks": ["arduino"],
+            "platforms": ["atmelavr", "atmelsam"],
+            "version": "1.19.1",
+            "export": {
+                "exclude": ["extras", "docs", "tests", "test", "*.doxyfile", "*.pdf"],
+                "include": None,
+            },
+            "authors": [
+                {
+                    "url": None,
+                    "maintainer": True,
+                    "email": "olikraus@gmail.com",
+                    "name": "oliver",
+                }
+            ],
+            "keywords": ["display"],
+            "homepage": None,
+            "name": "U8glib",
+        }.items()
+    )
+
+
+def test_broken_model():
+    # "version" field is required
+    with pytest.raises(
+        DataModelException, match="Missed value for `ManifestModel.version` field"
+    ):
+        assert ManifestModel(name="MyPackage")
+
+    # broken SemVer
+    with pytest.raises(
+        DataModelException,
+        match="Invalid semantic versioning format for `ManifestModel.version` field",
+    ):
+        assert ManifestModel(name="MyPackage", version="broken_version")
+
+    # the only name and version fields are required for base ManifestModel
+    assert ManifestModel(name="MyPackage", version="1.0")
+
+    # check strict model
+    with pytest.raises(
+        DataModelException,
+        match="Missed value for `StrictManifestModel.description` field",
+    ):
+        assert StrictManifestModel(name="MyPackage", version="1.0")