From 9b8316df3f78d136ae73c096168bd73ffebc4465 Mon Sep 17 00:00:00 2001 From: Erik Montnemery Date: Mon, 25 Nov 2024 13:52:32 +0100 Subject: [PATCH] Revert "Make WS command backup/generate send events" (#131530) Revert "Make WS command backup/generate send events (#130524)" This reverts commit 093b16c7235a0ee69d88ff102e2838a747a96692. --- homeassistant/components/backup/__init__.py | 4 +- homeassistant/components/backup/manager.py | 62 ++--------- homeassistant/components/backup/websocket.py | 11 +- tests/components/backup/conftest.py | 73 ------------- .../backup/snapshots/test_websocket.ambr | 17 +-- tests/components/backup/test_manager.py | 101 ++++++++++-------- tests/components/backup/test_websocket.py | 18 ++-- 7 files changed, 86 insertions(+), 200 deletions(-) delete mode 100644 tests/components/backup/conftest.py diff --git a/homeassistant/components/backup/__init__.py b/homeassistant/components/backup/__init__.py index 907fda4c7f8ddc..200cb4a3f65591 100644 --- a/homeassistant/components/backup/__init__.py +++ b/homeassistant/components/backup/__init__.py @@ -32,9 +32,7 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: async def async_handle_create_service(call: ServiceCall) -> None: """Service handler for creating backups.""" - await backup_manager.async_create_backup(on_progress=None) - if backup_task := backup_manager.backup_task: - await backup_task + await backup_manager.async_create_backup() hass.services.async_register(DOMAIN, "create", async_handle_create_service) diff --git a/homeassistant/components/backup/manager.py b/homeassistant/components/backup/manager.py index ddc0a1eac3fb67..4300f75eed0be9 100644 --- a/homeassistant/components/backup/manager.py +++ b/homeassistant/components/backup/manager.py @@ -4,7 +4,6 @@ import abc import asyncio -from collections.abc import Callable from dataclasses import asdict, dataclass import hashlib import io @@ -35,13 +34,6 @@ BUF_SIZE = 2**20 * 4 # 4MB -@dataclass(slots=True) -class NewBackup: - """New backup class.""" - - slug: str - - @dataclass(slots=True) class Backup: """Backup class.""" @@ -57,15 +49,6 @@ def as_dict(self) -> dict: return {**asdict(self), "path": self.path.as_posix()} -@dataclass(slots=True) -class BackupProgress: - """Backup progress class.""" - - done: bool - stage: str | None - success: bool | None - - class BackupPlatformProtocol(Protocol): """Define the format that backup platforms can have.""" @@ -82,7 +65,7 @@ class BaseBackupManager(abc.ABC): def __init__(self, hass: HomeAssistant) -> None: """Initialize the backup manager.""" self.hass = hass - self.backup_task: asyncio.Task | None = None + self.backing_up = False self.backups: dict[str, Backup] = {} self.loaded_platforms = False self.platforms: dict[str, BackupPlatformProtocol] = {} @@ -150,12 +133,7 @@ async def async_restore_backup(self, slug: str, **kwargs: Any) -> None: """Restore a backup.""" @abc.abstractmethod - async def async_create_backup( - self, - *, - on_progress: Callable[[BackupProgress], None] | None, - **kwargs: Any, - ) -> NewBackup: + async def async_create_backup(self, **kwargs: Any) -> Backup: """Generate a backup.""" @abc.abstractmethod @@ -314,36 +292,17 @@ def _move_and_cleanup() -> None: await self.hass.async_add_executor_job(_move_and_cleanup) await self.load_backups() - async def async_create_backup( - self, - *, - on_progress: Callable[[BackupProgress], None] | None, - **kwargs: Any, - ) -> NewBackup: + async def async_create_backup(self, **kwargs: Any) -> Backup: """Generate a backup.""" - if self.backup_task: + if self.backing_up: raise HomeAssistantError("Backup already in progress") - backup_name = f"Core {HAVERSION}" - date_str = dt_util.now().isoformat() - slug = _generate_slug(date_str, backup_name) - self.backup_task = self.hass.async_create_task( - self._async_create_backup(backup_name, date_str, slug, on_progress), - name="backup_manager_create_backup", - eager_start=False, # To ensure the task is not started before we return - ) - return NewBackup(slug=slug) - async def _async_create_backup( - self, - backup_name: str, - date_str: str, - slug: str, - on_progress: Callable[[BackupProgress], None] | None, - ) -> Backup: - """Generate a backup.""" - success = False try: + self.backing_up = True await self.async_pre_backup_actions() + backup_name = f"Core {HAVERSION}" + date_str = dt_util.now().isoformat() + slug = _generate_slug(date_str, backup_name) backup_data = { "slug": slug, @@ -370,12 +329,9 @@ async def _async_create_backup( if self.loaded_backups: self.backups[slug] = backup LOGGER.debug("Generated new backup with slug %s", slug) - success = True return backup finally: - if on_progress: - on_progress(BackupProgress(done=True, stage=None, success=success)) - self.backup_task = None + self.backing_up = False await self.async_post_backup_actions() def _mkdir_and_generate_backup_contents( diff --git a/homeassistant/components/backup/websocket.py b/homeassistant/components/backup/websocket.py index a7c61b7c66c541..3ac8a7ace3e67f 100644 --- a/homeassistant/components/backup/websocket.py +++ b/homeassistant/components/backup/websocket.py @@ -8,7 +8,6 @@ from homeassistant.core import HomeAssistant, callback from .const import DATA_MANAGER, LOGGER -from .manager import BackupProgress @callback @@ -41,7 +40,7 @@ async def handle_info( msg["id"], { "backups": list(backups.values()), - "backing_up": manager.backup_task is not None, + "backing_up": manager.backing_up, }, ) @@ -114,11 +113,7 @@ async def handle_create( msg: dict[str, Any], ) -> None: """Generate a backup.""" - - def on_progress(progress: BackupProgress) -> None: - connection.send_message(websocket_api.event_message(msg["id"], progress)) - - backup = await hass.data[DATA_MANAGER].async_create_backup(on_progress=on_progress) + backup = await hass.data[DATA_MANAGER].async_create_backup() connection.send_result(msg["id"], backup) @@ -132,6 +127,7 @@ async def handle_backup_start( ) -> None: """Backup start notification.""" manager = hass.data[DATA_MANAGER] + manager.backing_up = True LOGGER.debug("Backup start notification") try: @@ -153,6 +149,7 @@ async def handle_backup_end( ) -> None: """Backup end notification.""" manager = hass.data[DATA_MANAGER] + manager.backing_up = False LOGGER.debug("Backup end notification") try: diff --git a/tests/components/backup/conftest.py b/tests/components/backup/conftest.py deleted file mode 100644 index 631c774e63cf65..00000000000000 --- a/tests/components/backup/conftest.py +++ /dev/null @@ -1,73 +0,0 @@ -"""Test fixtures for the Backup integration.""" - -from __future__ import annotations - -from collections.abc import Generator -from pathlib import Path -from unittest.mock import MagicMock, Mock, patch - -import pytest - -from homeassistant.core import HomeAssistant - - -@pytest.fixture(name="mocked_json_bytes") -def mocked_json_bytes_fixture() -> Generator[Mock]: - """Mock json_bytes.""" - with patch( - "homeassistant.components.backup.manager.json_bytes", - return_value=b"{}", # Empty JSON - ) as mocked_json_bytes: - yield mocked_json_bytes - - -@pytest.fixture(name="mocked_tarfile") -def mocked_tarfile_fixture() -> Generator[Mock]: - """Mock tarfile.""" - with patch( - "homeassistant.components.backup.manager.SecureTarFile" - ) as mocked_tarfile: - yield mocked_tarfile - - -@pytest.fixture(name="mock_backup_generation") -def mock_backup_generation_fixture( - hass: HomeAssistant, mocked_json_bytes: Mock, mocked_tarfile: Mock -) -> Generator[None]: - """Mock backup generator.""" - - def _mock_iterdir(path: Path) -> list[Path]: - if not path.name.endswith("testing_config"): - return [] - return [ - Path("test.txt"), - Path(".DS_Store"), - Path(".storage"), - ] - - with ( - patch("pathlib.Path.iterdir", _mock_iterdir), - patch("pathlib.Path.stat", MagicMock(st_size=123)), - patch("pathlib.Path.is_file", lambda x: x.name != ".storage"), - patch( - "pathlib.Path.is_dir", - lambda x: x.name == ".storage", - ), - patch( - "pathlib.Path.exists", - lambda x: x != Path(hass.config.path("backups")), - ), - patch( - "pathlib.Path.is_symlink", - lambda _: False, - ), - patch( - "pathlib.Path.mkdir", - MagicMock(), - ), - patch( - "homeassistant.components.backup.manager.HAVERSION", - "2025.1.0", - ), - ): - yield diff --git a/tests/components/backup/snapshots/test_websocket.ambr b/tests/components/backup/snapshots/test_websocket.ambr index 42eb524e529dd7..096df37d70477c 100644 --- a/tests/components/backup/snapshots/test_websocket.ambr +++ b/tests/components/backup/snapshots/test_websocket.ambr @@ -210,23 +210,16 @@ dict({ 'id': 1, 'result': dict({ - 'slug': '27f5c632', + 'date': '1970-01-01T00:00:00.000Z', + 'name': 'Test', + 'path': 'abc123.tar', + 'size': 0.0, + 'slug': 'abc123', }), 'success': True, 'type': 'result', }) # --- -# name: test_generate[without_hassio].1 - dict({ - 'event': dict({ - 'done': True, - 'stage': None, - 'success': True, - }), - 'id': 1, - 'type': 'event', - }) -# --- # name: test_info[with_hassio] dict({ 'error': dict({ diff --git a/tests/components/backup/test_manager.py b/tests/components/backup/test_manager.py index 9d24964aedf7b6..a3f70267643b01 100644 --- a/tests/components/backup/test_manager.py +++ b/tests/components/backup/test_manager.py @@ -2,7 +2,7 @@ from __future__ import annotations -import asyncio +from pathlib import Path from unittest.mock import AsyncMock, MagicMock, Mock, mock_open, patch import aiohttp @@ -10,10 +10,7 @@ import pytest from homeassistant.components.backup import BackupManager -from homeassistant.components.backup.manager import ( - BackupPlatformProtocol, - BackupProgress, -) +from homeassistant.components.backup.manager import BackupPlatformProtocol from homeassistant.core import HomeAssistant from homeassistant.exceptions import HomeAssistantError from homeassistant.setup import async_setup_component @@ -23,30 +20,59 @@ from tests.common import MockPlatform, mock_platform -async def _mock_backup_generation( - manager: BackupManager, mocked_json_bytes: Mock, mocked_tarfile: Mock -) -> None: +async def _mock_backup_generation(manager: BackupManager): """Mock backup generator.""" - progress: list[BackupProgress] = [] - - def on_progress(_progress: BackupProgress) -> None: - """Mock progress callback.""" - progress.append(_progress) - - assert manager.backup_task is None - await manager.async_create_backup(on_progress=on_progress) - assert manager.backup_task is not None - assert progress == [] + def _mock_iterdir(path: Path) -> list[Path]: + if not path.name.endswith("testing_config"): + return [] + return [ + Path("test.txt"), + Path(".DS_Store"), + Path(".storage"), + ] - await manager.backup_task - assert progress == [BackupProgress(done=True, stage=None, success=True)] - - assert mocked_json_bytes.call_count == 1 - backup_json_dict = mocked_json_bytes.call_args[0][0] - assert isinstance(backup_json_dict, dict) - assert backup_json_dict["homeassistant"] == {"version": "2025.1.0"} - assert manager.backup_dir.as_posix() in str(mocked_tarfile.call_args_list[0][0][0]) + with ( + patch( + "homeassistant.components.backup.manager.SecureTarFile" + ) as mocked_tarfile, + patch("pathlib.Path.iterdir", _mock_iterdir), + patch("pathlib.Path.stat", MagicMock(st_size=123)), + patch("pathlib.Path.is_file", lambda x: x.name != ".storage"), + patch( + "pathlib.Path.is_dir", + lambda x: x.name == ".storage", + ), + patch( + "pathlib.Path.exists", + lambda x: x != manager.backup_dir, + ), + patch( + "pathlib.Path.is_symlink", + lambda _: False, + ), + patch( + "pathlib.Path.mkdir", + MagicMock(), + ), + patch( + "homeassistant.components.backup.manager.json_bytes", + return_value=b"{}", # Empty JSON + ) as mocked_json_bytes, + patch( + "homeassistant.components.backup.manager.HAVERSION", + "2025.1.0", + ), + ): + await manager.async_create_backup() + + assert mocked_json_bytes.call_count == 1 + backup_json_dict = mocked_json_bytes.call_args[0][0] + assert isinstance(backup_json_dict, dict) + assert backup_json_dict["homeassistant"] == {"version": "2025.1.0"} + assert manager.backup_dir.as_posix() in str( + mocked_tarfile.call_args_list[0][0][0] + ) async def _setup_mock_domain( @@ -150,26 +176,21 @@ async def test_getting_backup_that_does_not_exist( async def test_async_create_backup_when_backing_up(hass: HomeAssistant) -> None: """Test generate backup.""" - event = asyncio.Event() manager = BackupManager(hass) - manager.backup_task = hass.async_create_task(event.wait()) + manager.backing_up = True with pytest.raises(HomeAssistantError, match="Backup already in progress"): - await manager.async_create_backup(on_progress=None) - event.set() + await manager.async_create_backup() -@pytest.mark.usefixtures("mock_backup_generation") async def test_async_create_backup( hass: HomeAssistant, caplog: pytest.LogCaptureFixture, - mocked_json_bytes: Mock, - mocked_tarfile: Mock, ) -> None: """Test generate backup.""" manager = BackupManager(hass) manager.loaded_backups = True - await _mock_backup_generation(manager, mocked_json_bytes, mocked_tarfile) + await _mock_backup_generation(manager) assert "Generated new backup with slug " in caplog.text assert "Creating backup directory" in caplog.text @@ -226,9 +247,7 @@ async def test_not_loading_bad_platforms( ) -async def test_exception_plaform_pre( - hass: HomeAssistant, mocked_json_bytes: Mock, mocked_tarfile: Mock -) -> None: +async def test_exception_plaform_pre(hass: HomeAssistant) -> None: """Test exception in pre step.""" manager = BackupManager(hass) manager.loaded_backups = True @@ -245,12 +264,10 @@ async def _mock_step(hass: HomeAssistant) -> None: ) with pytest.raises(HomeAssistantError): - await _mock_backup_generation(manager, mocked_json_bytes, mocked_tarfile) + await _mock_backup_generation(manager) -async def test_exception_plaform_post( - hass: HomeAssistant, mocked_json_bytes: Mock, mocked_tarfile: Mock -) -> None: +async def test_exception_plaform_post(hass: HomeAssistant) -> None: """Test exception in post step.""" manager = BackupManager(hass) manager.loaded_backups = True @@ -267,7 +284,7 @@ async def _mock_step(hass: HomeAssistant) -> None: ) with pytest.raises(HomeAssistantError): - await _mock_backup_generation(manager, mocked_json_bytes, mocked_tarfile) + await _mock_backup_generation(manager) async def test_loading_platforms_when_running_async_pre_backup_actions( diff --git a/tests/components/backup/test_websocket.py b/tests/components/backup/test_websocket.py index 3e031f172aee98..125ba8adaad1c4 100644 --- a/tests/components/backup/test_websocket.py +++ b/tests/components/backup/test_websocket.py @@ -2,7 +2,6 @@ from unittest.mock import patch -from freezegun.api import FrozenDateTimeFactory import pytest from syrupy import SnapshotAssertion @@ -116,30 +115,29 @@ async def test_remove( @pytest.mark.parametrize( - ("with_hassio", "number_of_messages"), + "with_hassio", [ - pytest.param(True, 1, id="with_hassio"), - pytest.param(False, 2, id="without_hassio"), + pytest.param(True, id="with_hassio"), + pytest.param(False, id="without_hassio"), ], ) -@pytest.mark.usefixtures("mock_backup_generation") async def test_generate( hass: HomeAssistant, hass_ws_client: WebSocketGenerator, - freezer: FrozenDateTimeFactory, snapshot: SnapshotAssertion, with_hassio: bool, - number_of_messages: int, ) -> None: """Test generating a backup.""" await setup_backup_integration(hass, with_hassio=with_hassio) client = await hass_ws_client(hass) - freezer.move_to("2024-11-13 12:01:00+01:00") await hass.async_block_till_done() - await client.send_json_auto_id({"type": "backup/generate"}) - for _ in range(number_of_messages): + with patch( + "homeassistant.components.backup.manager.BackupManager.async_create_backup", + return_value=TEST_BACKUP, + ): + await client.send_json_auto_id({"type": "backup/generate"}) assert snapshot == await client.receive_json()