Cache existence of addon paths (#4944)

* Cache existence of addon paths

* Always update submodules

* Switch to an always cached model

* Cache on store addon only

* Fix tests

* refresh_cache to refresh_path_cache

* Fix name change in test

* Move logic into StoreManager

supervisor/addons/addon.py

@@ -180,6 +180,9 @@ class Addon(AddonModel):
 
     async def load(self) -> None:
         """Async initialize of object."""
+        if self.is_detached:
+            await super().refresh_path_cache()
+
         self._listeners.append(
             self.sys_bus.register_event(
                 BusEvent.DOCKER_CONTAINER_STATE_CHANGE, self.container_state_changed
@@ -230,6 +233,34 @@ class Addon(AddonModel):
         """Return True if add-on is detached."""
         return self.slug not in self.sys_store.data.addons
 
+    @property
+    def with_icon(self) -> bool:
+        """Return True if an icon exists."""
+        if self.is_detached:
+            return super().with_icon
+        return self.addon_store.with_icon
+
+    @property
+    def with_logo(self) -> bool:
+        """Return True if a logo exists."""
+        if self.is_detached:
+            return super().with_logo
+        return self.addon_store.with_logo
+
+    @property
+    def with_changelog(self) -> bool:
+        """Return True if a changelog exists."""
+        if self.is_detached:
+            return super().with_changelog
+        return self.addon_store.with_changelog
+
+    @property
+    def with_documentation(self) -> bool:
+        """Return True if a documentation exists."""
+        if self.is_detached:
+            return super().with_documentation
+        return self.addon_store.with_documentation
+
     @property
     def available(self) -> bool:
         """Return True if this add-on is available on this platform."""
@@ -1399,3 +1430,9 @@ class Addon(AddonModel):
             ContainerState.UNHEALTHY,
         ]:
             await self._restart_after_problem(event.state)
+
+    def refresh_path_cache(self) -> Awaitable[None]:
+        """Refresh cache of existing paths."""
+        if self.is_detached:
+            return super().refresh_path_cache()
+        return self.addon_store.refresh_path_cache()

supervisor/addons/manager.py

@@ -77,15 +77,20 @@ class AddonManager(CoreSysAttributes):
 
     async def load(self) -> None:
         """Start up add-on management."""
-        tasks = []
+        # Refresh cache for all store addons
+        tasks: list[Awaitable[None]] = [
+            store.refresh_path_cache() for store in self.store.values()
+        ]
+
+        # Load all installed addons
         for slug in self.data.system:
             addon = self.local[slug] = Addon(self.coresys, slug)
-            tasks.append(self.sys_create_task(addon.load()))
+            tasks.append(addon.load())
 
         # Run initial tasks
-        _LOGGER.info("Found %d installed add-ons", len(tasks))
+        _LOGGER.info("Found %d installed add-ons", len(self.data.system))
         if tasks:
-            await asyncio.wait(tasks)
+            await asyncio.gather(*tasks)
 
         # Sync DNS
         await self.sync_dns()

supervisor/addons/model.py

@@ -1,7 +1,7 @@
 """Init file for Supervisor add-ons."""
 from abc import ABC, abstractmethod
 from collections import defaultdict
-from collections.abc import Callable
+from collections.abc import Awaitable, Callable
 from contextlib import suppress
 from datetime import datetime
 import logging
@@ -118,6 +118,10 @@ class AddonModel(JobGroup, ABC):
             coresys, JOB_GROUP_ADDON.format_map(defaultdict(str, slug=slug)), slug
         )
         self.slug: str = slug
+        self._path_icon_exists: bool = False
+        self._path_logo_exists: bool = False
+        self._path_changelog_exists: bool = False
+        self._path_documentation_exists: bool = False
 
     @property
     @abstractmethod
@@ -511,22 +515,22 @@ class AddonModel(JobGroup, ABC):
     @property
     def with_icon(self) -> bool:
         """Return True if an icon exists."""
-        return self.path_icon.exists()
+        return self._path_icon_exists
 
     @property
     def with_logo(self) -> bool:
         """Return True if a logo exists."""
-        return self.path_logo.exists()
+        return self._path_logo_exists
 
     @property
     def with_changelog(self) -> bool:
         """Return True if a changelog exists."""
-        return self.path_changelog.exists()
+        return self._path_changelog_exists
 
     @property
     def with_documentation(self) -> bool:
         """Return True if a documentation exists."""
-        return self.path_documentation.exists()
+        return self._path_documentation_exists
 
     @property
     def supported_arch(self) -> list[str]:
@@ -635,6 +639,17 @@ class AddonModel(JobGroup, ABC):
         """Return breaking versions of addon."""
         return self.data[ATTR_BREAKING_VERSIONS]
 
+    def refresh_path_cache(self) -> Awaitable[None]:
+        """Refresh cache of existing paths."""
+
+        def check_paths():
+            self._path_icon_exists = self.path_icon.exists()
+            self._path_logo_exists = self.path_logo.exists()
+            self._path_changelog_exists = self.path_changelog.exists()
+            self._path_documentation_exists = self.path_documentation.exists()
+
+        return self.sys_run_in_executor(check_paths)
+
     def validate_availability(self) -> None:
         """Validate if addon is available for current system."""
         return self._validate_availability(self.data, logger=_LOGGER.error)

supervisor/resolution/fixups/store_execute_reload.py

@@ -41,7 +41,7 @@ class FixupStoreExecuteReload(FixupBase):
         # Load data again
         try:
             await repository.load()
-            await repository.update()
+            await self.sys_store.reload(repository)
         except StoreError:
             raise ResolutionFixupError() from None
 

supervisor/store/__init__.py

@@ -1,5 +1,6 @@
 """Add-on Store handler."""
 import asyncio
+from collections.abc import Awaitable
 import logging
 
 from ..const import ATTR_REPOSITORIES, URL_HASSIO_ADDONS
@@ -85,15 +86,39 @@ class StoreManager(CoreSysAttributes, FileConfiguration):
         conditions=[JobCondition.SUPERVISOR_UPDATED],
         on_condition=StoreJobError,
     )
