Add support for deleting To-do items in Google Tasks (#102967)

* Add support for deleting To-do items in Google Tasks

* Cleanup multipart test

* Fix comments

* Add additional error checking to increase coverage

* Apply suggestions and fix tests

homeassistant/components/google_tasks/api.py

@@ -1,18 +1,36 @@
 """API for Google Tasks bound to Home Assistant OAuth."""
 
+import json
+import logging
 from typing import Any
 
 from google.oauth2.credentials import Credentials
 from googleapiclient.discovery import Resource, build
-from googleapiclient.http import HttpRequest
+from googleapiclient.errors import HttpError
+from googleapiclient.http import BatchHttpRequest, HttpRequest
 
 from homeassistant.const import CONF_ACCESS_TOKEN
 from homeassistant.core import HomeAssistant
 from homeassistant.helpers import config_entry_oauth2_flow
 
+from .exceptions import GoogleTasksApiError
+
+_LOGGER = logging.getLogger(__name__)
+
 MAX_TASK_RESULTS = 100
 
 
+def _raise_if_error(result: Any | dict[str, Any]) -> None:
+    """Raise a GoogleTasksApiError if the response contains an error."""
+    if not isinstance(result, dict):
+        raise GoogleTasksApiError(
+            f"Google Tasks API replied with unexpected response: {result}"
+        )
+    if error := result.get("error"):
+        message = error.get("message", "Unknown Error")
+        raise GoogleTasksApiError(f"Google Tasks API response: {message}")
+
+
 class AsyncConfigEntryAuth:
     """Provide Google Tasks authentication tied to an OAuth2 based config entry."""
 
@@ -40,7 +58,7 @@ class AsyncConfigEntryAuth:
         """Get all TaskList resources."""
         service = await self._get_service()
         cmd: HttpRequest = service.tasklists().list()
-        result = await self._hass.async_add_executor_job(cmd.execute)
+        result = await self._execute(cmd)
         return result["items"]
 
     async def list_tasks(self, task_list_id: str) -> list[dict[str, Any]]:
@@ -49,7 +67,7 @@ class AsyncConfigEntryAuth:
         cmd: HttpRequest = service.tasks().list(
             tasklist=task_list_id, maxResults=MAX_TASK_RESULTS
         )
-        result = await self._hass.async_add_executor_job(cmd.execute)
+        result = await self._execute(cmd)
         return result["items"]
 
     async def insert(
@@ -63,7 +81,7 @@ class AsyncConfigEntryAuth:
             tasklist=task_list_id,
             body=task,
         )
-        await self._hass.async_add_executor_job(cmd.execute)
+        await self._execute(cmd)
 
     async def patch(
         self,
@@ -78,4 +96,43 @@ class AsyncConfigEntryAuth:
             task=task_id,
             body=task,
         )
-        await self._hass.async_add_executor_job(cmd.execute)
+        await self._execute(cmd)
+
+    async def delete(
+        self,
+        task_list_id: str,
+        task_ids: list[str],
+    ) -> None:
+        """Delete a task resources."""
+        service = await self._get_service()
+        batch: BatchHttpRequest = service.new_batch_http_request()
+
+        def response_handler(_, response, exception: HttpError) -> None:
+            if exception is not None:
+                raise GoogleTasksApiError(
+                    f"Google Tasks API responded with error ({exception.status_code})"
+                ) from exception
+            data = json.loads(response)
+            _raise_if_error(data)
+
+        for task_id in task_ids:
+            batch.add(
+                service.tasks().delete(
+                    tasklist=task_list_id,
+                    task=task_id,
+                ),
+                request_id=task_id,
+                callback=response_handler,
+            )
+        await self._execute(batch)
+
+    async def _execute(self, request: HttpRequest | BatchHttpRequest) -> Any:
+        try:
+            result = await self._hass.async_add_executor_job(request.execute)
+        except HttpError as err:
+            raise GoogleTasksApiError(
+                f"Google Tasks API responded with error ({err.status_code})"
+            ) from err
+        if result:
+            _raise_if_error(result)
+        return result

homeassistant/components/google_tasks/exceptions.py

@@ -0,0 +1,7 @@
+"""Exceptions for Google Tasks api calls."""
+
+from homeassistant.exceptions import HomeAssistantError
+
+
+class GoogleTasksApiError(HomeAssistantError):
+    """Error talking to the Google Tasks API."""

homeassistant/components/google_tasks/todo.py

@@ -65,7 +65,9 @@ class GoogleTaskTodoListEntity(
 
     _attr_has_entity_name = True
     _attr_supported_features = (
-        TodoListEntityFeature.CREATE_TODO_ITEM | TodoListEntityFeature.UPDATE_TODO_ITEM
+        TodoListEntityFeature.CREATE_TODO_ITEM
+        | TodoListEntityFeature.UPDATE_TODO_ITEM
+        | TodoListEntityFeature.DELETE_TODO_ITEM
     )
 
     def __init__(
@@ -114,3 +116,8 @@ class GoogleTaskTodoListEntity(
             task=_convert_todo_item(item),
         )
         await self.coordinator.async_refresh()
+
+    async def async_delete_todo_items(self, uids: list[str]) -> None:
+        """Delete To-do items."""
+        await self.coordinator.api.delete(self._task_list_id, uids)
+        await self.coordinator.async_refresh()

tests/components/google_tasks/snapshots/test_todo.ambr

@@ -8,6 +8,12 @@
 # name: test_create_todo_list_item[api_responses0].1
   '{"title": "Soda", "status": "needsAction"}'
 # ---
+# name: test_delete_todo_list_item[_handler]
+  tuple(
+    'https://tasks.googleapis.com/batch',
+    'POST',
+  )
+# ---
 # name: test_partial_update_status[api_responses0]
   tuple(
     'https://tasks.googleapis.com/tasks/v1/lists/task-list-id-1/tasks/some-task-id?alt=json',

tests/components/google_tasks/test_todo.py

@@ -2,6 +2,7 @@
 
 
 from collections.abc import Awaitable, Callable
+from http import HTTPStatus
 import json
 from typing import Any
 from unittest.mock import Mock, patch
@@ -13,6 +14,7 @@ from syrupy.assertion import SnapshotAssertion
 from homeassistant.components.todo import DOMAIN as TODO_DOMAIN
 from homeassistant.const import Platform
 from homeassistant.core import HomeAssistant
+from homeassistant.exceptions import HomeAssistantError
 
 from tests.typing import WebSocketGenerator
 
@@ -29,12 +31,30 @@ EMPTY_RESPONSE = {}
 LIST_TASKS_RESPONSE = {
     "items": [],
 }
+ERROR_RESPONSE = {
+    "error": {
+        "code": 400,
+        "message": "Invalid task ID",
+        "errors": [
+            {"message": "Invalid task ID", "domain": "global", "reason": "invalid"}
+        ],
+    }
+}
+CONTENT_ID = "Content-ID"
+BOUNDARY = "batch_00972cc8-75bd-11ee-9692-0242ac110002"  # Arbitrary uuid
 
 LIST_TASKS_RESPONSE_WATER = {
     "items": [
         {"id": "some-task-id", "title": "Water", "status": "needsAction"},
     ],
 }
+LIST_TASKS_RESPONSE_MULTIPLE = {
+    "items": [
+        {"id": "some-task-id-1", "title": "Water", "status": "needsAction"},
+        {"id": "some-task-id-2", "title": "Milk", "status": "needsAction"},
+        {"id": "some-task-id-3", "title": "Cheese", "status": "needsAction"},
+    ],
+}
 
 
 @pytest.fixture
@@ -88,14 +108,87 @@ def mock_api_responses() -> list[dict | list]:
     return []
 
 
+def create_response_object(api_response: dict | list) -> tuple[Response, bytes]:
+    """Create an http response."""
+    return (
+        Response({"Content-Type": "application/json"}),
+        json.dumps(api_response).encode(),
+    )
+
+
+def create_batch_response_object(
+    content_ids: list[str], api_responses: list[dict | list | Response]
+) -> tuple[Response, bytes]:
+    """Create a batch response in the multipart/mixed format."""
+    assert len(api_responses) == len(content_ids)
+    content = []
+    for api_response in api_responses:
+        status = 200
+        body = ""
+        if isinstance(api_response, Response):
+            status = api_response.status
+        else:
+            body = json.dumps(api_response)
+        content.extend(
+            [
+                f"--{BOUNDARY}",
+                "Content-Type: application/http",
+                f"{CONTENT_ID}: {content_ids.pop()}",
+                "",
+                f"HTTP/1.1 {status} OK",
+                "Content-Type: application/json; charset=UTF-8",
+                "",
+                body,
+            ]
+        )
+    content.append(f"--{BOUNDARY}--")
+    body = ("\r\n".join(content)).encode()
+    return (
+        Response(
+            {
+                "Content-Type": f"multipart/mixed; boundary={BOUNDARY}",
+                "Content-ID": "1",
+            }
+        ),
+        body,
+    )
+
+
+def create_batch_response_handler(
+    api_responses: list[dict | list | Response],
+) -> Callable[[Any], tuple[Response, bytes]]:
+    """Create a fake http2lib response handler that supports generating batch responses.
+
+    Multi-part response objects are dynamically generated since they
+    need to match the Content-ID of the incoming request.
+    """
+
+    def _handler(url, method, **kwargs) -> tuple[Response, bytes]:
+        next_api_response = api_responses.pop(0)
+        if method == "POST" and (body := kwargs.get("body")):
+            content_ids = [
+                line[len(CONTENT_ID) + 2 :]
+                for line in body.splitlines()
+                if line.startswith(f"{CONTENT_ID}:")
+            ]
+            if content_ids:
+                return create_batch_response_object(content_ids, next_api_response)
+        return create_response_object(next_api_response)
+
+    return _handler
+
+
+@pytest.fixture(name="response_handler")
+def mock_response_handler(api_responses: list[dict | list]) -> list:
+    """Create a mock http2lib response handler."""
+    return [create_response_object(api_response) for api_response in api_responses]
+
+
 @pytest.fixture(autouse=True)
-def mock_http_response(api_responses: list[dict | list]) -> Mock:
+def mock_http_response(response_handler: list | Callable) -> Mock:
     """Fixture to fake out http2lib responses."""
-    responses = [
+
-        (Response({}), bytes(json.dumps(api_response), encoding="utf-8"))
+    with patch("httplib2.Http.request", side_effect=response_handler) as mock_response:
-        for api_response in api_responses
-    ]
-    with patch("httplib2.Http.request", side_effect=responses) as mock_response:
         yield mock_response
 
 
@@ -146,6 +239,29 @@ async def test_get_items(
     assert state.state == "1"
 
 
+@pytest.mark.parametrize(
+    "response_handler",
+    [
+        ([(Response({"status": HTTPStatus.INTERNAL_SERVER_ERROR}), b"")]),
+    ],
+)
+async def test_list_items_server_error(
+    hass: HomeAssistant,
+    setup_credentials: None,
+    integration_setup: Callable[[], Awaitable[bool]],
+    hass_ws_client: WebSocketGenerator,
+    ws_get_items: Callable[[], Awaitable[dict[str, str]]],
+) -> None:
+    """Test an error returned by the server when setting up the platform."""
+
+    assert await integration_setup()
+
+    await hass_ws_client(hass)
+
+    state = hass.states.get("todo.my_tasks")
+    assert state is None
+
+
 @pytest.mark.parametrize(
     "api_responses",
     [
@@ -176,6 +292,33 @@ async def test_empty_todo_list(
     assert state.state == "0"
 
 
+@pytest.mark.parametrize(
+    "api_responses",
+    [
+        [
+            LIST_TASK_LIST_RESPONSE,
+            ERROR_RESPONSE,
+        ]
+    ],
+)
+async def test_task_items_error_response(
+    hass: HomeAssistant,
+    setup_credentials: None,
+    integration_setup: Callable[[], Awaitable[bool]],
+    hass_ws_client: WebSocketGenerator,
+    ws_get_items: Callable[[], Awaitable[dict[str, str]]],
+) -> None:
+    """Test an error while getting todo list items."""
+
+    assert await integration_setup()
+
+    await hass_ws_client(hass)
+
+    state = hass.states.get("todo.my_tasks")
+    assert state
+    assert state.state == "unavailable"
+
+
 @pytest.mark.parametrize(
     "api_responses",
     [
@@ -183,7 +326,7 @@ async def test_empty_todo_list(
             LIST_TASK_LIST_RESPONSE,
             LIST_TASKS_RESPONSE,
             EMPTY_RESPONSE,  # create
-            LIST_TASKS_RESPONSE,  # refresh after create
+            LIST_TASKS_RESPONSE,  # refresh after delete
         ]
     ],
 )
@@ -216,6 +359,41 @@ async def test_create_todo_list_item(
     assert call.kwargs.get("body") == snapshot
 
 
+@pytest.mark.parametrize(
+    "api_responses",
+    [
+        [
+            LIST_TASK_LIST_RESPONSE,
+            LIST_TASKS_RESPONSE_WATER,
+            ERROR_RESPONSE,
+        ]
+    ],
+)
+async def test_create_todo_list_item_error(
+    hass: HomeAssistant,
+    setup_credentials: None,
+    integration_setup: Callable[[], Awaitable[bool]],
+    mock_http_response: Mock,
+    snapshot: SnapshotAssertion,
+) -> None:
+    """Test for an error response when creating a To-do Item."""
+
+    assert await integration_setup()
+
+    state = hass.states.get("todo.my_tasks")
+    assert state
+    assert state.state == "1"
+
+    with pytest.raises(HomeAssistantError, match="Invalid task ID"):
+        await hass.services.async_call(
+            TODO_DOMAIN,
+            "add_item",
+            {"item": "Soda"},
+            target={"entity_id": "todo.my_tasks"},
+            blocking=True,
+        )
+
+
 @pytest.mark.parametrize(
     "api_responses",
     [
@@ -256,6 +434,41 @@ async def test_update_todo_list_item(
     assert call.kwargs.get("body") == snapshot
 
 
+@pytest.mark.parametrize(
+    "api_responses",
+    [
+        [
+            LIST_TASK_LIST_RESPONSE,
+            LIST_TASKS_RESPONSE_WATER,
+            ERROR_RESPONSE,  # update fails
+        ]
+    ],
+)
+async def test_update_todo_list_item_error(
+    hass: HomeAssistant,
+    setup_credentials: None,
+    integration_setup: Callable[[], Awaitable[bool]],
+    mock_http_response: Any,
+    snapshot: SnapshotAssertion,
+) -> None:
+    """Test for an error response when updating a To-do Item."""
+
+    assert await integration_setup()
+
+    state = hass.states.get("todo.my_tasks")
+    assert state
+    assert state.state == "1"
+
+    with pytest.raises(HomeAssistantError, match="Invalid task ID"):
+        await hass.services.async_call(
+            TODO_DOMAIN,
+            "update_item",
+            {"item": "some-task-id", "rename": "Soda", "status": "completed"},
+            target={"entity_id": "todo.my_tasks"},
+            blocking=True,
+        )
+
+
 @pytest.mark.parametrize(
     "api_responses",
     [
@@ -334,3 +547,170 @@ async def test_partial_update_status(
     assert call
     assert call.args == snapshot
     assert call.kwargs.get("body") == snapshot
+
+
+@pytest.mark.parametrize(
+    "response_handler",
+    [
+        (
+            create_batch_response_handler(
+                [
+                    LIST_TASK_LIST_RESPONSE,
+                    LIST_TASKS_RESPONSE_MULTIPLE,
+                    [EMPTY_RESPONSE, EMPTY_RESPONSE, EMPTY_RESPONSE],  # Delete batch
+                    LIST_TASKS_RESPONSE,  # refresh after create
+                ]
+            )
+        )
+    ],
+)
+async def test_delete_todo_list_item(
+    hass: HomeAssistant,
+    setup_credentials: None,
+    integration_setup: Callable[[], Awaitable[bool]],
+    mock_http_response: Any,
+    snapshot: SnapshotAssertion,
+) -> None:
+    """Test for deleting multiple To-do Items."""
+
+    assert await integration_setup()
+
+    state = hass.states.get("todo.my_tasks")
+    assert state
+    assert state.state == "3"
+
+    await hass.services.async_call(
+        TODO_DOMAIN,
+        "remove_item",
+        {"item": ["some-task-id-1", "some-task-id-2", "some-task-id-3"]},
+        target={"entity_id": "todo.my_tasks"},
+        blocking=True,
+    )
+    assert len(mock_http_response.call_args_list) == 4
+    call = mock_http_response.call_args_list[2]
+    assert call
+    assert call.args == snapshot
+
+
+@pytest.mark.parametrize(
+    "response_handler",
+    [
+        (
+            create_batch_response_handler(
+                [
+                    LIST_TASK_LIST_RESPONSE,
+                    LIST_TASKS_RESPONSE_MULTIPLE,
+                    [
+                        EMPTY_RESPONSE,
+                        ERROR_RESPONSE,  # one item is a failure
+                        EMPTY_RESPONSE,
+                    ],
+                    LIST_TASKS_RESPONSE,  # refresh after create
+                ]
+            )
+        )
+    ],
+)
+async def test_delete_partial_failure(
+    hass: HomeAssistant,
+    setup_credentials: None,
+    integration_setup: Callable[[], Awaitable[bool]],
+    mock_http_response: Any,
+    snapshot: SnapshotAssertion,
+) -> None:
+    """Test for partial failure when deleting multiple To-do Items."""
+
+    assert await integration_setup()
+
+    state = hass.states.get("todo.my_tasks")
+    assert state
+    assert state.state == "3"
+
+    with pytest.raises(HomeAssistantError, match="Invalid task ID"):
+        await hass.services.async_call(
+            TODO_DOMAIN,
+            "remove_item",
+            {"item": ["some-task-id-1", "some-task-id-2", "some-task-id-3"]},
+            target={"entity_id": "todo.my_tasks"},
+            blocking=True,
+        )
+
+
+@pytest.mark.parametrize(
+    "response_handler",
+    [
+        (
+            create_batch_response_handler(
+                [
+                    LIST_TASK_LIST_RESPONSE,
+                    LIST_TASKS_RESPONSE_MULTIPLE,
+                    [
+                        "1234-invalid-json",
+                    ],
+                ]
+            )
+        )
+    ],
+)
+async def test_delete_invalid_json_response(
+    hass: HomeAssistant,
+    setup_credentials: None,
+    integration_setup: Callable[[], Awaitable[bool]],
+    mock_http_response: Any,
+    snapshot: SnapshotAssertion,
+) -> None:
+    """Test delete with an invalid json response."""
+
+    assert await integration_setup()
+
+    state = hass.states.get("todo.my_tasks")
+    assert state
+    assert state.state == "3"
+
+    with pytest.raises(HomeAssistantError, match="unexpected response"):
+        await hass.services.async_call(
+            TODO_DOMAIN,
+            "remove_item",
+            {"item": ["some-task-id-1"]},
+            target={"entity_id": "todo.my_tasks"},
+            blocking=True,
+        )
+
+
+@pytest.mark.parametrize(
+    "response_handler",
+    [
+        (
+            create_batch_response_handler(
+                [
+                    LIST_TASK_LIST_RESPONSE,
+                    LIST_TASKS_RESPONSE_MULTIPLE,
+                    [Response({"status": HTTPStatus.INTERNAL_SERVER_ERROR})],
+                ]
+            )
+        )
+    ],
+)
+async def test_delete_server_error(
+    hass: HomeAssistant,
+    setup_credentials: None,
+    integration_setup: Callable[[], Awaitable[bool]],
+    mock_http_response: Any,
+    snapshot: SnapshotAssertion,
+) -> None:
+    """Test delete with an invalid json response."""
+
+    assert await integration_setup()
+
+    state = hass.states.get("todo.my_tasks")
+    assert state
+    assert state.state == "3"
+
+    with pytest.raises(HomeAssistantError, match="responded with error"):
+        await hass.services.async_call(
+            TODO_DOMAIN,
+            "remove_item",
+            {"item": ["some-task-id-1"]},
+            target={"entity_id": "todo.my_tasks"},
+            blocking=True,
+        )