Skip to content

Commit 99c0405

Browse files
agnersagners
authored
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.
1 parent eefe2f2 commit 99c0405

File tree

5 files changed

+39
-45
lines changed

5 files changed

+39
-45
lines changed

‎supervisor/backups/backup.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -931,5 +931,5 @@ def restore_repositories(self, replace: bool = False) -> Awaitable[None]:
931931
Return a coroutine.
932932
"""
933933
return self.sys_store.update_repositories(
934-
self.repositories, issue_on_error=True, replace=replace
934+
set(self.repositories), issue_on_error=True, replace=replace
935935
)

‎supervisor/store/__init__.py‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
from .const import FILE_HASSIO_STORE, BuiltinRepository
2222
from .data import StoreData
2323
from .repository import Repository
24-
from .validate import SCHEMA_STORE_FILE, ensure_builtin_repositories
24+
from .validate import DEFAULT_REPOSITORIES, SCHEMA_STORE_FILE
2525

2626
_LOGGER: logging.Logger = logging.getLogger(__name__)
2727

@@ -63,11 +63,14 @@ def get(self, slug: str) -> Repository:
6363
return self.repositories[slug]
6464

6565
async def load(self) -> None:
66-
"""Start up add-on management."""
67-
# Init custom repositories and load add-ons
68-
await self.update_repositories(
69-
self._data[ATTR_REPOSITORIES], issue_on_error=True
66+
"""Start up add-on store management."""
67+
# Make sure the built-in repositories are all present
68+
# This is especially important when adding new built-in repositories
69+
# to make sure existing installations have them.
70+
all_repositories: set[str] = (
71+
set(self._data.get(ATTR_REPOSITORIES, [])) | DEFAULT_REPOSITORIES
7072
)
73+
await self.update_repositories(all_repositories, issue_on_error=True)
7174

7275
@Job(
7376
name="store_manager_reload",
@@ -223,22 +226,16 @@ async def remove_repository(self, repository: Repository, *, persist: bool = Tru
223226
@Job(name="store_manager_update_repositories")
224227
async def update_repositories(
225228
self,
226-
list_repositories: list[str],
229+
list_repositories: set[str],
227230
*,
228231
issue_on_error: bool = False,
229232
replace: bool = True,
230233
):
231234
"""Update repositories by adding new ones and removing stale ones."""
232235
current_repositories = {repository.source for repository in self.all}
233236

234-
# Determine changes needed
235-
if replace:
236-
target_repositories = set(ensure_builtin_repositories(list_repositories))
237-
repositories_to_add = target_repositories - current_repositories
238-
else:
239-
# When not replacing, just add the new repositories
240-
repositories_to_add = set(list_repositories) - current_repositories
241-
target_repositories = current_repositories | repositories_to_add
237+
# Determine repositories to add
238+
repositories_to_add = list_repositories - current_repositories
242239

243240
# Add new repositories
244241
add_errors = await asyncio.gather(
@@ -259,7 +256,7 @@ async def update_repositories(
259256
repositories_to_remove: list[Repository] = [
260257
repository
261258
for repository in self.all
262-
if repository.source not in target_repositories
259+
if repository.source not in list_repositories
263260
and not repository.is_builtin
264261
]
265262

‎supervisor/store/const.py‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,3 @@ def git_url(self) -> str:
3535
return URL_HASSIO_ADDONS
3636
else:
3737
return self.value # For URL-based repos, value is the URL
38-
39-
40-
# All repositories that are considered "built-in" and protected from removal
41-
ALL_BUILTIN_REPOSITORIES = {repo.value for repo in BuiltinRepository}

‎supervisor/store/validate.py‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
from ..const import ATTR_MAINTAINER, ATTR_NAME, ATTR_REPOSITORIES, ATTR_URL
66
from ..validate import RE_REPOSITORY
7-
from .const import ALL_BUILTIN_REPOSITORIES, BuiltinRepository
7+
from .const import BuiltinRepository
88

99
# pylint: disable=no-value-for-parameter
1010
SCHEMA_REPOSITORY_CONFIG = vol.Schema(
@@ -17,15 +17,6 @@
1717
)
1818

1919

20-
def ensure_builtin_repositories(addon_repositories: list[str]) -> list[str]:
21-
"""Ensure builtin repositories are in list.
22-
23-
Note: This should not be used in validation as the resulting list is not
24-
stable. This can have side effects when comparing data later on.
25-
"""
26-
return list(set(addon_repositories) | ALL_BUILTIN_REPOSITORIES)
27-
28-
2920
def validate_repository(repository: str) -> str:
3021
"""Validate a valid repository."""
3122
if repository in BuiltinRepository:
@@ -44,10 +35,12 @@ def validate_repository(repository: str) -> str:
4435

4536
repositories = vol.All([validate_repository], vol.Unique())
4637

38+
DEFAULT_REPOSITORIES = {repo.value for repo in BuiltinRepository}
39+
4740
SCHEMA_STORE_FILE = vol.Schema(
4841
{
4942
vol.Optional(
50-
ATTR_REPOSITORIES, default=list(ALL_BUILTIN_REPOSITORIES)
43+
ATTR_REPOSITORIES, default=lambda: list(DEFAULT_REPOSITORIES)
5144
): repositories,
5245
},
5346
extra=vol.REMOVE_EXTRA,

‎tests/store/test_custom_repository.py‎

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
from supervisor.resolution.const import SuggestionType
1818
from supervisor.store import StoreManager
1919
from supervisor.store.addon import AddonStore
20-
from supervisor.store.const import ALL_BUILTIN_REPOSITORIES
20+
from supervisor.store.const import BuiltinRepository
2121
from supervisor.store.repository import Repository
2222

2323

@@ -50,7 +50,9 @@ async def test_add_valid_repository(
5050
patch("pathlib.Path.exists", return_value=True),
5151
):
5252
if use_update:
53-
await store_manager.update_repositories(current + ["http://example.com"])
53+
await store_manager.update_repositories(
54+
set(current) | {"http://example.com"}
55+
)
5456
else:
5557
await store_manager.add_repository("http://example.com")
5658

@@ -70,7 +72,7 @@ async def test_add_invalid_repository(coresys: CoreSys, store_manager: StoreMana
7072
),
7173
):
7274
await store_manager.update_repositories(
73-
current + ["http://example.com"], issue_on_error=True
75+
set(current) | {"http://example.com"}, issue_on_error=True
7476
)
7577

7678
assert not await get_repository_by_url(
@@ -96,7 +98,9 @@ async def test_error_on_invalid_repository(
9698
pytest.raises(StoreError),
9799
):
98100
if use_update:
99-
await store_manager.update_repositories(current + ["http://example.com"])
101+
await store_manager.update_repositories(
102+
set(current) | {"http://example.com"}
103+
)
100104
else:
101105
await store_manager.add_repository("http://example.com")
102106

@@ -118,7 +122,7 @@ async def test_add_invalid_repository_file(
118122
patch("pathlib.Path.exists", return_value=False),
119123
):
120124
await store_manager.update_repositories(
121-
current + ["http://example.com"], issue_on_error=True
125+
set(current) | {"http://example.com"}, issue_on_error=True
122126
)
123127

124128
assert not await get_repository_by_url(
@@ -146,7 +150,7 @@ async def test_add_repository_with_git_error(
146150
current = coresys.store.repository_urls
147151
with patch("supervisor.store.git.GitRepo.load", side_effect=git_error):
148152
await store_manager.update_repositories(
149-
current + ["http://example.com"], issue_on_error=True
153+
set(current) | {"http://example.com"}, issue_on_error=True
150154
)
151155

152156
assert "http://example.com" in coresys.store.repository_urls
@@ -175,7 +179,9 @@ async def test_error_on_repository_with_git_error(
175179
pytest.raises(StoreError),
176180
):
177181
if use_update:
178-
await store_manager.update_repositories(current + ["http://example.com"])
182+
await store_manager.update_repositories(
183+
set(current) | {"http://example.com"}
184+
)
179185
else:
180186
await store_manager.add_repository("http://example.com")
181187

@@ -189,7 +195,9 @@ async def test_preinstall_valid_repository(
189195
):
190196
"""Test add core repository valid."""
191197
with patch("supervisor.store.git.GitRepo.load", return_value=None):
192-
await store_manager.update_repositories(list(ALL_BUILTIN_REPOSITORIES))
198+
await store_manager.update_repositories(
199+
{repo.value for repo in BuiltinRepository}
200+
)
193201

194202
def validate():
195203
assert store_manager.get("core").validate()
@@ -213,7 +221,7 @@ async def test_remove_repository(
213221
assert test_repository.slug in coresys.store.repositories
214222

215223
if use_update:
216-
await store_manager.update_repositories([])
224+
await store_manager.update_repositories(set())
217225
else:
218226
await store_manager.remove_repository(test_repository)
219227

@@ -241,7 +249,7 @@ async def test_remove_used_repository(
241249
match="Can't remove 'https://github.com/awesome-developer/awesome-repo'. It's used by installed add-ons",
242250
):
243251
if use_update:
244-
await store_manager.update_repositories([])
252+
await store_manager.update_repositories(set())
245253
else:
246254
await store_manager.remove_repository(
247255
coresys.store.repositories[store_addon.repository]
@@ -252,7 +260,7 @@ async def test_update_partial_error(coresys: CoreSys, store_manager: StoreManage
252260
"""Test partial error on update does partial save and errors."""
253261
with patch("supervisor.store.repository.RepositoryGit.validate", return_value=True):
254262
with patch("supervisor.store.git.GitRepo.load", return_value=None):
255-
await store_manager.update_repositories([])
263+
await store_manager.update_repositories(set())
256264

257265
store_manager.data.update.assert_called_once()
258266
store_manager.data.update.reset_mock()
@@ -268,7 +276,7 @@ async def test_update_partial_error(coresys: CoreSys, store_manager: StoreManage
268276
pytest.raises(StoreError),
269277
):
270278
await store_manager.update_repositories(
271-
current + ["http://example.com", "http://example2.com"]
279+
set(current) | {"http://example.com", "http://example2.com"}
272280
)
273281

274282
assert len(coresys.store.repository_urls) == initial + 1
@@ -303,7 +311,7 @@ async def test_add_with_update_repositories(
303311
),
304312
patch("pathlib.Path.exists", return_value=True),
305313
):
306-
await store_manager.update_repositories(["http://example.com"], replace=False)
314+
await store_manager.update_repositories({"http://example.com"}, replace=False)
307315

308316
assert test_repository.source in coresys.store.repository_urls
309317
assert "http://example.com" in coresys.store.repository_urls
@@ -322,7 +330,7 @@ async def test_add_repository_fails_if_out_of_date(
322330
):
323331
if use_update:
324332
await store_manager.update_repositories(
325-
coresys.store.repository_urls + ["http://example.com"],
333+
set(coresys.store.repository_urls) | {"http://example.com"}
326334
)
327335
else:
328336
await store_manager.add_repository("http://example.com")

0 commit comments

Comments
 (0)