Improve repo validation (#3590)

* Improve repo validation

* Add test

* Tweak schemas

* Improve docstring

* Remove V2_BASE_DATA_JSON_SCHEMA

custom_components/hacs/utils/validate.py

@@ -1,6 +1,7 @@
 """Validation utilities."""
 from __future__ import annotations
 
+from collections.abc import Callable
 from dataclasses import dataclass, field
 from typing import Any
 
@@ -70,81 +71,88 @@ def _country_validator(values) -> list[str]:
 )
 
 
+def validate_repo_data(schema: vol.Schema) -> Callable[[Any], Any]:
+    """Return a validator for repo data.
+
+    This is used instead of vol.All to always try both the repo schema and
+    and the validate_version validator.
+    """
+
+    def validate_repo_data(data: Any) -> Any:
+        """Validate integration repo data."""
+        schema_errors: vol.MultipleInvalid | None = None
+        try:
+            schema(data)
+        except vol.MultipleInvalid as err:
+            schema_errors = err
+        try:
+            validate_version(data)
+        except vol.Invalid as err:
+            if schema_errors:
+                schema_errors.add(err)
+            else:
+                raise
+        if schema_errors:
+            raise schema_errors
+        return data
+
+    return validate_repo_data
+
+
 def validate_version(data: Any) -> Any:
     """Ensure at least one of last_commit or last_version is present."""
     if "last_commit" not in data and "last_version" not in data:
         raise vol.Invalid("Expected at least one of [`last_commit`, `last_version`], got none")
     return data
 
 
