Refactor addon git repo (#5987)

* Refactor Repository into setup with inheritance

* Remove subclasses of GitRepo

Author: mdegat01

supervisor/resolution/fixups/store_execute_reset.py

@@ -42,13 +42,8 @@ async def process_fixup(self, reference: str | None = None) -> None:
             _LOGGER.warning("Can't find store %s for fixup", reference)
             return
 
-        # Local add-ons are not a git repo, can't remove and re-pull
         try:
-            if repository.git:
-                await repository.git.reset()
-
-            # Load data again
-            await repository.load()
+            await repository.reset()
         except StoreError:
             raise ResolutionFixupError() from None
 

supervisor/store/__init__.py

@@ -135,7 +135,7 @@ async def _add_repository(
         if url == URL_HASSIO_ADDONS:
             url = StoreType.CORE
 
-        repository = Repository(self.coresys, url)
+        repository = Repository.create(self.coresys, url)
 
         if repository.slug in self.repositories:
             raise StoreError(f"Can't add {url}, already in the store", _LOGGER.error)
@@ -183,7 +183,7 @@ async def _add_repository(
                 raise err
 
         else:
-            if not await self.sys_run_in_executor(repository.validate):
+            if not await repository.validate():
                 if add_with_errors:
                     _LOGGER.error("%s is not a valid add-on repository", url)
                     self.sys_resolution.create_issue(

supervisor/store/git.py

@@ -1,6 +1,5 @@
 """Init file for Supervisor add-on Git."""
 
-from abc import ABC, abstractmethod
 import asyncio
 import errno
 import functools as ft
@@ -16,17 +15,14 @@
 from ..jobs.decorator import Job, JobCondition
 from ..resolution.const import ContextType, IssueType, SuggestionType, UnhealthyReason
 from ..utils import remove_folder
-from .utils import get_hash_from_repository
-from .validate import RE_REPOSITORY, BuiltinRepository
+from .validate import RE_REPOSITORY
 
 _LOGGER: logging.Logger = logging.getLogger(__name__)
 
 
-class GitRepo(CoreSysAttributes, ABC):
+class GitRepo(CoreSysAttributes):
     """Manage Add-on Git repository."""
 
-    builtin: bool
-
     def __init__(self, coresys: CoreSys, path: Path, url: str):
         """Initialize Git base wrapper."""
         self.coresys: CoreSys = coresys
@@ -239,38 +235,8 @@ async def pull(self) -> bool:
                 )
                 raise StoreGitError() from err
 
-    @abstractmethod
     async def remove(self) -> None:
         """Remove a repository."""
-
-
-class GitRepoBuiltin(GitRepo):
-    """Built-in add-ons repository."""
-
-    builtin: bool = True
-
-    def __init__(self, coresys: CoreSys, repository: BuiltinRepository):
-        """Initialize Git Supervisor add-on repository."""
-        super().__init__(coresys, repository.get_path(coresys), repository.url)
-
-    async def remove(self) -> None:
-        """Raise. Cannot remove built-in repositories."""
-        raise RuntimeError("Cannot remove built-in repositories!")
-
-
-class GitRepoCustom(GitRepo):
-    """Custom add-ons repository."""
-
-    builtin: bool = False
-
-    def __init__(self, coresys, url):
-        """Initialize custom Git Supervisor addo-n repository."""
-        path = Path(coresys.config.path_addons_git, get_hash_from_repository(url))
-
-        super().__init__(coresys, path, url)
-
-    async def remove(self) -> None:
-        """Remove a custom repository."""
         if self.lock.locked():
             _LOGGER.warning(
                 "Cannot remove add-on repository %s, there is already a task in progress",

supervisor/store/repository.py

@@ -1,5 +1,8 @@
 """Represent a Supervisor repository."""
 
+from __future__ import annotations
+
+from abc import ABC, abstractmethod
 import logging
 from pathlib import Path
 
@@ -12,36 +15,32 @@
 from ..exceptions import ConfigurationFileError, StoreError
 from ..utils.common import read_json_or_yaml_file
 from .const import StoreType
-from .git import GitRepo, GitRepoBuiltin, GitRepoCustom
+from .git import GitRepo
 from .utils import get_hash_from_repository
 from .validate import SCHEMA_REPOSITORY_CONFIG, BuiltinRepository
 
 _LOGGER: logging.Logger = logging.getLogger(__name__)
 UNKNOWN = "unknown"
 
 
-class Repository(CoreSysAttributes):
+class Repository(CoreSysAttributes, ABC):
     """Add-on store repository in Supervisor."""
 
     def __init__(self, coresys: CoreSys, repository: str):
         """Initialize add-on store repository object."""
+        self._slug: str
+        self._type: StoreType
         self.coresys: CoreSys = coresys
-        self.git: GitRepo | None = None
-
         self.source: str = repository
+
+    @staticmethod
+    def create(coresys: CoreSys, repository: str) -> Repository:
+        """Create a repository instance."""
         if repository == StoreType.LOCAL:
-            self._slug = repository
-            self._type = StoreType.LOCAL
-            self._latest_mtime: float | None = None
-        elif repository in BuiltinRepository:
-            builtin = BuiltinRepository(repository)
-            self.git = GitRepoBuiltin(coresys, builtin)
-            self._slug = builtin.id
-            self._type = builtin.type
-        else:
-            self.git = GitRepoCustom(coresys, repository)
-            self._slug = get_hash_from_repository(repository)
-            self._type = StoreType.GIT
+            return RepositoryLocal(coresys)
+        if repository in BuiltinRepository:
+            return RepositoryGitBuiltin(coresys, BuiltinRepository(repository))
+        return RepositoryCustom(coresys, repository)
 
     def __repr__(self) -> str:
         """Return internal representation."""
@@ -77,52 +76,117 @@ def maintainer(self) -> str:
         """Return url of repository."""
         return self.data.get(ATTR_MAINTAINER, UNKNOWN)
 
-    def validate(self) -> bool:
-        """Check if store is valid.
+    @abstractmethod
+    async def validate(self) -> bool:
+        """Check if store is valid."""
+
+    @abstractmethod
+    async def load(self) -> None:
+        """Load addon repository."""
+
+    @abstractmethod
+    async def update(self) -> bool:
+        """Update add-on repository.
 
-        Must be run in executor.
+        Returns True if the repository was updated.
         """
-        if not self.git or self.type == StoreType.CORE:
-            return True
 
-        # If exists?
-        for filetype in FILE_SUFFIX_CONFIGURATION:
-            repository_file = Path(self.git.path / f"repository{filetype}")
-            if repository_file.exists():
-                break
+    @abstractmethod
+    async def remove(self) -> None:
+        """Remove add-on repository."""
 
-        if not repository_file.exists():
-            return False
+    @abstractmethod
+    async def reset(self) -> None:
+        """Reset add-on repository to fix corruption issue with files."""
 
-        # If valid?
-        try:
-            SCHEMA_REPOSITORY_CONFIG(read_json_or_yaml_file(repository_file))
-        except (ConfigurationFileError, vol.Invalid) as err:
-            _LOGGER.warning("Could not validate repository configuration %s", err)
-            return False
 
+class RepositoryBuiltin(Repository, ABC):
+    """A built-in add-on repository."""
+
+    def __init__(self, coresys: CoreSys, builtin: BuiltinRepository) -> None:
+        """Initialize object."""
+        super().__init__(coresys, builtin.value)
+        self._builtin = builtin
+        self._slug = builtin.id
+        self._type = builtin.type
+
+    async def validate(self) -> bool:
+        """Assume built-in repositories are always valid."""
         return True
 
+    async def remove(self) -> None:
+        """Raise. Not supported for built-in repositories."""
+        raise StoreError("Can't remove built-in repositories!", _LOGGER.error)
+
+
+class RepositoryGit(Repository, ABC):
+    """A git based add-on repository."""
+
+    _git: GitRepo
+
     async def load(self) -> None:
         """Load addon repository."""
-        if not self.git:
-            self._latest_mtime, _ = await self.sys_run_in_executor(
-                get_latest_mtime, self.sys_config.path_addons_local
-            )
-            return
-        await self.git.load()
+        await self._git.load()
 
     async def update(self) -> bool:
         """Update add-on repository.
 
         Returns True if the repository was updated.
         """
-        if not await self.sys_run_in_executor(self.validate):
+        if not await self.validate():
             return False
 
-        if self.git:
-            return await self.git.pull()
+        return await self._git.pull()
+
+    async def validate(self) -> bool:
+        """Check if store is valid."""
+
+        def validate_file() -> bool:
+            # If exists?
+            for filetype in FILE_SUFFIX_CONFIGURATION:
+                repository_file = Path(self._git.path / f"repository{filetype}")
+                if repository_file.exists():
+                    break
+
+            if not repository_file.exists():
+                return False
+
+            # If valid?
+            try:
+                SCHEMA_REPOSITORY_CONFIG(read_json_or_yaml_file(repository_file))
+            except (ConfigurationFileError, vol.Invalid) as err:
+                _LOGGER.warning("Could not validate repository configuration %s", err)
+                return False
 
+            return True
+
+        return await self.sys_run_in_executor(validate_file)
+
+    async def reset(self) -> None:
+        """Reset add-on repository to fix corruption issue with files."""
+        await self._git.reset()
+        await self.load()
+
+
+class RepositoryLocal(RepositoryBuiltin):
+    """A local add-on repository."""
+
+    def __init__(self, coresys: CoreSys) -> None:
+        """Initialize object."""
+        super().__init__(coresys, BuiltinRepository.LOCAL)
+        self._latest_mtime: float | None = None
+
+    async def load(self) -> None:
+        """Load addon repository."""
+        self._latest_mtime, _ = await self.sys_run_in_executor(
+            get_latest_mtime, self.sys_config.path_addons_local
+        )
+
+    async def update(self) -> bool:
+        """Update add-on repository.
+
+        Returns True if the repository was updated.
+        """
         # Check local modifications
         latest_mtime, modified_path = await self.sys_run_in_executor(
             get_latest_mtime, self.sys_config.path_addons_local
@@ -138,9 +202,32 @@ async def update(self) -> bool:
 
         return False
 
+    async def reset(self) -> None:
+        """Raise. Not supported for local repository."""
+        raise StoreError(
+            "Can't reset local repository as it is not git based!", _LOGGER.error
+        )
+
+
+class RepositoryGitBuiltin(RepositoryBuiltin, RepositoryGit):
+    """A built-in add-on repository based on git."""
+
+    def __init__(self, coresys: CoreSys, builtin: BuiltinRepository) -> None:
+        """Initialize object."""
+        super().__init__(coresys, builtin)
+        self._git = GitRepo(coresys, builtin.get_path(coresys), builtin.url)
+
+
+class RepositoryCustom(RepositoryGit):
+    """A custom add-on repository."""
+
+    def __init__(self, coresys: CoreSys, url: str) -> None:
+        """Initialize object."""
+        super().__init__(coresys, url)
+        self._slug = get_hash_from_repository(url)
+        self._type = StoreType.GIT
+        self._git = GitRepo(coresys, coresys.config.path_addons_git / self._slug, url)
+
     async def remove(self) -> None:
         """Remove add-on repository."""
-        if not self.git or self.git.builtin:
-            raise StoreError("Can't remove built-in repositories!", _LOGGER.error)
-
-        await self.git.remove()
+        await self._git.remove()

tests/addons/test_addon.py

@@ -820,7 +820,7 @@ async def test_paths_cache(coresys: CoreSys, install_addon_ssh: Addon):
 
     with (
         patch("supervisor.addons.addon.Path.exists", return_value=True),
-        patch("supervisor.store.repository.Repository.update", return_value=True),
+        patch("supervisor.store.repository.RepositoryLocal.update", return_value=True),
     ):
         await coresys.store.reload(coresys.store.get("local"))
 

tests/addons/test_manager.py

@@ -29,7 +29,7 @@
 from supervisor.resolution.const import ContextType, IssueType, SuggestionType
 from supervisor.resolution.data import Issue, Suggestion
 from supervisor.store.addon import AddonStore
-from supervisor.store.repository import Repository
+from supervisor.store.repository import RepositoryLocal
 from supervisor.utils import check_exception_chain
 from supervisor.utils.common import write_json_file
 
@@ -442,7 +442,7 @@ async def mock_update(_, version, image, *args, **kwargs):
     update_task = coresys.create_task(simulate_update())
     await asyncio.sleep(0)
 
-    with patch.object(Repository, "update", return_value=True):
+    with patch.object(RepositoryLocal, "update", return_value=True):
         await coresys.store.reload()
 
     assert "image" not in coresys.store.data.addons["local_ssh"]

tests/api/test_store.py

@@ -97,8 +97,8 @@ async def test_api_store_add_repository(
 ) -> None:
     """Test POST /store/repositories REST API."""
     with (
-        patch("supervisor.store.repository.Repository.load", return_value=None),
-        patch("supervisor.store.repository.Repository.validate", return_value=True),
+        patch("supervisor.store.repository.RepositoryGit.load", return_value=None),
+        patch("supervisor.store.repository.RepositoryGit.validate", return_value=True),
     ):
         response = await api_client.post(
             "/store/repositories", json={"repository": REPO_URL}