From 2846dcc0352e44637c7bc21069f3c3d8bc1da4d2 Mon Sep 17 00:00:00 2001 From: Leonardo Rivera Date: Mon, 4 May 2026 15:17:41 -0300 Subject: [PATCH] Add delete service action to OneDrive integration (#168064) Co-authored-by: Josef Zweck Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- homeassistant/components/onedrive/icons.json | 3 + homeassistant/components/onedrive/services.py | 90 ++++- .../components/onedrive/services.yaml | 13 + .../components/onedrive/strings.json | 26 +- tests/components/onedrive/test_services.py | 314 +++++++++++++++++- 5 files changed, 431 insertions(+), 15 deletions(-) diff --git a/homeassistant/components/onedrive/icons.json b/homeassistant/components/onedrive/icons.json index 66f510b8e82..a8933728ae0 100644 --- a/homeassistant/components/onedrive/icons.json +++ b/homeassistant/components/onedrive/icons.json @@ -22,6 +22,9 @@ } }, "services": { + "delete": { + "service": "mdi:cloud-remove" + }, "upload": { "service": "mdi:cloud-upload" } diff --git a/homeassistant/components/onedrive/services.py b/homeassistant/components/onedrive/services.py index d88cf185b39..1693454f386 100644 --- a/homeassistant/components/onedrive/services.py +++ b/homeassistant/components/onedrive/services.py @@ -2,7 +2,7 @@ import asyncio from dataclasses import asdict -from pathlib import Path +from pathlib import Path, PurePosixPath from typing import cast from onedrive_personal_sdk.exceptions import OneDriveException @@ -19,11 +19,12 @@ from homeassistant.core import ( from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers import config_validation as cv, service -from .const import DOMAIN +from .const import CONF_DELETE_PERMANENTLY, DOMAIN from .coordinator import OneDriveConfigEntry CONF_CONFIG_ENTRY_ID = "config_entry_id" CONF_DESTINATION_FOLDER = "destination_folder" +CONF_DESTINATION_PATH = "destination_path" UPLOAD_SERVICE = "upload" UPLOAD_SERVICE_SCHEMA = vol.Schema( @@ -33,6 +34,17 @@ UPLOAD_SERVICE_SCHEMA = vol.Schema( vol.Required(CONF_DESTINATION_FOLDER): cv.string, } ) + +DELETE_SERVICE = "delete" +DELETE_SERVICE_SCHEMA = vol.Schema( + { + vol.Required(CONF_CONFIG_ENTRY_ID): cv.string, + vol.Required(CONF_DESTINATION_PATH): vol.All( + cv.ensure_list, vol.Length(min=1), [cv.string] + ), + } +) + CONTENT_SIZE_LIMIT = 250 * 1024 * 1024 @@ -76,6 +88,29 @@ def _read_file_contents( return results +def _raise_invalid_destination_path(destination_path: str) -> None: + raise HomeAssistantError( + translation_domain=DOMAIN, + translation_key="invalid_destination_path", + translation_placeholders={"destination_path": destination_path}, + ) + + +def _validate_destination_path(destination_path: str) -> str: + """Validate and normalize a remote destination path. + + Returns the normalized path or raises HomeAssistantError. + """ + normalized = destination_path.strip("/") + if not normalized: + _raise_invalid_destination_path(destination_path) + parts = PurePosixPath(normalized).parts + for part in parts: + if part == ".." or ":" in part: + _raise_invalid_destination_path(destination_path) + return str(PurePosixPath(normalized)) + + @callback def async_setup_services(hass: HomeAssistant) -> None: """Register OneDrive services.""" @@ -122,6 +157,50 @@ def async_setup_services(hass: HomeAssistant) -> None: return {"files": [asdict(item_result) for item_result in upload_results]} return None + async def async_handle_delete(call: ServiceCall) -> None: + """Delete one or more files from OneDrive.""" + config_entry: OneDriveConfigEntry = service.async_get_config_entry( + hass, DOMAIN, call.data[CONF_CONFIG_ENTRY_ID] + ) + client = config_entry.runtime_data.client + delete_permanently = config_entry.options.get(CONF_DELETE_PERMANENTLY, False) + file_paths = [ + _validate_destination_path(p) + for p in cast(list[str], call.data[CONF_DESTINATION_PATH]) + ] + + try: + approot_id = (await client.get_approot()).id + except OneDriveException as err: + raise HomeAssistantError( + translation_domain=DOMAIN, + translation_key="connection_error", + ) from err + + results = await asyncio.gather( + *[ + client.delete_drive_item( + f"{approot_id}:/{file_path}:", delete_permanently + ) + for file_path in file_paths + ], + return_exceptions=True, + ) + failures: list[tuple[str, OneDriveException]] = [] + for file_path, result in zip(file_paths, results, strict=True): + if isinstance(result, OneDriveException): + failures.append((file_path, result)) + if failures: + raise HomeAssistantError( + translation_domain=DOMAIN, + translation_key="delete_error", + translation_placeholders={ + "paths": ", ".join(f"`{path}`" for path, _ in failures) + }, + ) from ExceptionGroup( + "OneDrive delete errors", [err for _, err in failures] + ) + hass.services.async_register( DOMAIN, UPLOAD_SERVICE, @@ -130,3 +209,10 @@ def async_setup_services(hass: HomeAssistant) -> None: supports_response=SupportsResponse.OPTIONAL, description_placeholders={"example_image_path": "/config/www/image.jpg"}, ) + + hass.services.async_register( + DOMAIN, + DELETE_SERVICE, + async_handle_delete, + schema=DELETE_SERVICE_SCHEMA, + ) diff --git a/homeassistant/components/onedrive/services.yaml b/homeassistant/components/onedrive/services.yaml index fdc934ef9b2..d39968d74f0 100644 --- a/homeassistant/components/onedrive/services.yaml +++ b/homeassistant/components/onedrive/services.yaml @@ -14,3 +14,16 @@ upload: required: true selector: text: + +delete: + fields: + config_entry_id: + required: true + selector: + config_entry: + integration: onedrive + destination_path: + required: true + selector: + text: + multiple: true diff --git a/homeassistant/components/onedrive/strings.json b/homeassistant/components/onedrive/strings.json index 1f1aa5ec389..5ba210929b0 100644 --- a/homeassistant/components/onedrive/strings.json +++ b/homeassistant/components/onedrive/strings.json @@ -90,9 +90,15 @@ "authentication_failed": { "message": "Authentication failed" }, + "connection_error": { + "message": "[%key:component::onedrive::config::abort::connection_error%]" + }, "create_folder_error": { "message": "Failed to create folder: {message}" }, + "delete_error": { + "message": "Failed to delete from OneDrive: {paths}" + }, "failed_to_get_folder": { "message": "Failed to get {folder} folder" }, @@ -105,6 +111,9 @@ "filenames_do_not_exist": { "message": "The following files do not exist: {filenames}" }, + "invalid_destination_path": { + "message": "Invalid destination path `{destination_path}`: must be non-empty, must not contain `:` or `..` path segments" + }, "no_access_to_path": { "message": "Cannot read {filename}, no access to path; `allowlist_external_dirs` may need to be adjusted in `configuration.yaml`" }, @@ -142,6 +151,21 @@ } }, "services": { + "delete": { + "description": "Deletes one or more files from OneDrive.", + "fields": { + "config_entry_id": { + "description": "The config entry representing the OneDrive you want to delete from.", + "name": "Config entry ID" + }, + "destination_path": { + "description": "One or more paths to files inside the OneDrive app folder (Apps/Home Assistant) to delete.", + "example": "[\"photos/snapshots/image.jpg\", \"photos/snapshots/image2.jpg\"]", + "name": "Destination paths" + } + }, + "name": "Delete files" + }, "upload": { "description": "Uploads one or more files to OneDrive.", "fields": { @@ -150,7 +174,7 @@ "name": "Config entry ID" }, "destination_folder": { - "description": "Folder inside the Home Assistant app folder (Apps/Home Assistant) you want to upload the files to. Will be created if it does not exist.", + "description": "Folder inside the OneDrive app folder (Apps/Home Assistant) you want to upload the files to. Will be created if it does not exist.", "example": "photos/snapshots", "name": "Destination folder" }, diff --git a/tests/components/onedrive/test_services.py b/tests/components/onedrive/test_services.py index cb0bb8721ed..5e40ea90a46 100644 --- a/tests/components/onedrive/test_services.py +++ b/tests/components/onedrive/test_services.py @@ -8,11 +8,14 @@ from unittest.mock import MagicMock, Mock, patch from onedrive_personal_sdk.exceptions import OneDriveException import pytest +import voluptuous as vol -from homeassistant.components.onedrive.const import DOMAIN +from homeassistant.components.onedrive.const import CONF_DELETE_PERMANENTLY, DOMAIN from homeassistant.components.onedrive.services import ( CONF_CONFIG_ENTRY_ID, CONF_DESTINATION_FOLDER, + CONF_DESTINATION_PATH, + DELETE_SERVICE, UPLOAD_SERVICE, ) from homeassistant.config_entries import ConfigEntryState @@ -25,7 +28,8 @@ from . import setup_integration from tests.common import MockConfigEntry TEST_FILENAME = "doorbell_snapshot.jpg" -DESINATION_FOLDER = "TestFolder" +TEST_DESTINATION_PATH = "photos/snapshots/image.jpg" +DESTINATION_FOLDER = "TestFolder" @dataclass @@ -83,7 +87,7 @@ async def test_upload_service( { CONF_CONFIG_ENTRY_ID: mock_config_entry.entry_id, CONF_FILENAME: TEST_FILENAME, - CONF_DESTINATION_FOLDER: DESINATION_FOLDER, + CONF_DESTINATION_FOLDER: DESTINATION_FOLDER, }, blocking=True, return_response=True, @@ -109,7 +113,7 @@ async def test_upload_service_no_response( { CONF_CONFIG_ENTRY_ID: mock_config_entry.entry_id, CONF_FILENAME: TEST_FILENAME, - CONF_DESTINATION_FOLDER: DESINATION_FOLDER, + CONF_DESTINATION_FOLDER: DESTINATION_FOLDER, }, blocking=True, ) @@ -130,7 +134,7 @@ async def test_upload_service_config_entry_not_found( { CONF_CONFIG_ENTRY_ID: "invalid-config-entry-id", CONF_FILENAME: TEST_FILENAME, - CONF_DESTINATION_FOLDER: DESINATION_FOLDER, + CONF_DESTINATION_FOLDER: DESTINATION_FOLDER, }, blocking=True, return_response=True, @@ -156,7 +160,7 @@ async def test_config_entry_not_loaded( { CONF_CONFIG_ENTRY_ID: mock_config_entry.entry_id, CONF_FILENAME: TEST_FILENAME, - CONF_DESTINATION_FOLDER: DESINATION_FOLDER, + CONF_DESTINATION_FOLDER: DESTINATION_FOLDER, }, blocking=True, return_response=True, @@ -180,7 +184,7 @@ async def test_path_is_not_allowed( { CONF_CONFIG_ENTRY_ID: mock_config_entry.entry_id, CONF_FILENAME: TEST_FILENAME, - CONF_DESTINATION_FOLDER: DESINATION_FOLDER, + CONF_DESTINATION_FOLDER: DESTINATION_FOLDER, }, blocking=True, return_response=True, @@ -201,7 +205,7 @@ async def test_filename_does_not_exist( { CONF_CONFIG_ENTRY_ID: mock_config_entry.entry_id, CONF_FILENAME: TEST_FILENAME, - CONF_DESTINATION_FOLDER: DESINATION_FOLDER, + CONF_DESTINATION_FOLDER: DESTINATION_FOLDER, }, blocking=True, return_response=True, @@ -225,7 +229,7 @@ async def test_multiple_filenames_do_not_exist( { CONF_CONFIG_ENTRY_ID: mock_config_entry.entry_id, CONF_FILENAME: [TEST_FILENAME, second_filename], - CONF_DESTINATION_FOLDER: DESINATION_FOLDER, + CONF_DESTINATION_FOLDER: DESTINATION_FOLDER, }, blocking=True, return_response=True, @@ -251,7 +255,7 @@ async def test_upload_service_fails_upload( { CONF_CONFIG_ENTRY_ID: mock_config_entry.entry_id, CONF_FILENAME: TEST_FILENAME, - CONF_DESTINATION_FOLDER: DESINATION_FOLDER, + CONF_DESTINATION_FOLDER: DESTINATION_FOLDER, }, blocking=True, return_response=True, @@ -275,7 +279,7 @@ async def test_upload_size_limit( { CONF_CONFIG_ENTRY_ID: mock_config_entry.entry_id, CONF_FILENAME: TEST_FILENAME, - CONF_DESTINATION_FOLDER: DESINATION_FOLDER, + CONF_DESTINATION_FOLDER: DESTINATION_FOLDER, }, blocking=True, return_response=True, @@ -300,8 +304,294 @@ async def test_create_album_failed( { CONF_CONFIG_ENTRY_ID: mock_config_entry.entry_id, CONF_FILENAME: TEST_FILENAME, - CONF_DESTINATION_FOLDER: DESINATION_FOLDER, + CONF_DESTINATION_FOLDER: DESTINATION_FOLDER, }, blocking=True, return_response=True, ) + + +async def test_delete_service_config_entry_not_found( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, +) -> None: + """Test delete service call with a config entry that does not exist.""" + await setup_integration(hass, mock_config_entry) + with pytest.raises(ServiceValidationError) as err: + await hass.services.async_call( + DOMAIN, + DELETE_SERVICE, + { + CONF_CONFIG_ENTRY_ID: "invalid-config-entry-id", + CONF_DESTINATION_PATH: [TEST_DESTINATION_PATH], + }, + blocking=True, + ) + assert err.value.translation_key == "service_config_entry_not_found" + + +async def test_delete_service_config_entry_not_loaded( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, +) -> None: + """Test delete service call with a config entry that is not loaded.""" + await setup_integration(hass, mock_config_entry) + await hass.config_entries.async_unload(mock_config_entry.entry_id) + await hass.async_block_till_done() + + assert mock_config_entry.state is ConfigEntryState.NOT_LOADED + + with pytest.raises(ServiceValidationError) as err: + await hass.services.async_call( + DOMAIN, + DELETE_SERVICE, + { + CONF_CONFIG_ENTRY_ID: mock_config_entry.entry_id, + CONF_DESTINATION_PATH: [TEST_DESTINATION_PATH], + }, + blocking=True, + ) + assert err.value.translation_key == "service_config_entry_not_loaded" + + +async def test_delete_service( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_onedrive_client: MagicMock, +) -> None: + """Test delete service call removes the remote file.""" + await setup_integration(hass, mock_config_entry) + + assert hass.services.has_service(DOMAIN, DELETE_SERVICE) + + await hass.services.async_call( + DOMAIN, + DELETE_SERVICE, + { + CONF_CONFIG_ENTRY_ID: mock_config_entry.entry_id, + CONF_DESTINATION_PATH: [TEST_DESTINATION_PATH], + }, + blocking=True, + ) + + mock_onedrive_client.delete_drive_item.assert_called_once() + call_args = mock_onedrive_client.delete_drive_item.call_args + assert call_args.args[0] == f"id:/{TEST_DESTINATION_PATH}:" + assert call_args.args[1] is False + + +async def test_delete_service_delete_permanently( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_onedrive_client: MagicMock, +) -> None: + """Test delete service passes delete_permanently=True when option is set.""" + await setup_integration(hass, mock_config_entry) + hass.config_entries.async_update_entry( + mock_config_entry, options={CONF_DELETE_PERMANENTLY: True} + ) + + await hass.services.async_call( + DOMAIN, + DELETE_SERVICE, + { + CONF_CONFIG_ENTRY_ID: mock_config_entry.entry_id, + CONF_DESTINATION_PATH: [TEST_DESTINATION_PATH], + }, + blocking=True, + ) + + call_args = mock_onedrive_client.delete_drive_item.call_args + assert call_args.args[0] == f"id:/{TEST_DESTINATION_PATH}:" + assert call_args.args[1] is True + + +async def test_delete_service_multiple_files( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_onedrive_client: MagicMock, +) -> None: + """Test delete service removes multiple remote files in parallel.""" + await setup_integration(hass, mock_config_entry) + second_path = "photos/snapshots/image2.jpg" + + await hass.services.async_call( + DOMAIN, + DELETE_SERVICE, + { + CONF_CONFIG_ENTRY_ID: mock_config_entry.entry_id, + CONF_DESTINATION_PATH: [TEST_DESTINATION_PATH, second_path], + }, + blocking=True, + ) + + assert mock_onedrive_client.delete_drive_item.call_count == 2 + called_paths = { + c.args[0] for c in mock_onedrive_client.delete_drive_item.call_args_list + } + assert called_paths == { + f"id:/{TEST_DESTINATION_PATH}:", + f"id:/{second_path}:", + } + + +async def test_delete_service_fails( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_onedrive_client: MagicMock, +) -> None: + """Test delete service raises HomeAssistantError on OneDriveException.""" + await setup_integration(hass, mock_config_entry) + mock_onedrive_client.delete_drive_item.side_effect = OneDriveException("api error") + + with pytest.raises(HomeAssistantError) as exc_info: + await hass.services.async_call( + DOMAIN, + DELETE_SERVICE, + { + CONF_CONFIG_ENTRY_ID: mock_config_entry.entry_id, + CONF_DESTINATION_PATH: [TEST_DESTINATION_PATH], + }, + blocking=True, + ) + assert exc_info.value.translation_key == "delete_error" + assert TEST_DESTINATION_PATH in exc_info.value.translation_placeholders["paths"] + + +async def test_delete_service_multiple_files_all_fail( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_onedrive_client: MagicMock, +) -> None: + """Test delete service aggregates errors from multiple failed deletions.""" + await setup_integration(hass, mock_config_entry) + second_path = "photos/snapshots/image2.jpg" + mock_onedrive_client.delete_drive_item.side_effect = [ + OneDriveException("error one"), + OneDriveException("error two"), + ] + + with pytest.raises(HomeAssistantError) as exc_info: + await hass.services.async_call( + DOMAIN, + DELETE_SERVICE, + { + CONF_CONFIG_ENTRY_ID: mock_config_entry.entry_id, + CONF_DESTINATION_PATH: [TEST_DESTINATION_PATH, second_path], + }, + blocking=True, + ) + + assert mock_onedrive_client.delete_drive_item.call_count == 2 + assert isinstance(exc_info.value.__cause__, ExceptionGroup) + assert len(exc_info.value.__cause__.exceptions) == 2 + assert TEST_DESTINATION_PATH in exc_info.value.translation_placeholders["paths"] + assert second_path in exc_info.value.translation_placeholders["paths"] + + +async def test_delete_service_multiple_files_partial_failure( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_onedrive_client: MagicMock, +) -> None: + """Test delete service attempts all deletions before raising on partial failure.""" + await setup_integration(hass, mock_config_entry) + second_path = "photos/snapshots/image2.jpg" + mock_onedrive_client.delete_drive_item.side_effect = [ + None, + OneDriveException("error two"), + ] + + with pytest.raises(HomeAssistantError) as exc_info: + await hass.services.async_call( + DOMAIN, + DELETE_SERVICE, + { + CONF_CONFIG_ENTRY_ID: mock_config_entry.entry_id, + CONF_DESTINATION_PATH: [TEST_DESTINATION_PATH, second_path], + }, + blocking=True, + ) + + assert mock_onedrive_client.delete_drive_item.call_count == 2 + called_paths = { + c.args[0] for c in mock_onedrive_client.delete_drive_item.call_args_list + } + assert called_paths == { + f"id:/{TEST_DESTINATION_PATH}:", + f"id:/{second_path}:", + } + assert exc_info.value.translation_key == "delete_error" + assert second_path in exc_info.value.translation_placeholders["paths"] + + +async def test_delete_service_get_approot_fails( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_onedrive_client: MagicMock, +) -> None: + """Test delete service raises HomeAssistantError when get_approot fails.""" + await setup_integration(hass, mock_config_entry) + mock_onedrive_client.get_approot.side_effect = OneDriveException("network error") + + with pytest.raises(HomeAssistantError) as exc_info: + await hass.services.async_call( + DOMAIN, + DELETE_SERVICE, + { + CONF_CONFIG_ENTRY_ID: mock_config_entry.entry_id, + CONF_DESTINATION_PATH: [TEST_DESTINATION_PATH], + }, + blocking=True, + ) + assert exc_info.value.translation_key == "connection_error" + + +async def test_delete_empty_destination_path( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, +) -> None: + """Test delete service raises when destination_path is an empty list.""" + await setup_integration(hass, mock_config_entry) + + with pytest.raises(vol.Invalid): + await hass.services.async_call( + DOMAIN, + DELETE_SERVICE, + { + CONF_CONFIG_ENTRY_ID: mock_config_entry.entry_id, + CONF_DESTINATION_PATH: [], + }, + blocking=True, + ) + + +@pytest.mark.parametrize( + "bad_path", + [ + "", + "/", + "//", + "photos/../secrets", + "photos/file:name.jpg", + "../escape", + ], +) +async def test_delete_invalid_destination_path( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + bad_path: str, +) -> None: + """Test delete service raises HomeAssistantError for invalid destination paths.""" + await setup_integration(hass, mock_config_entry) + + with pytest.raises(HomeAssistantError, match="Invalid destination path"): + await hass.services.async_call( + DOMAIN, + DELETE_SERVICE, + { + CONF_CONFIG_ENTRY_ID: mock_config_entry.entry_id, + CONF_DESTINATION_PATH: bad_path, + }, + blocking=True, + )