From d197acc0692c7cf115aa74416ba318b6a5817104 Mon Sep 17 00:00:00 2001 From: Jan-Philipp Benecke Date: Tue, 25 Feb 2025 11:46:40 +0100 Subject: [PATCH] Reduce requests made by webdav (#139238) * Reduce requests made by webdav * Update homeassistant/components/webdav/backup.py Co-authored-by: Martin Hjelmare --------- Co-authored-by: Martin Hjelmare --- homeassistant/components/webdav/backup.py | 70 +++++++++++++---------- tests/components/webdav/conftest.py | 19 +----- tests/components/webdav/const.py | 49 +++++----------- tests/components/webdav/test_backup.py | 38 ++++++++++-- 4 files changed, 90 insertions(+), 86 deletions(-) diff --git a/homeassistant/components/webdav/backup.py b/homeassistant/components/webdav/backup.py index 2c19ca450e3..a51866fde61 100644 --- a/homeassistant/components/webdav/backup.py +++ b/homeassistant/components/webdav/backup.py @@ -95,6 +95,23 @@ def suggested_filenames(backup: AgentBackup) -> tuple[str, str]: return f"{base_name}.tar", f"{base_name}.metadata.json" +def _is_current_metadata_version(properties: list[Property]) -> bool: + """Check if any property is of the current metadata version.""" + return any( + prop.value == METADATA_VERSION + for prop in properties + if prop.namespace == "homeassistant" and prop.name == "metadata_version" + ) + + +def _backup_id_from_properties(properties: list[Property]) -> str | None: + """Return the backup ID from properties.""" + for prop in properties: + if prop.namespace == "homeassistant" and prop.name == "backup_id": + return prop.value + return None + + class WebDavBackupAgent(BackupAgent): """Backup agent interface.""" @@ -217,7 +234,7 @@ class WebDavBackupAgent(BackupAgent): metadata_files = await self._list_metadata_files() return [ await self._download_metadata(metadata_file) - for metadata_file in metadata_files + for metadata_file in metadata_files.values() ] @handle_backup_errors @@ -229,40 +246,33 @@ class WebDavBackupAgent(BackupAgent): """Return a backup.""" return await self._find_backup_by_id(backup_id) - async def _list_metadata_files(self) -> list[str]: + async def _list_metadata_files(self) -> dict[str, str]: """List metadata files.""" - files = await self._client.list_with_infos(self._backup_path) - return [ - file["path"] - for file in files - if file["path"].endswith(".json") - and await self._is_current_metadata_version(file["path"]) - ] - - async def _is_current_metadata_version(self, path: str) -> bool: - """Check if is current metadata version.""" - metadata_version = await self._client.get_property( - path, - PropertyRequest( - namespace="homeassistant", - name="metadata_version", - ), - ) - return metadata_version.value == METADATA_VERSION if metadata_version else False - - async def _find_backup_by_id(self, backup_id: str) -> AgentBackup | None: - """Find a backup by its backup ID on remote.""" - metadata_files = await self._list_metadata_files() - for metadata_file in metadata_files: - remote_backup_id = await self._client.get_property( - metadata_file, + files = await self._client.list_with_properties( + self._backup_path, + [ + PropertyRequest( + namespace="homeassistant", + name="metadata_version", + ), PropertyRequest( namespace="homeassistant", name="backup_id", ), - ) - if remote_backup_id and remote_backup_id.value == backup_id: - return await self._download_metadata(metadata_file) + ], + ) + return { + backup_id: file_name + for file_name, properties in files.items() + if file_name.endswith(".json") and _is_current_metadata_version(properties) + if (backup_id := _backup_id_from_properties(properties)) + } + + async def _find_backup_by_id(self, backup_id: str) -> AgentBackup | None: + """Find a backup by its backup ID on remote.""" + metadata_files = await self._list_metadata_files() + if metadata_file := metadata_files.get(backup_id): + return await self._download_metadata(metadata_file) return None diff --git a/tests/components/webdav/conftest.py b/tests/components/webdav/conftest.py index ccd3437aaa0..4fdd6fb7870 100644 --- a/tests/components/webdav/conftest.py +++ b/tests/components/webdav/conftest.py @@ -4,18 +4,12 @@ from collections.abc import AsyncIterator, Generator from json import dumps from unittest.mock import AsyncMock, patch -from aiowebdav2 import Property, PropertyRequest import pytest from homeassistant.components.webdav.const import DOMAIN from homeassistant.const import CONF_PASSWORD, CONF_URL, CONF_USERNAME -from .const import ( - BACKUP_METADATA, - MOCK_GET_PROPERTY_BACKUP_ID, - MOCK_GET_PROPERTY_METADATA_VERSION, - MOCK_LIST_WITH_INFOS, -) +from .const import BACKUP_METADATA, MOCK_LIST_WITH_PROPERTIES from tests.common import MockConfigEntry @@ -44,14 +38,6 @@ def mock_config_entry() -> MockConfigEntry: ) -def _get_property(path: str, request: PropertyRequest) -> Property: - """Return the property of a file.""" - if path.endswith(".json") and request.name == "metadata_version": - return MOCK_GET_PROPERTY_METADATA_VERSION - - return MOCK_GET_PROPERTY_BACKUP_ID - - async def _download_mock(path: str, timeout=None) -> AsyncIterator[bytes]: """Mock the download function.""" if path.endswith(".json"): @@ -72,9 +58,8 @@ def mock_webdav_client() -> Generator[AsyncMock]: mock = mock_webdav_client.return_value mock.check.return_value = True mock.mkdir.return_value = True - mock.list_with_infos.return_value = MOCK_LIST_WITH_INFOS + mock.list_with_properties.return_value = MOCK_LIST_WITH_PROPERTIES mock.download_iter.side_effect = _download_mock mock.upload_iter.return_value = None mock.clean.return_value = None - mock.get_property.side_effect = _get_property yield mock diff --git a/tests/components/webdav/const.py b/tests/components/webdav/const.py index 777008b07a5..52cad9a163b 100644 --- a/tests/components/webdav/const.py +++ b/tests/components/webdav/const.py @@ -16,37 +16,18 @@ BACKUP_METADATA = { "size": 34519040, } -MOCK_LIST_WITH_INFOS = [ - { - "content_type": "application/x-tar", - "created": "2025-02-10T17:47:22Z", - "etag": '"84d7d000-62dcd4ce886b4"', - "isdir": "False", - "modified": "Mon, 10 Feb 2025 17:47:22 GMT", - "name": "None", - "path": "/Automatic_backup_2025.2.1_2025-02-10_18.31_30202686.tar", - "size": "2228736000", - }, - { - "content_type": "application/json", - "created": "2025-02-10T17:47:22Z", - "etag": '"8d0-62dcd4cec050a"', - "isdir": "False", - "modified": "Mon, 10 Feb 2025 17:47:22 GMT", - "name": "None", - "path": "/Automatic_backup_2025.2.1_2025-02-10_18.31_30202686.metadata.json", - "size": "2256", - }, -] - -MOCK_GET_PROPERTY_METADATA_VERSION = Property( - namespace="homeassistant", - name="metadata_version", - value="1", -) - -MOCK_GET_PROPERTY_BACKUP_ID = Property( - namespace="homeassistant", - name="backup_id", - value="23e64aec", -) +MOCK_LIST_WITH_PROPERTIES = { + "/Automatic_backup_2025.2.1_2025-02-10_18.31_30202686.tar": [], + "/Automatic_backup_2025.2.1_2025-02-10_18.31_30202686.metadata.json": [ + Property( + namespace="homeassistant", + name="backup_id", + value="23e64aec", + ), + Property( + namespace="homeassistant", + name="metadata_version", + value="1", + ), + ], +} diff --git a/tests/components/webdav/test_backup.py b/tests/components/webdav/test_backup.py index 2219e92f700..c20e73cc786 100644 --- a/tests/components/webdav/test_backup.py +++ b/tests/components/webdav/test_backup.py @@ -6,6 +6,7 @@ from collections.abc import AsyncGenerator from io import StringIO from unittest.mock import Mock, patch +from aiowebdav2 import Property from aiowebdav2.exceptions import UnauthorizedError, WebDavError import pytest @@ -16,7 +17,7 @@ from homeassistant.core import HomeAssistant from homeassistant.helpers.backup import async_initialize_backup from homeassistant.setup import async_setup_component -from .const import BACKUP_METADATA, MOCK_LIST_WITH_INFOS +from .const import BACKUP_METADATA, MOCK_LIST_WITH_PROPERTIES from tests.common import AsyncMock, MockConfigEntry from tests.typing import ClientSessionGenerator, WebSocketGenerator @@ -210,7 +211,7 @@ async def test_error_on_agents_download( """Test we get not found on a not existing backup on download.""" client = await hass_client() backup_id = BACKUP_METADATA["backup_id"] - webdav_client.list_with_infos.side_effect = [MOCK_LIST_WITH_INFOS, []] + webdav_client.list_with_properties.side_effect = [MOCK_LIST_WITH_PROPERTIES, {}] resp = await client.get( f"/api/backup/download/{backup_id}?agent_id={DOMAIN}.{mock_config_entry.entry_id}" @@ -261,7 +262,7 @@ async def test_agents_delete_not_found_does_not_throw( webdav_client: AsyncMock, ) -> None: """Test agent delete backup.""" - webdav_client.list_with_infos.return_value = [] + webdav_client.list_with_properties.return_value = {} client = await hass_ws_client(hass) await client.send_json_auto_id( @@ -282,7 +283,7 @@ async def test_agents_backup_not_found( webdav_client: AsyncMock, ) -> None: """Test backup not found.""" - webdav_client.list_with_infos.return_value = [] + webdav_client.list_with_properties.return_value = [] backup_id = BACKUP_METADATA["backup_id"] client = await hass_ws_client(hass) await client.send_json_auto_id({"type": "backup/details", "backup_id": backup_id}) @@ -299,7 +300,7 @@ async def test_raises_on_403( mock_config_entry: MockConfigEntry, ) -> None: """Test we raise on 403.""" - webdav_client.list_with_infos.side_effect = UnauthorizedError( + webdav_client.list_with_properties.side_effect = UnauthorizedError( "https://webdav.example.com" ) backup_id = BACKUP_METADATA["backup_id"] @@ -323,3 +324,30 @@ async def test_listeners_get_cleaned_up(hass: HomeAssistant) -> None: remove_listener() assert hass.data.get(DATA_BACKUP_AGENT_LISTENERS) is None + + +async def test_metadata_misses_backup_id( + hass: HomeAssistant, + hass_ws_client: WebSocketGenerator, + webdav_client: AsyncMock, + mock_config_entry: MockConfigEntry, +) -> None: + """Test getting a backup when metadata has backup id property.""" + MOCK_LIST_WITH_PROPERTIES[ + "/Automatic_backup_2025.2.1_2025-02-10_18.31_30202686.metadata.json" + ] = [ + Property( + namespace="homeassistant", + name="metadata_version", + value="1", + ) + ] + webdav_client.list_with_properties.return_value = MOCK_LIST_WITH_PROPERTIES + + backup_id = BACKUP_METADATA["backup_id"] + client = await hass_ws_client(hass) + await client.send_json_auto_id({"type": "backup/details", "backup_id": backup_id}) + response = await client.receive_json() + + assert response["success"] + assert response["result"]["backup"] is None