From 5a8b0ac40f249d787159965b9668cfb6dd5a9b18 Mon Sep 17 00:00:00 2001 From: Erik Date: Mon, 8 Apr 2024 14:51:07 +0200 Subject: [PATCH 1/5] Add async wrapper for commonly used filesystem functions --- custom_components/hacs/__init__.py | 5 ++-- custom_components/hacs/base.py | 13 +++++----- custom_components/hacs/frontend.py | 7 +++--- custom_components/hacs/repositories/base.py | 13 +++++----- custom_components/hacs/repositories/plugin.py | 2 +- custom_components/hacs/repositories/theme.py | 2 +- custom_components/hacs/utils/file_system.py | 24 +++++++++++++++++++ 7 files changed, 45 insertions(+), 21 deletions(-) create mode 100644 custom_components/hacs/utils/file_system.py diff --git a/custom_components/hacs/__init__.py b/custom_components/hacs/__init__.py index 755ec25d9ed..8a03a696f24 100644 --- a/custom_components/hacs/__init__.py +++ b/custom_components/hacs/__init__.py @@ -31,6 +31,7 @@ from .frontend import async_register_frontend from .utils.configuration_schema import hacs_config_combined from .utils.data import HacsData +from .utils.file_system import async_exists from .utils.logger import LOGGER from .utils.queue_manager import QueueManager from .utils.version import version_left_higher_or_equal_then_right @@ -135,7 +136,7 @@ async def async_startup(): hass.config.path("custom_components/custom_updater.py"), hass.config.path("custom_components/custom_updater/__init__.py"), ): - if os.path.exists(location): + if await async_exists(location): hacs.log.critical( "This cannot be used with custom_updater. " "To use this you need to remove custom_updater form %s", @@ -167,7 +168,7 @@ async def async_startup(): hacs.set_active_categories() async_register_websocket_commands(hass) - async_register_frontend(hass, hacs) + await async_register_frontend(hass, hacs) if hacs.configuration.config_type == ConfigurationType.YAML: hass.async_create_task( diff --git a/custom_components/hacs/base.py b/custom_components/hacs/base.py index 76c7785cd20..710a8acd5be 100644 --- a/custom_components/hacs/base.py +++ b/custom_components/hacs/base.py @@ -60,6 +60,7 @@ ) from .repositories import RERPOSITORY_CLASSES from .utils.decode import decode_content +from .utils.file_system import async_exists from .utils.json import json_loads from .utils.logger import LOGGER from .utils.queue_manager import QueueManager @@ -463,7 +464,7 @@ def _write_file(): self.log.error("Could not write data to %s - %s", file_path, error) return False - return os.path.exists(file_path) + return await async_exists(file_path) async def async_can_update(self) -> int: """Helper to calculate the number of repositories we can fetch data for.""" @@ -1143,10 +1144,9 @@ async def async_handle_critical_repositories(self, _=None) -> None: self.log.critical("Restarting Home Assistant") self.hass.async_create_task(self.hass.async_stop(100)) - @callback - def async_setup_frontend_endpoint_plugin(self) -> None: + async def async_setup_frontend_endpoint_plugin(self) -> None: """Setup the http endpoints for plugins if its not already handled.""" - if self.status.active_frontend_endpoint_plugin or not os.path.exists( + if self.status.active_frontend_endpoint_plugin or not await async_exists( self.hass.config.path("www/community") ): return @@ -1167,13 +1167,12 @@ def async_setup_frontend_endpoint_plugin(self) -> None: self.status.active_frontend_endpoint_plugin = True - @callback - def async_setup_frontend_endpoint_themes(self) -> None: + async def async_setup_frontend_endpoint_themes(self) -> None: """Setup the http endpoints for themes if its not already handled.""" if ( self.configuration.experimental or self.status.active_frontend_endpoint_theme - or not os.path.exists(self.hass.config.path("themes")) + or not await async_exists(self.hass.config.path("themes")) ): return diff --git a/custom_components/hacs/frontend.py b/custom_components/hacs/frontend.py index 6557517db9a..f9db6227c31 100644 --- a/custom_components/hacs/frontend.py +++ b/custom_components/hacs/frontend.py @@ -29,12 +29,11 @@ def add_extra_js_url(hass: HomeAssistant, url: str, es5: bool = False) -> None: from .base import HacsBase -@callback -def async_register_frontend(hass: HomeAssistant, hacs: HacsBase) -> None: +async def async_register_frontend(hass: HomeAssistant, hacs: HacsBase) -> None: """Register the frontend.""" # Setup themes endpoint if needed - hacs.async_setup_frontend_endpoint_themes() + await hacs.async_setup_frontend_endpoint_themes() # Register frontend if hacs.configuration.dev and (frontend_path := os.getenv("HACS_FRONTEND_DIR")): @@ -82,4 +81,4 @@ def async_register_frontend(hass: HomeAssistant, hacs: HacsBase) -> None: ) # Setup plugin endpoint if needed - hacs.async_setup_frontend_endpoint_plugin() + await hacs.async_setup_frontend_endpoint_plugin() diff --git a/custom_components/hacs/repositories/base.py b/custom_components/hacs/repositories/base.py index 4af6fb00c34..8df9b0a0a21 100644 --- a/custom_components/hacs/repositories/base.py +++ b/custom_components/hacs/repositories/base.py @@ -31,6 +31,7 @@ from ..utils.backup import Backup, BackupNetDaemon from ..utils.decode import decode_content from ..utils.decorator import concurrent +from ..utils.file_system import async_exists, async_remove from ..utils.filters import filter_content_return_one_of_type from ..utils.json import json_loads from ..utils.logger import LOGGER @@ -802,8 +803,8 @@ async def remove_local_directory(self) -> None: f"{self.hacs.configuration.theme_path}/" f"{self.data.name}.yaml" ) - if os.path.exists(path): - os.remove(path) + if await async_exists(path): + await async_remove(path) local_path = self.content.path.local elif self.data.category == "integration": if not self.data.domain: @@ -817,18 +818,18 @@ async def remove_local_directory(self) -> None: else: local_path = self.content.path.local - if os.path.exists(local_path): + if await async_exists(local_path): if not is_safe(self.hacs, local_path): self.logger.error("%s Path %s is blocked from removal", self.string, local_path) return False self.logger.debug("%s Removing %s", self.string, local_path) if self.data.category in ["python_script", "template"]: - os.remove(local_path) + await async_remove(local_path) else: shutil.rmtree(local_path) - while os.path.exists(local_path): + while await async_exists(local_path): await sleep(1) else: self.logger.debug( @@ -948,7 +949,7 @@ async def async_install_repository(self, *, version: str | None = None, **_) -> await self.hacs.hass.async_add_executor_job(persistent_directory.create) elif self.repository_manifest.persistent_directory: - if os.path.exists( + if await async_exists( f"{self.content.path.local}/{self.repository_manifest.persistent_directory}" ): persistent_directory = Backup( diff --git a/custom_components/hacs/repositories/plugin.py b/custom_components/hacs/repositories/plugin.py index 63d95e4b8be..35be9854e98 100644 --- a/custom_components/hacs/repositories/plugin.py +++ b/custom_components/hacs/repositories/plugin.py @@ -55,7 +55,7 @@ async def validate_repository(self): async def async_post_installation(self): """Run post installation steps.""" - self.hacs.async_setup_frontend_endpoint_plugin() + await self.hacs.async_setup_frontend_endpoint_plugin() @concurrent(concurrenttasks=10, backoff_time=5) async def update_repository(self, ignore_issues=False, force=False): diff --git a/custom_components/hacs/repositories/theme.py b/custom_components/hacs/repositories/theme.py index 54d417f02be..9e6c8fad458 100644 --- a/custom_components/hacs/repositories/theme.py +++ b/custom_components/hacs/repositories/theme.py @@ -37,7 +37,7 @@ async def async_post_installation(self): except BaseException: # lgtm [py/catch-base-exception] pylint: disable=broad-except pass - self.hacs.async_setup_frontend_endpoint_themes() + await self.hacs.async_setup_frontend_endpoint_themes() async def validate_repository(self): """Validate.""" diff --git a/custom_components/hacs/utils/file_system.py b/custom_components/hacs/utils/file_system.py new file mode 100644 index 00000000000..61069b4645b --- /dev/null +++ b/custom_components/hacs/utils/file_system.py @@ -0,0 +1,24 @@ +"""File system functions.""" + +from __future__ import annotations + +import os +from typing import TypeAlias + +from homeassistant.core import HomeAssistant + +# From typeshed +StrOrBytesPath: TypeAlias = str | bytes | os.PathLike[str] | os.PathLike[bytes] +FileDescriptorOrPath: TypeAlias = int | StrOrBytesPath + + +async def async_exists(hass: HomeAssistant, path: FileDescriptorOrPath) -> None: + """Test whether a path exists.""" + return await hass.async_add_executor_job(os.path.exists, path) + + +async def async_remove( + hass: HomeAssistant, path: StrOrBytesPath, *, dir_fd: int | None = None +) -> None: + """Remove a path.""" + return await hass.async_add_executor_job(os.remove, path) From cd21663ca1cdb37dd58e443598927c6fe4575d7a Mon Sep 17 00:00:00 2001 From: Erik Date: Mon, 8 Apr 2024 15:14:15 +0200 Subject: [PATCH 2/5] Don't forget to pass hass, fix return type --- custom_components/hacs/__init__.py | 2 +- custom_components/hacs/base.py | 6 +++--- custom_components/hacs/repositories/base.py | 13 +++++++------ custom_components/hacs/utils/file_system.py | 2 +- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/custom_components/hacs/__init__.py b/custom_components/hacs/__init__.py index 8a03a696f24..07cf16a4806 100644 --- a/custom_components/hacs/__init__.py +++ b/custom_components/hacs/__init__.py @@ -136,7 +136,7 @@ async def async_startup(): hass.config.path("custom_components/custom_updater.py"), hass.config.path("custom_components/custom_updater/__init__.py"), ): - if await async_exists(location): + if await async_exists(hass, location): hacs.log.critical( "This cannot be used with custom_updater. " "To use this you need to remove custom_updater form %s", diff --git a/custom_components/hacs/base.py b/custom_components/hacs/base.py index 710a8acd5be..00a02c71dfb 100644 --- a/custom_components/hacs/base.py +++ b/custom_components/hacs/base.py @@ -464,7 +464,7 @@ def _write_file(): self.log.error("Could not write data to %s - %s", file_path, error) return False - return await async_exists(file_path) + return await async_exists(self.hass, file_path) async def async_can_update(self) -> int: """Helper to calculate the number of repositories we can fetch data for.""" @@ -1147,7 +1147,7 @@ async def async_handle_critical_repositories(self, _=None) -> None: async def async_setup_frontend_endpoint_plugin(self) -> None: """Setup the http endpoints for plugins if its not already handled.""" if self.status.active_frontend_endpoint_plugin or not await async_exists( - self.hass.config.path("www/community") + self.hass, self.hass.config.path("www/community") ): return @@ -1172,7 +1172,7 @@ async def async_setup_frontend_endpoint_themes(self) -> None: if ( self.configuration.experimental or self.status.active_frontend_endpoint_theme - or not await async_exists(self.hass.config.path("themes")) + or not await async_exists(self.hass, self.hass.config.path("themes")) ): return diff --git a/custom_components/hacs/repositories/base.py b/custom_components/hacs/repositories/base.py index 8df9b0a0a21..80635448eac 100644 --- a/custom_components/hacs/repositories/base.py +++ b/custom_components/hacs/repositories/base.py @@ -803,8 +803,8 @@ async def remove_local_directory(self) -> None: f"{self.hacs.configuration.theme_path}/" f"{self.data.name}.yaml" ) - if await async_exists(path): - await async_remove(path) + if await async_exists(self.hacs.hass, path): + await async_remove(self.hacs.hass, path) local_path = self.content.path.local elif self.data.category == "integration": if not self.data.domain: @@ -818,18 +818,18 @@ async def remove_local_directory(self) -> None: else: local_path = self.content.path.local - if await async_exists(local_path): + if await async_exists(self.hacs.hass, local_path): if not is_safe(self.hacs, local_path): self.logger.error("%s Path %s is blocked from removal", self.string, local_path) return False self.logger.debug("%s Removing %s", self.string, local_path) if self.data.category in ["python_script", "template"]: - await async_remove(local_path) + await async_remove(self.hacs.hass, local_path) else: shutil.rmtree(local_path) - while await async_exists(local_path): + while await async_exists(self.hacs.hass, local_path): await sleep(1) else: self.logger.debug( @@ -950,7 +950,8 @@ async def async_install_repository(self, *, version: str | None = None, **_) -> elif self.repository_manifest.persistent_directory: if await async_exists( - f"{self.content.path.local}/{self.repository_manifest.persistent_directory}" + self.hacs.hass, + f"{self.content.path.local}/{self.repository_manifest.persistent_directory}", ): persistent_directory = Backup( hacs=self.hacs, diff --git a/custom_components/hacs/utils/file_system.py b/custom_components/hacs/utils/file_system.py index 61069b4645b..e03454b2c40 100644 --- a/custom_components/hacs/utils/file_system.py +++ b/custom_components/hacs/utils/file_system.py @@ -12,7 +12,7 @@ FileDescriptorOrPath: TypeAlias = int | StrOrBytesPath -async def async_exists(hass: HomeAssistant, path: FileDescriptorOrPath) -> None: +async def async_exists(hass: HomeAssistant, path: FileDescriptorOrPath) -> bool: """Test whether a path exists.""" return await hass.async_add_executor_job(os.path.exists, path) From 4221b0399a03b53212c153d0a27415ac36f13d66 Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 12 Apr 2024 09:22:16 +0200 Subject: [PATCH 3/5] Add missing_ok flag to async_remove --- custom_components/hacs/repositories/base.py | 3 +-- custom_components/hacs/utils/file_system.py | 9 +++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/custom_components/hacs/repositories/base.py b/custom_components/hacs/repositories/base.py index ab7f2ce2527..78968a1978c 100644 --- a/custom_components/hacs/repositories/base.py +++ b/custom_components/hacs/repositories/base.py @@ -797,8 +797,7 @@ async def remove_local_directory(self) -> None: f"{self.hacs.configuration.theme_path}/" f"{self.data.name}.yaml" ) - if await async_exists(self.hacs.hass, path): - await async_remove(self.hacs.hass, path) + await async_remove(self.hacs.hass, path, missing_ok=True) local_path = self.content.path.local elif self.data.category == "integration": if not self.data.domain: diff --git a/custom_components/hacs/utils/file_system.py b/custom_components/hacs/utils/file_system.py index e03454b2c40..8c5fff929cc 100644 --- a/custom_components/hacs/utils/file_system.py +++ b/custom_components/hacs/utils/file_system.py @@ -18,7 +18,12 @@ async def async_exists(hass: HomeAssistant, path: FileDescriptorOrPath) -> bool: async def async_remove( - hass: HomeAssistant, path: StrOrBytesPath, *, dir_fd: int | None = None + hass: HomeAssistant, path: StrOrBytesPath, *, missing_ok: bool = False ) -> None: """Remove a path.""" - return await hass.async_add_executor_job(os.remove, path) + try: + return await hass.async_add_executor_job(os.remove, path) + except FileNotFoundError: + if missing_ok: + return + raise From c9acb68c92ab37ec5bbb4035a63a7d8339123088 Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 12 Apr 2024 09:29:40 +0200 Subject: [PATCH 4/5] Add tests --- tests/utils/test_fs_util.py | 40 +++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 tests/utils/test_fs_util.py diff --git a/tests/utils/test_fs_util.py b/tests/utils/test_fs_util.py new file mode 100644 index 00000000000..b1ae2f6b8b3 --- /dev/null +++ b/tests/utils/test_fs_util.py @@ -0,0 +1,40 @@ +"""Test fs_util.""" + +from contextlib import nullcontext as does_not_raise +import os + +import pytest + +from custom_components.hacs.utils.file_system import async_exists, async_remove + +from tests.common import fixture + + +async def test_async_exists(hass, tmpdir): + """Test async_exists.""" + assert not await async_exists(hass, tmpdir/"tmptmp") + + open(tmpdir/"tmptmp", 'w').close() + assert await async_exists(hass, tmpdir/"tmptmp") + + +async def test_async_remove(hass, tmpdir): + """Test async_remove.""" + assert not await async_exists(hass, tmpdir/"tmptmp") + with pytest.raises(FileNotFoundError): + await async_remove(hass, tmpdir/"tmptmp") + with pytest.raises(FileNotFoundError): + await async_remove(hass, tmpdir/"tmptmp", missing_ok=False) + await async_remove(hass, tmpdir/"tmptmp", missing_ok=True) + + open(tmpdir/"tmptmp", 'w').close() + await async_remove(hass, tmpdir/"tmptmp") + assert not await async_exists(hass, tmpdir/"tmptmp") + + open(tmpdir/"tmptmp", 'w').close() + await async_remove(hass, tmpdir/"tmptmp", missing_ok=False) + assert not await async_exists(hass, tmpdir/"tmptmp") + + open(tmpdir/"tmptmp", 'w').close() + await async_remove(hass, tmpdir/"tmptmp", missing_ok=True) + assert not await async_exists(hass, tmpdir/"tmptmp") From 9c80e02396b7174e38506f670de08ade4247624d Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 12 Apr 2024 09:37:30 +0200 Subject: [PATCH 5/5] Black --- tests/utils/test_fs_util.py | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/tests/utils/test_fs_util.py b/tests/utils/test_fs_util.py index b1ae2f6b8b3..230799971ff 100644 --- a/tests/utils/test_fs_util.py +++ b/tests/utils/test_fs_util.py @@ -12,29 +12,29 @@ async def test_async_exists(hass, tmpdir): """Test async_exists.""" - assert not await async_exists(hass, tmpdir/"tmptmp") + assert not await async_exists(hass, tmpdir / "tmptmp") - open(tmpdir/"tmptmp", 'w').close() - assert await async_exists(hass, tmpdir/"tmptmp") + open(tmpdir / "tmptmp", "w").close() + assert await async_exists(hass, tmpdir / "tmptmp") async def test_async_remove(hass, tmpdir): """Test async_remove.""" - assert not await async_exists(hass, tmpdir/"tmptmp") + assert not await async_exists(hass, tmpdir / "tmptmp") with pytest.raises(FileNotFoundError): - await async_remove(hass, tmpdir/"tmptmp") + await async_remove(hass, tmpdir / "tmptmp") with pytest.raises(FileNotFoundError): - await async_remove(hass, tmpdir/"tmptmp", missing_ok=False) - await async_remove(hass, tmpdir/"tmptmp", missing_ok=True) + await async_remove(hass, tmpdir / "tmptmp", missing_ok=False) + await async_remove(hass, tmpdir / "tmptmp", missing_ok=True) - open(tmpdir/"tmptmp", 'w').close() - await async_remove(hass, tmpdir/"tmptmp") - assert not await async_exists(hass, tmpdir/"tmptmp") + open(tmpdir / "tmptmp", "w").close() + await async_remove(hass, tmpdir / "tmptmp") + assert not await async_exists(hass, tmpdir / "tmptmp") - open(tmpdir/"tmptmp", 'w').close() - await async_remove(hass, tmpdir/"tmptmp", missing_ok=False) - assert not await async_exists(hass, tmpdir/"tmptmp") + open(tmpdir / "tmptmp", "w").close() + await async_remove(hass, tmpdir / "tmptmp", missing_ok=False) + assert not await async_exists(hass, tmpdir / "tmptmp") - open(tmpdir/"tmptmp", 'w').close() - await async_remove(hass, tmpdir/"tmptmp", missing_ok=True) - assert not await async_exists(hass, tmpdir/"tmptmp") + open(tmpdir / "tmptmp", "w").close() + await async_remove(hass, tmpdir / "tmptmp", missing_ok=True) + assert not await async_exists(hass, tmpdir / "tmptmp")