Refactoring around add-on store Repository classes (#5990)

* Rename repository fixture to test_repository

Also don't remove the built-in repositories. The list was incomplete,
and tests don't seem to require that anymore.

* Get rid of StoreType

The type doesn't have much value, we have constant strings anyways.

* Introduce types.py

* Use slug to determine which repository urls to return

* Simplify BuiltinRepository enum

* Mock GitRepo load

* Improve URL handling and repository creation logic

* Refactor update_repositories

* Get rid of get_from_url

It is no longer used in production code.

* More refactoring

* Address pylint

* Introduce is_git_based property to Repository class

Return all git based URLs, including the Core repository.

* Revert "Introduce is_git_based property to Repository class"

This reverts commit dfd5ad7.

* Fold type.py into const.py

Align more with how Supervisor code is typically structured.

* Update supervisor/store/__init__.py

Co-authored-by: Mike Degatano <[email protected]>

* Apply repository remove suggestion

* Fix tests

---------

Co-authored-by: Mike Degatano <[email protected]>

Author: agners

supervisor/backups/backup.py

@@ -931,5 +931,5 @@ def restore_repositories(self, replace: bool = False) -> Awaitable[None]:
         Return a coroutine.
         """
         return self.sys_store.update_repositories(
-            self.repositories, add_with_errors=True, replace=replace
+            self.repositories, issue_on_error=True, replace=replace
         )

supervisor/store/__init__.py

@@ -4,7 +4,7 @@
 from collections.abc import Awaitable
 import logging
 
-from ..const import ATTR_REPOSITORIES, URL_HASSIO_ADDONS
+from ..const import ATTR_REPOSITORIES, REPOSITORY_CORE, URL_HASSIO_ADDONS
 from ..coresys import CoreSys, CoreSysAttributes
 from ..exceptions import (
     StoreError,
@@ -18,14 +18,10 @@
 from ..resolution.const import ContextType, IssueType, SuggestionType
 from ..utils.common import FileConfiguration
 from .addon import AddonStore
-from .const import FILE_HASSIO_STORE, StoreType
+from .const import FILE_HASSIO_STORE, BuiltinRepository
 from .data import StoreData
 from .repository import Repository
-from .validate import (
-    BUILTIN_REPOSITORIES,
-    SCHEMA_STORE_FILE,
-    ensure_builtin_repositories,
-)
+from .validate import SCHEMA_STORE_FILE, ensure_builtin_repositories
 
 _LOGGER: logging.Logger = logging.getLogger(__name__)
 
@@ -56,7 +52,8 @@ def repository_urls(self) -> list[str]:
         return [
             repository.source
             for repository in self.all
-            if repository.type == StoreType.GIT
+            if repository.slug
+            not in {BuiltinRepository.LOCAL.value, BuiltinRepository.CORE.value}
         ]
 
     def get(self, slug: str) -> Repository:
@@ -65,19 +62,11 @@ def get(self, slug: str) -> Repository:
             raise StoreNotFound()
         return self.repositories[slug]
 
-    def get_from_url(self, url: str) -> Repository:
-        """Return Repository with slug."""
-        for repository in self.all:
-            if repository.source != url:
-                continue
-            return repository
-        raise StoreNotFound()
-
     async def load(self) -> None:
         """Start up add-on management."""
         # Init custom repositories and load add-ons
         await self.update_repositories(
-            self._data[ATTR_REPOSITORIES], add_with_errors=True
+            self._data[ATTR_REPOSITORIES], issue_on_error=True
         )
 
     @Job(
@@ -126,14 +115,14 @@ async def reload(self, repository: Repository | None = None) -> None:
     )
     async def add_repository(self, url: str, *, persist: bool = True) -> None:
         """Add a repository."""
-        await self._add_repository(url, persist=persist, add_with_errors=False)
+        await self._add_repository(url, persist=persist, issue_on_error=False)
 
     async def _add_repository(
-        self, url: str, *, persist: bool = True, add_with_errors: bool = False
+        self, url: str, *, persist: bool = True, issue_on_error: bool = False
     ) -> None:
         """Add a repository."""
         if url == URL_HASSIO_ADDONS:
-            url = StoreType.CORE
+            url = REPOSITORY_CORE
 
         repository = Repository.create(self.coresys, url)
 
@@ -145,7 +134,7 @@ async def _add_repository(
             await repository.load()
         except StoreGitCloneError as err:
             _LOGGER.error("Can't retrieve data from %s due to %s", url, err)
-            if add_with_errors:
+            if issue_on_error:
                 self.sys_resolution.create_issue(
                     IssueType.FATAL_ERROR,
                     ContextType.STORE,
@@ -158,7 +147,7 @@ async def _add_repository(
 
         except StoreGitError as err:
             _LOGGER.error("Can't load data from repository %s due to %s", url, err)
-            if add_with_errors:
+            if issue_on_error:
                 self.sys_resolution.create_issue(
                     IssueType.FATAL_ERROR,
                     ContextType.STORE,
@@ -171,7 +160,7 @@ async def _add_repository(
 
         except StoreJobError as err:
             _LOGGER.error("Can't add repository %s due to %s", url, err)
-            if add_with_errors:
+            if issue_on_error:
                 self.sys_resolution.create_issue(
                     IssueType.FATAL_ERROR,
                     ContextType.STORE,
@@ -184,7 +173,7 @@ async def _add_repository(
 
         else:
             if not await repository.validate():
-                if add_with_errors:
+                if issue_on_error:
                     _LOGGER.error("%s is not a valid add-on repository", url)
                     self.sys_resolution.create_issue(
                         IssueType.CORRUPT_REPOSITORY,
@@ -213,7 +202,7 @@ async def _add_repository(
 
     async def remove_repository(self, repository: Repository, *, persist: bool = True):
         """Remove a repository."""
-        if repository.source in BUILTIN_REPOSITORIES:
+        if repository.is_builtin:
             raise StoreInvalidAddonRepo(
                 "Can't remove built-in repositories!", logger=_LOGGER.error
             )
@@ -236,38 +225,54 @@ async def update_repositories(
         self,
         list_repositories: list[str],
         *,
-        add_with_errors: bool = False,
+        issue_on_error: bool = False,
         replace: bool = True,
     ):
-        """Add a new custom repository."""
-        new_rep = set(
-            ensure_builtin_repositories(list_repositories)
-            if replace
-            else list_repositories + self.repository_urls
-        )
-        old_rep = {repository.source for repository in self.all}
+        """Update repositories by adding new ones and removing stale ones."""
+        current_repositories = {repository.source for repository in self.all}
+
+        # Determine changes needed
+        if replace:
+            target_repositories = set(ensure_builtin_repositories(list_repositories))
+            repositories_to_add = target_repositories - current_repositories
+        else:
+            # When not replacing, just add the new repositories
+            repositories_to_add = set(list_repositories) - current_repositories
+            target_repositories = current_repositories | repositories_to_add
 
         # Add new repositories
         add_errors = await asyncio.gather(
             *[
-                self._add_repository(url, persist=False, add_with_errors=True)
-                if add_with_errors
+                # Use _add_repository to avoid JobCondition.SUPERVISOR_UPDATED
+                # to prevent proper loading of repositories on startup.
+                self._add_repository(url, persist=False, issue_on_error=True)
+                if issue_on_error
                 else self.add_repository(url, persist=False)
-                for url in new_rep - old_rep
+                for url in repositories_to_add
             ],
             return_exceptions=True,
         )
 
-        # Delete stale repositories
-        remove_errors = await asyncio.gather(
-            *[
-                self.remove_repository(self.get_from_url(url), persist=False)
-                for url in old_rep - new_rep - BUILTIN_REPOSITORIES
-            ],
-            return_exceptions=True,
-        )
+        remove_errors: list[BaseException | None] = []
+        if replace:
+            # Determine repositories to remove
+            repositories_to_remove: list[Repository] = [
+                repository
+                for repository in self.all
+                if repository.source not in target_repositories
+                and not repository.is_builtin
+            ]
+
+            # Remove repositories
+            remove_errors = await asyncio.gather(
+                *[
+                    self.remove_repository(repository, persist=False)
+                    for repository in repositories_to_remove
+                ],
+                return_exceptions=True,
+            )
 
-        # Always update data, even there are errors, some changes may have succeeded
+        # Always update data, even if there are errors, some changes may have succeeded
         await self.data.update()
         await self._read_addons()
 

supervisor/store/const.py

@@ -3,14 +3,39 @@
 from enum import StrEnum
 from pathlib import Path
 
-from ..const import SUPERVISOR_DATA
+from ..const import (
+    REPOSITORY_CORE,
+    REPOSITORY_LOCAL,
+    SUPERVISOR_DATA,
+    URL_HASSIO_ADDONS,
+)
 
 FILE_HASSIO_STORE = Path(SUPERVISOR_DATA, "store.json")
+"""Repository type definitions for the store."""
 
 
-class StoreType(StrEnum):
-    """Store Types."""
+class BuiltinRepository(StrEnum):
+    """All built-in repositories that come pre-configured."""
 
-    CORE = "core"
-    LOCAL = "local"
-    GIT = "git"
+    # Local repository (non-git, special handling)
+    LOCAL = REPOSITORY_LOCAL
+
+    # Git-based built-in repositories
+    CORE = REPOSITORY_CORE
+    COMMUNITY_ADDONS = "https://github.com/hassio-addons/repository"
+    ESPHOME = "https://github.com/esphome/home-assistant-addon"
+    MUSIC_ASSISTANT = "https://github.com/music-assistant/home-assistant-addon"
+
+    @property
+    def git_url(self) -> str:
+        """Return the git URL for this repository."""
+        if self == BuiltinRepository.LOCAL:
+            raise RuntimeError("Local repository does not have a git URL")
+        if self == BuiltinRepository.CORE:
+            return URL_HASSIO_ADDONS
+        else:
+            return self.value  # For URL-based repos, value is the URL
+
+
+# All repositories that are considered "built-in" and protected from removal
+ALL_BUILTIN_REPOSITORIES = {repo.value for repo in BuiltinRepository}

supervisor/store/data.py

@@ -25,7 +25,6 @@
 from ..resolution.const import ContextType, IssueType, SuggestionType, UnhealthyReason
 from ..utils.common import find_one_filetype, read_json_or_yaml_file
 from ..utils.json import read_json_file
-from .const import StoreType
 from .utils import extract_hash_from_path
 from .validate import SCHEMA_REPOSITORY_CONFIG
 
@@ -169,7 +168,7 @@ def _get_addons_list() -> list[Path]:
                 self.sys_resolution.add_unhealthy_reason(
                     UnhealthyReason.OSERROR_BAD_MESSAGE
                 )
-            elif path.stem != StoreType.LOCAL:
+            elif repository != REPOSITORY_LOCAL:
                 suggestion = [SuggestionType.EXECUTE_RESET]
             self.sys_resolution.create_issue(
                 IssueType.CORRUPT_REPOSITORY,