From 99c040520e0b00bf9695a04606b613a1cae3b64b Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Mon, 14 Jul 2025 22:19:06 +0200 Subject: [PATCH] Drop ensure_builtin_repositories() (#6012) * Drop ensure_builtin_repositories With the new Repository classes we have the is_builtin property, so we can easily make sure that built-ins are not removed. This allows us to further cleanup the code by removing the ensure_builtin_repositories function and the ALL_BUILTIN_REPOSITORIES constant. * Make sure we add built-ins on load * Reuse default set and avoid unnecessary copy Reuse default set and avoid unnecessary copying during validation if the default is not being used. --- supervisor/backups/backup.py | 2 +- supervisor/store/__init__.py | 27 +++++++++----------- supervisor/store/const.py | 4 --- supervisor/store/validate.py | 15 +++-------- tests/store/test_custom_repository.py | 36 ++++++++++++++++----------- 5 files changed, 39 insertions(+), 45 deletions(-) diff --git a/supervisor/backups/backup.py b/supervisor/backups/backup.py index 9f476ce55..8ebb6c838 100644 --- a/supervisor/backups/backup.py +++ b/supervisor/backups/backup.py @@ -931,5 +931,5 @@ class Backup(JobGroup): Return a coroutine. """ return self.sys_store.update_repositories( - self.repositories, issue_on_error=True, replace=replace + set(self.repositories), issue_on_error=True, replace=replace ) diff --git a/supervisor/store/__init__.py b/supervisor/store/__init__.py index 54f605863..1f43bb561 100644 --- a/supervisor/store/__init__.py +++ b/supervisor/store/__init__.py @@ -21,7 +21,7 @@ from .addon import AddonStore from .const import FILE_HASSIO_STORE, BuiltinRepository from .data import StoreData from .repository import Repository -from .validate import SCHEMA_STORE_FILE, ensure_builtin_repositories +from .validate import DEFAULT_REPOSITORIES, SCHEMA_STORE_FILE _LOGGER: logging.Logger = logging.getLogger(__name__) @@ -63,11 +63,14 @@ class StoreManager(CoreSysAttributes, FileConfiguration): return self.repositories[slug] 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], issue_on_error=True + """Start up add-on store management.""" + # Make sure the built-in repositories are all present + # This is especially important when adding new built-in repositories + # to make sure existing installations have them. + all_repositories: set[str] = ( + set(self._data.get(ATTR_REPOSITORIES, [])) | DEFAULT_REPOSITORIES ) + await self.update_repositories(all_repositories, issue_on_error=True) @Job( name="store_manager_reload", @@ -223,7 +226,7 @@ class StoreManager(CoreSysAttributes, FileConfiguration): @Job(name="store_manager_update_repositories") async def update_repositories( self, - list_repositories: list[str], + list_repositories: set[str], *, issue_on_error: bool = False, replace: bool = True, @@ -231,14 +234,8 @@ class StoreManager(CoreSysAttributes, FileConfiguration): """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 + # Determine repositories to add + repositories_to_add = list_repositories - current_repositories # Add new repositories add_errors = await asyncio.gather( @@ -259,7 +256,7 @@ class StoreManager(CoreSysAttributes, FileConfiguration): repositories_to_remove: list[Repository] = [ repository for repository in self.all - if repository.source not in target_repositories + if repository.source not in list_repositories and not repository.is_builtin ] diff --git a/supervisor/store/const.py b/supervisor/store/const.py index 39638058e..f4417f5a0 100644 --- a/supervisor/store/const.py +++ b/supervisor/store/const.py @@ -35,7 +35,3 @@ class BuiltinRepository(StrEnum): 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} diff --git a/supervisor/store/validate.py b/supervisor/store/validate.py index 31701a625..e649caa7c 100644 --- a/supervisor/store/validate.py +++ b/supervisor/store/validate.py @@ -4,7 +4,7 @@ import voluptuous as vol from ..const import ATTR_MAINTAINER, ATTR_NAME, ATTR_REPOSITORIES, ATTR_URL from ..validate import RE_REPOSITORY -from .const import ALL_BUILTIN_REPOSITORIES, BuiltinRepository +from .const import BuiltinRepository # pylint: disable=no-value-for-parameter SCHEMA_REPOSITORY_CONFIG = vol.Schema( @@ -17,15 +17,6 @@ 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) | ALL_BUILTIN_REPOSITORIES) - - def validate_repository(repository: str) -> str: """Validate a valid repository.""" if repository in BuiltinRepository: @@ -44,10 +35,12 @@ def validate_repository(repository: str) -> str: repositories = vol.All([validate_repository], vol.Unique()) +DEFAULT_REPOSITORIES = {repo.value for repo in BuiltinRepository} + SCHEMA_STORE_FILE = vol.Schema( { vol.Optional( - ATTR_REPOSITORIES, default=list(ALL_BUILTIN_REPOSITORIES) + ATTR_REPOSITORIES, default=lambda: list(DEFAULT_REPOSITORIES) ): repositories, }, extra=vol.REMOVE_EXTRA, diff --git a/tests/store/test_custom_repository.py b/tests/store/test_custom_repository.py index 16dc33877..51c9c2bfc 100644 --- a/tests/store/test_custom_repository.py +++ b/tests/store/test_custom_repository.py @@ -17,7 +17,7 @@ from supervisor.exceptions import ( from supervisor.resolution.const import SuggestionType from supervisor.store import StoreManager from supervisor.store.addon import AddonStore -from supervisor.store.const import ALL_BUILTIN_REPOSITORIES +from supervisor.store.const import BuiltinRepository from supervisor.store.repository import Repository @@ -50,7 +50,9 @@ async def test_add_valid_repository( patch("pathlib.Path.exists", return_value=True), ): if use_update: - await store_manager.update_repositories(current + ["http://example.com"]) + await store_manager.update_repositories( + set(current) | {"http://example.com"} + ) else: await store_manager.add_repository("http://example.com") @@ -70,7 +72,7 @@ async def test_add_invalid_repository(coresys: CoreSys, store_manager: StoreMana ), ): await store_manager.update_repositories( - current + ["http://example.com"], issue_on_error=True + set(current) | {"http://example.com"}, issue_on_error=True ) assert not await get_repository_by_url( @@ -96,7 +98,9 @@ async def test_error_on_invalid_repository( pytest.raises(StoreError), ): if use_update: - await store_manager.update_repositories(current + ["http://example.com"]) + await store_manager.update_repositories( + set(current) | {"http://example.com"} + ) else: await store_manager.add_repository("http://example.com") @@ -118,7 +122,7 @@ async def test_add_invalid_repository_file( patch("pathlib.Path.exists", return_value=False), ): await store_manager.update_repositories( - current + ["http://example.com"], issue_on_error=True + set(current) | {"http://example.com"}, issue_on_error=True ) assert not await get_repository_by_url( @@ -146,7 +150,7 @@ async def test_add_repository_with_git_error( current = coresys.store.repository_urls with patch("supervisor.store.git.GitRepo.load", side_effect=git_error): await store_manager.update_repositories( - current + ["http://example.com"], issue_on_error=True + set(current) | {"http://example.com"}, issue_on_error=True ) assert "http://example.com" in coresys.store.repository_urls @@ -175,7 +179,9 @@ async def test_error_on_repository_with_git_error( pytest.raises(StoreError), ): if use_update: - await store_manager.update_repositories(current + ["http://example.com"]) + await store_manager.update_repositories( + set(current) | {"http://example.com"} + ) else: await store_manager.add_repository("http://example.com") @@ -189,7 +195,9 @@ async def test_preinstall_valid_repository( ): """Test add core repository valid.""" with patch("supervisor.store.git.GitRepo.load", return_value=None): - await store_manager.update_repositories(list(ALL_BUILTIN_REPOSITORIES)) + await store_manager.update_repositories( + {repo.value for repo in BuiltinRepository} + ) def validate(): assert store_manager.get("core").validate() @@ -213,7 +221,7 @@ async def test_remove_repository( assert test_repository.slug in coresys.store.repositories if use_update: - await store_manager.update_repositories([]) + await store_manager.update_repositories(set()) else: await store_manager.remove_repository(test_repository) @@ -241,7 +249,7 @@ async def test_remove_used_repository( match="Can't remove 'https://github.com/awesome-developer/awesome-repo'. It's used by installed add-ons", ): if use_update: - await store_manager.update_repositories([]) + await store_manager.update_repositories(set()) else: await store_manager.remove_repository( coresys.store.repositories[store_addon.repository] @@ -252,7 +260,7 @@ async def test_update_partial_error(coresys: CoreSys, store_manager: StoreManage """Test partial error on update does partial save and errors.""" with patch("supervisor.store.repository.RepositoryGit.validate", return_value=True): with patch("supervisor.store.git.GitRepo.load", return_value=None): - await store_manager.update_repositories([]) + await store_manager.update_repositories(set()) store_manager.data.update.assert_called_once() store_manager.data.update.reset_mock() @@ -268,7 +276,7 @@ async def test_update_partial_error(coresys: CoreSys, store_manager: StoreManage pytest.raises(StoreError), ): await store_manager.update_repositories( - current + ["http://example.com", "http://example2.com"] + set(current) | {"http://example.com", "http://example2.com"} ) assert len(coresys.store.repository_urls) == initial + 1 @@ -303,7 +311,7 @@ async def test_add_with_update_repositories( ), patch("pathlib.Path.exists", return_value=True), ): - await store_manager.update_repositories(["http://example.com"], replace=False) + await store_manager.update_repositories({"http://example.com"}, replace=False) assert test_repository.source in coresys.store.repository_urls assert "http://example.com" in coresys.store.repository_urls @@ -322,7 +330,7 @@ async def test_add_repository_fails_if_out_of_date( ): if use_update: await store_manager.update_repositories( - coresys.store.repository_urls + ["http://example.com"], + set(coresys.store.repository_urls) | {"http://example.com"} ) else: await store_manager.add_repository("http://example.com")