-V2_BASE_DATA_JSON_SCHEMA = {
-    vol.Required("description"): vol.Any(str, None),
-    vol.Optional("downloads"): int,
-    vol.Optional("etag_releases"): str,
-    vol.Required("etag_repository"): str,
-    vol.Required("full_name"): str,
-    vol.Optional("last_commit"): str,
-    vol.Required("last_fetched"): vol.Any(int, float),
-    vol.Required("last_updated"): str,
-    vol.Optional("last_version"): str,
-    vol.Required("manifest"): {
-        vol.Optional("country"): vol.Any([str], False),
-        vol.Optional("name"): str,
+V2_COMMON_DATA_JSON_SCHEMA = vol.Schema(
+    {
+        vol.Required("description"): vol.Any(str, None),
+        vol.Optional("downloads"): int,
+        vol.Optional("etag_releases"): str,
+        vol.Required("etag_repository"): str,
+        vol.Required("full_name"): str,
+        vol.Optional("last_commit"): str,
+        vol.Required("last_fetched"): vol.Any(int, float),
+        vol.Required("last_updated"): str,
+        vol.Optional("last_version"): str,
+        vol.Required("manifest"): {
+            vol.Optional("country"): vol.Any([str], False),
+            vol.Optional("name"): str,
+        },
+        vol.Optional("open_issues"): int,
+        vol.Optional("stargazers_count"): int,
+        vol.Optional("topics"): [str],
     },
-    vol.Optional("open_issues"): int,
-    vol.Optional("stargazers_count"): int,
-    vol.Optional("topics"): [str],
-}
-
-V2_COMMON_DATA_JSON_SCHEMA = vol.All(
-    vol.Schema(
-        V2_BASE_DATA_JSON_SCHEMA,
-        extra=vol.PREVENT_EXTRA,
-    ),
-    validate_version,
+    extra=vol.PREVENT_EXTRA,
 )
 
-V2_INTEGRATION_DATA_JSON_SCHEMA = vol.All(
-    vol.Schema(
-        {
-            **V2_BASE_DATA_JSON_SCHEMA,
-            vol.Required("domain"): str,
-            vol.Required("manifest_name"): str,
-        },
-        extra=vol.PREVENT_EXTRA,
-    ),
-    validate_version,
+V2_INTEGRATION_DATA_JSON_SCHEMA = V2_COMMON_DATA_JSON_SCHEMA.extend(
+    {
+        vol.Required("domain"): str,
+        vol.Required("manifest_name"): str,
+    },
 )
 
-V2_NETDAEMON_DATA_JSON_SCHEMA = vol.All(
-    vol.Schema(
-        {
-            **V2_BASE_DATA_JSON_SCHEMA,
-            vol.Required("domain"): str,
-        },
-        extra=vol.PREVENT_EXTRA,
-    ),
-    validate_version,
+V2_NETDAEMON_DATA_JSON_SCHEMA = V2_COMMON_DATA_JSON_SCHEMA.extend(
+    {
+        vol.Required("domain"): str,
+    },
 )
 
 V2_REPO_SCHEMA = {
-    "appdaemon": V2_COMMON_DATA_JSON_SCHEMA,
-    "integration": V2_INTEGRATION_DATA_JSON_SCHEMA,
-    "netdaemon": V2_NETDAEMON_DATA_JSON_SCHEMA,
-    "plugin": V2_COMMON_DATA_JSON_SCHEMA,
-    "python_script": V2_COMMON_DATA_JSON_SCHEMA,
-    "template": V2_COMMON_DATA_JSON_SCHEMA,
-    "theme": V2_COMMON_DATA_JSON_SCHEMA,
+    "appdaemon": validate_repo_data(V2_COMMON_DATA_JSON_SCHEMA),
+    "integration": validate_repo_data(V2_INTEGRATION_DATA_JSON_SCHEMA),
+    "netdaemon": validate_repo_data(V2_NETDAEMON_DATA_JSON_SCHEMA),
+    "plugin": validate_repo_data(V2_COMMON_DATA_JSON_SCHEMA),
+    "python_script": validate_repo_data(V2_COMMON_DATA_JSON_SCHEMA),
+    "template": validate_repo_data(V2_COMMON_DATA_JSON_SCHEMA),
+    "theme": validate_repo_data(V2_COMMON_DATA_JSON_SCHEMA),
 }
 
 V2_REPOS_SCHEMA = {
-    "appdaemon": vol.Schema({str: V2_COMMON_DATA_JSON_SCHEMA}),
-    "integration": vol.Schema({str: V2_INTEGRATION_DATA_JSON_SCHEMA}),
-    "netdaemon": vol.Schema({str: V2_NETDAEMON_DATA_JSON_SCHEMA}),
-    "plugin": vol.Schema({str: V2_COMMON_DATA_JSON_SCHEMA}),
-    "python_script": vol.Schema({str: V2_COMMON_DATA_JSON_SCHEMA}),
-    "template": vol.Schema({str: V2_COMMON_DATA_JSON_SCHEMA}),
-    "theme": vol.Schema({str: V2_COMMON_DATA_JSON_SCHEMA}),
+    category: vol.Schema({str: V2_REPO_SCHEMA[category]}) for category in V2_REPO_SCHEMA
 }
 
 V2_CRITICAL_REPO_SCHEMA = vol.Schema(

tests/utils/test_validate.py

@@ -2,7 +2,7 @@
 
 from awesomeversion import AwesomeVersion
 import pytest
-from voluptuous.error import Invalid
+from voluptuous.error import Invalid, MultipleInvalid
 
 from custom_components.hacs.utils.validate import (
     HACS_MANIFEST_JSON_SCHEMA as hacs_json_schema,
@@ -510,6 +510,66 @@ def test_repo_data_json_schema_bad_data(categories: list[str], data: dict, expec
             V2_REPOS_SCHEMA[category]({"test_repo": data})
 
 
+@pytest.mark.parametrize(
+    ("categories", "data"),
+    [
+        # This has multiple errors:
+        # - No last_commit or last_version
+        # - No description
+        # - full_name is wrong type
+        (
+            ["appdaemon", "plugin", "python_script", "template", "theme"],
+            {
+                "etag_repository": "blah",
+                "full_name": 123,
+                "last_fetched": 0,
+                "last_updated": "blah",
+                "manifest": {},
+            },
+        ),
+        (
+            ["integration"],
+            {
+                "domain": "abc",
+                "etag_repository": "blah",
+                "full_name": 123,
+                "last_fetched": 0,
+                "last_updated": "blah",
+                "manifest": {},
+                "manifest_name": "abc",
+            },
+        ),
+    ],
+)
+def test_repo_data_json_schema_multiple_bad_data(categories: list[str], data):
+    """Test validating https://data-v2.hacs.xyz/xxx/data.json with multiple errors.
+
+    This tests we get both dict-based schema errors and custom validation errors.
+    """
+    expected_errors_1 = {
+        ("expected str", ("full_name",)),
+        ("required key not provided", ("description",)),
+        ("Expected at least one of [`last_commit`, `last_version`], got none", ()),
+    }
+    expected_errors_2 = {
+        ("expected str", ("test_repo", "full_name")),
+        ("required key not provided", ("test_repo", "description")),
+        ("Expected at least one of [`last_commit`, `last_version`], got none", ("test_repo",)),
+    }
+    for category in categories:
+        with pytest.raises(MultipleInvalid) as exc_info:
+            V2_REPO_SCHEMA[category](data)
+        msgs = [(err.msg, tuple(err.path)) for err in exc_info.value.errors]
+        assert len(msgs) == 3
+        assert set(msgs) == expected_errors_1
+
+        with pytest.raises(MultipleInvalid) as exc_info:
+            V2_REPOS_SCHEMA[category]({"test_repo": data})
+        msgs = [(err.msg, tuple(err.path)) for err in exc_info.value.errors]
+        assert len(msgs) == 3
+        assert set(msgs) == expected_errors_2
+
+
 def test_removed_repo_data_json_schema():
     """Test validating https://data-v2.hacs.xyz/removed/data.json."""
     data = fixture("v2-removed-data.json")