Avoid reordering add-on repositories on Backup load (#5595)

* Avoid reordering add-on repositories on Backup load

The `ensure_builtin_repositories` function uses a set to deduplicate
items, which sometimes led to a change of order in elements. This is
problematic when deduplicating Backups.

Simply avoid mangling the list of add-on repositories on load. Instead
rely on `update_repositories` which uses the same function to ensure
built-in repositories when loading the store configuration and restoring
a backup file.

* Update tests

* ruff format

* ruff check

* ruff check fixes

* ruff format

* Update tests/store/test_validate.py

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Simplify test

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

supervisor/backups/backup.py

@@ -162,12 +162,12 @@ class Backup(JobGroup):
 
     @property
     def repositories(self) -> list[str]:
-        """Return backup date."""
+        """Return add-on store repositories."""
         return self._data[ATTR_REPOSITORIES]
 
     @repositories.setter
     def repositories(self, value: list[str]) -> None:
-        """Set backup date."""
+        """Set add-on store repositories."""
         self._data[ATTR_REPOSITORIES] = value
 
     @property
@@ -286,7 +286,7 @@ class Backup(JobGroup):
                 or k not in other._data
                 or self._data[k] != other._data[k]
             ):
-                _LOGGER.debug(
+                _LOGGER.info(
                     "Backup %s and %s not equal because %s field has different value: %s and %s",
                     self.slug,
                     other.slug,

supervisor/store/validate.py

@@ -28,6 +28,15 @@ SCHEMA_REPOSITORY_CONFIG = vol.Schema(
 )
 
 
+def ensure_builtin_repositories(addon_repositories: list[str]) -> list[str]:
+    """Ensure builtin repositories are in list.
+
+    Note: This should not be used in validation as the resulting list is not
+    stable. This can have side effects when comparing data later on.
+    """
+    return list(set(addon_repositories) | BUILTIN_REPOSITORIES)
+
+
 def validate_repository(repository: str) -> str:
     """Validate a valid repository."""
     if repository in [StoreType.CORE, StoreType.LOCAL]:
@@ -44,13 +53,7 @@ def validate_repository(repository: str) -> str:
     return repository
 
 
-def ensure_builtin_repositories(addon_repositories: list[str]) -> list[str]:
+repositories = vol.All([validate_repository], vol.Unique())
-    """Ensure builtin repositories are in list."""
-    return list(set(addon_repositories) | BUILTIN_REPOSITORIES)
-
-
-# pylint: disable=no-value-for-parameter
-repositories = vol.All([validate_repository], vol.Unique(), ensure_builtin_repositories)
 
 SCHEMA_STORE_FILE = vol.Schema(
     {

tests/backups/test_backup.py

@@ -45,3 +45,30 @@ async def test_consolidate_conflict_varied_encryption(
         in caplog.text
     )
     assert enc_backup.all_locations == {None: {"path": unc_tar, "protected": False}}
+
+
+async def test_consolidate(
+    coresys: CoreSys,
+    tmp_path: Path,
+    tmp_supervisor_data: Path,
+    caplog: pytest.LogCaptureFixture,
+):
+    """Test consolidate with two backups in different location and varied encryption."""
+    (mount_dir := coresys.config.path_mounts / "backup_test").mkdir()
+    enc_tar = Path(copy(get_fixture_path("test_consolidate.tar"), tmp_path))
+    enc_backup = Backup(coresys, enc_tar, "test", None)
+    await enc_backup.load()
+
+    unc_tar = Path(copy(get_fixture_path("test_consolidate_unc.tar"), mount_dir))
+    unc_backup = Backup(coresys, unc_tar, "test", "backup_test")
+    await unc_backup.load()
+
+    enc_backup.consolidate(unc_backup)
+    assert (
+        "Backup in backup_test and None both have slug d9c48f8b but are not the same!"
+        not in caplog.text
+    )
+    assert enc_backup.all_locations == {
+        None: {"path": enc_tar, "protected": True},
+        "backup_test": {"path": unc_tar, "protected": False},
+    }

tests/store/test_validate.py

@@ -1,45 +1,14 @@
 """Test schema validation."""
 
-from typing import Any
-
 import pytest
 from voluptuous import Invalid
 
-from supervisor.const import ATTR_REPOSITORIES
+from supervisor.store.validate import repositories
-from supervisor.store.validate import SCHEMA_STORE_FILE, repositories
-
-
-@pytest.mark.parametrize(
-    "config",
-    [
-        {},
-        {ATTR_REPOSITORIES: []},
-        {ATTR_REPOSITORIES: ["https://github.com/esphome/home-assistant-addon"]},
-    ],
-)
-async def test_default_config(config: dict[Any]):
-    """Test built-ins included by default."""
-    conf = SCHEMA_STORE_FILE(config)
-    assert ATTR_REPOSITORIES in conf
-    assert "core" in conf[ATTR_REPOSITORIES]
-    assert "local" in conf[ATTR_REPOSITORIES]
-    assert "https://github.com/hassio-addons/repository" in conf[ATTR_REPOSITORIES]
-    assert (
-        len(
-            [
-                repo
-                for repo in conf[ATTR_REPOSITORIES]
-                if repo == "https://github.com/esphome/home-assistant-addon"
-            ]
-        )
-        == 1
-    )
 
 
 @pytest.mark.parametrize(
     "repo_list,valid",
     [
-        ([], True),
         (["core", "local"], True),
         (["https://github.com/hassio-addons/repository"], True),
         (["not_a_url"], False),
@@ -49,15 +18,7 @@ async def test_default_config(config: dict[Any]):
 async def test_repository_validate(repo_list: list[str], valid: bool):
     """Test repository list validate."""
     if valid:
-        processed = repositories(repo_list)
+        assert repositories(repo_list) == repo_list
-        assert len(processed) == 5
-        assert set(repositories(repo_list)) == {
-            "core",
-            "local",
-            "https://github.com/hassio-addons/repository",
-            "https://github.com/esphome/home-assistant-addon",
-            "https://github.com/music-assistant/home-assistant-addon",
-        }
     else:
         with pytest.raises(Invalid):
             repositories(repo_list)