-    async def reload(self) -> None:
+    async def reload(self, repository: Repository | None = None) -> None:
         """Update add-ons from repository and reload list."""
-        tasks = [self.sys_create_task(repository.update()) for repository in self.all]
-        if tasks:
-            await asyncio.wait(tasks)
+        # Make a copy to prevent race with other tasks
+        repositories = [repository] if repository else self.all.copy()
+        results: list[bool | Exception] = await asyncio.gather(
+            *[repo.update() for repo in repositories], return_exceptions=True
+        )
 
-        # read data from repositories
-        await self.load()
-        self._read_addons()
+        # Determine which repositories were updated
+        updated_repos: set[str] = set()
+        for i, result in enumerate(results):
+            if result is True:
+                updated_repos.add(repositories[i].slug)
+            elif result:
+                _LOGGER.error(
+                    "Could not reload repository %s due to %r",
+                    repositories[i].slug,
+                    result,
+                )
+
+        # Update path cache for all addons in updated repos
+        if updated_repos:
+            await asyncio.gather(
+                *[
+                    addon.refresh_path_cache()
+                    for addon in self.sys_addons.store.values()
+                    if addon.repository in updated_repos
+                ]
+            )
+
+            # read data from repositories
+            await self.load()
+            await self._read_addons()
 
     @Job(
         name="store_manager_add_repository",
@@ -185,7 +210,7 @@ class StoreManager(CoreSysAttributes, FileConfiguration):
         # Persist changes
         if persist:
             await self.data.update()
-            self._read_addons()
+            await self._read_addons()
 
     async def remove_repository(self, repository: Repository, *, persist: bool = True):
         """Remove a repository."""
@@ -205,7 +230,7 @@ class StoreManager(CoreSysAttributes, FileConfiguration):
 
         if persist:
             await self.data.update()
-            self._read_addons()
+            await self._read_addons()
 
     @Job(name="store_manager_update_repositories")
     async def update_repositories(
@@ -245,14 +270,14 @@ class StoreManager(CoreSysAttributes, FileConfiguration):
 
         # Always update data, even there are errors, some changes may have succeeded
         await self.data.update()
-        self._read_addons()
+        await self._read_addons()
 
         # Raise the first error we found (if any)
         for error in add_errors + remove_errors:
             if error:
                 raise error
 
-    def _read_addons(self) -> None:
+    async def _read_addons(self) -> None:
         """Reload add-ons inside store."""
         all_addons = set(self.data.addons)
 
@@ -268,8 +293,13 @@ class StoreManager(CoreSysAttributes, FileConfiguration):
         )
 
         # new addons
-        for slug in add_addons:
-            self.sys_addons.store[slug] = AddonStore(self.coresys, slug)
+        if add_addons:
+            cache_updates: list[Awaitable[None]] = []
+            for slug in add_addons:
+                self.sys_addons.store[slug] = AddonStore(self.coresys, slug)
+                cache_updates.append(self.sys_addons.store[slug].refresh_path_cache())
+
+            await asyncio.gather(*cache_updates)
 
         # remove
         for slug in del_addons:

supervisor/store/git.py

@@ -117,7 +117,7 @@ class GitRepo(CoreSysAttributes):
         conditions=[JobCondition.FREE_SPACE, JobCondition.INTERNET_SYSTEM],
         on_condition=StoreJobError,
     )
-    async def pull(self):
+    async def pull(self) -> bool:
         """Pull Git add-on repo."""
         if self.lock.locked():
             _LOGGER.warning("There is already a task in progress")
@@ -140,10 +140,13 @@ class GitRepo(CoreSysAttributes):
                     )
                 )
 
-                # Jump on top of that
-                await self.sys_run_in_executor(
-                    ft.partial(self.repo.git.reset, f"origin/{branch}", hard=True)
-                )
+                if changed := self.repo.commit(branch) != self.repo.commit(
+                    f"origin/{branch}"
+                ):
+                    # Jump on top of that
+                    await self.sys_run_in_executor(
+                        ft.partial(self.repo.git.reset, f"origin/{branch}", hard=True)
+                    )
 
                 # Update submodules
                 await self.sys_run_in_executor(
@@ -160,6 +163,8 @@ class GitRepo(CoreSysAttributes):
                 # Cleanup old data
                 await self.sys_run_in_executor(ft.partial(self.repo.git.clean, "-xdf"))
 
+                return changed
+
             except (
                 git.InvalidGitRepositoryError,
                 git.NoSuchPathError,

supervisor/store/repository.py

@@ -1,4 +1,5 @@
 """Represent a Supervisor repository."""
+
 import logging
 from pathlib import Path
 
@@ -101,11 +102,11 @@ class Repository(CoreSysAttributes):
             return
         await self.git.load()
 
-    async def update(self) -> None:
+    async def update(self) -> bool:
         """Update add-on repository."""
-        if self.type == StoreType.LOCAL or not self.validate():
-            return
-        await self.git.pull()
+        if not self.validate():
+            return False
+        return self.type == StoreType.LOCAL or await self.git.pull()
 
     async def remove(self) -> None:
         """Remove add-on repository."""

tests/addons/test_addon.py

@@ -748,3 +748,18 @@ def test_auto_update_available(coresys: CoreSys, install_addon_example: Addon):
         Addon, "version", new=PropertyMock(return_value=AwesomeVersion("test"))
     ):
         assert install_addon_example.auto_update_available is False
+
+
+async def test_paths_cache(coresys: CoreSys, install_addon_ssh: Addon):
+    """Test cache for key paths that may or may not exist."""
+    with patch("supervisor.addons.addon.Path.exists", return_value=True):
+        assert not install_addon_ssh.with_logo
+        assert not install_addon_ssh.with_icon
+        assert not install_addon_ssh.with_changelog
+        assert not install_addon_ssh.with_documentation
+
+        await coresys.store.reload(coresys.store.get("local"))
+        assert install_addon_ssh.with_logo
+        assert install_addon_ssh.with_icon
+        assert install_addon_ssh.with_changelog
+        assert install_addon_ssh.with_documentation

tests/addons/test_manager.py

@@ -393,7 +393,7 @@ async def test_store_data_changes_during_update(
     update_task = coresys.create_task(simulate_update())
     await asyncio.sleep(0)
 
-    with patch.object(Repository, "update"):
+    with patch.object(Repository, "update", return_value=True):
         await coresys.store.reload()
 
     assert "image" not in coresys.store.data.addons["local_ssh"]

tests/store/test_store_manager.py

@@ -24,12 +24,19 @@ from tests.common import load_yaml_fixture
 async def test_default_load(coresys: CoreSys):
     """Test default load from config."""
     store_manager = StoreManager(coresys)
+    refresh_cache_calls: set[str] = set()
+
+    async def mock_refresh_cache(obj: AddonStore):
+        nonlocal refresh_cache_calls
+        refresh_cache_calls.add(obj.slug)
 
     with patch(
         "supervisor.store.repository.Repository.load", return_value=None
     ), patch.object(
         type(coresys.config), "addons_repositories", return_value=[]
-    ), patch("pathlib.Path.exists", return_value=True):
+    ), patch("pathlib.Path.exists", return_value=True), patch.object(
+        AddonStore, "refresh_path_cache", new=mock_refresh_cache
+    ):
         await store_manager.load()
 
     assert len(store_manager.all) == 4
@@ -44,10 +51,15 @@ async def test_default_load(coresys: CoreSys):
         "https://github.com/esphome/home-assistant-addon"
         in store_manager.repository_urls
     )
+    assert refresh_cache_calls == {"local_ssh", "local_example", "core_samba"}
 
 
 async def test_load_with_custom_repository(coresys: CoreSys):
     """Test load from config with custom repository."""
+
+    async def mock_refresh_cache(_):
+        pass
+
     with patch(
         "supervisor.utils.common.read_json_or_yaml_file",
         return_value={"repositories": ["http://example.com"]},
@@ -60,7 +72,9 @@ async def test_load_with_custom_repository(coresys: CoreSys):
         type(coresys.config), "addons_repositories", return_value=[]
     ), patch(
         "supervisor.store.repository.Repository.validate", return_value=True
-    ), patch("pathlib.Path.exists", return_value=True):
+    ), patch("pathlib.Path.exists", return_value=True), patch.object(
+        AddonStore, "refresh_path_cache", new=mock_refresh_cache
+    ):
         await store_manager.load()
 
     assert len(store_manager.all) == 5