From 82fb7999cba6cf2850ada077ed352ffd07d8b19c Mon Sep 17 00:00:00 2001 From: Franck Nijhof Date: Sat, 29 Aug 2026 14:15:46 +0200 Subject: [PATCH] Refresh the Alexa to-do list after writing to it (#180598) --- .../components/alexa_devices/coordinator.py | 56 ++-- .../components/alexa_devices/todo.py | 114 ++++---- tests/components/alexa_devices/test_todo.py | 250 +++++++++++++++++- 3 files changed, 353 insertions(+), 67 deletions(-) diff --git a/homeassistant/components/alexa_devices/coordinator.py b/homeassistant/components/alexa_devices/coordinator.py index 3430c2a5abc4..faf557f3c740 100644 --- a/homeassistant/components/alexa_devices/coordinator.py +++ b/homeassistant/components/alexa_devices/coordinator.py @@ -1,5 +1,6 @@ """Support for Alexa Devices.""" +from asyncio import Lock from collections.abc import AsyncGenerator from contextlib import asynccontextmanager from datetime import timedelta @@ -167,6 +168,7 @@ class AmazonDevicesCoordinator(DataUpdateCoordinator[dict[str, AmazonDevice]]): } self._todo_list_items: dict[str, dict[str, AmazonListItem]] = {} + self._todo_refresh_lock = Lock() self.api.on_todo_event.append(self.todo_event_handler) self.api.on_todo_event.freeze() @@ -308,23 +310,45 @@ class AmazonDevicesCoordinator(DataUpdateCoordinator[dict[str, AmazonDevice]]): todo_list.id ] = await self.api.get_todo_list_items(todo_list.id) - async def todo_event_handler(self, list_event: AmazonListEvent) -> None: - """Handle changes on To-Do lists.""" - if list_event.type == AmazonListEventType.DELETED: - self._todo_list_items.get(list_event.list_id, {}).pop( - list_event.item_id, None - ) - elif ( - list_event.type - in (AmazonListEventType.UPDATED, AmazonListEventType.CREATED) - ) and list_event.items: - if list_event.list_id not in self._todo_list_items: - # List was newly created after initial sync - self._todo_list_items[list_event.list_id] = {} + async def refresh_todo_list_items(self, list_id: str) -> None: + """Refresh the cached items of a single to-do list. - self._todo_list_items[list_event.list_id][list_event.item_id] = ( - list_event.items - ) + Cached items are otherwise only filled by the initial sync and by + pushed events, so a write of our own needs a pull to become visible. + + The pulls are serialized, as an older answer landing last would leave + the cache behind with nothing to repair it. + """ + async with self._todo_refresh_lock, alexa_api_call(self): + self._todo_list_items[list_id] = await self.api.get_todo_list_items(list_id) + + # Reading the list back proves the API answers again + self.last_update_success = True + + self.async_update_listeners() + + async def todo_event_handler(self, list_event: AmazonListEvent) -> None: + """Handle changes on To-Do lists. + + Takes the refresh lock, so an event arriving while a list is being + read back is applied on top of that read instead of under it. + """ + async with self._todo_refresh_lock: + if list_event.type == AmazonListEventType.DELETED: + self._todo_list_items.get(list_event.list_id, {}).pop( + list_event.item_id, None + ) + elif ( + list_event.type + in (AmazonListEventType.UPDATED, AmazonListEventType.CREATED) + ) and list_event.items: + if list_event.list_id not in self._todo_list_items: + # List was newly created after initial sync + self._todo_list_items[list_event.list_id] = {} + + self._todo_list_items[list_event.list_id][list_event.item_id] = ( + list_event.items + ) self.async_update_listeners() diff --git a/homeassistant/components/alexa_devices/todo.py b/homeassistant/components/alexa_devices/todo.py index bd5d05219c7b..fa145d1232e4 100644 --- a/homeassistant/components/alexa_devices/todo.py +++ b/homeassistant/components/alexa_devices/todo.py @@ -126,6 +126,8 @@ class AlexaToDoList(AmazonServiceEntity, TodoListEntity): self._list.name, ) + await self.coordinator.refresh_todo_list_items(self._list.id) + @override async def async_delete_todo_items(self, uids: list[str]) -> None: """Delete items from the to-do list.""" @@ -133,25 +135,29 @@ class AlexaToDoList(AmazonServiceEntity, TodoListEntity): list_items_lookup = self.coordinator.todo_list_items[self._list.id] - for uid in uids: - existing_item = list_items_lookup[uid] + try: + for uid in uids: + existing_item = list_items_lookup[uid] - LOGGER.debug( - "Deleting item %s (ID: %s) with version %s", - existing_item.name, - uid, - existing_item.version, - ) - async with alexa_api_call(self.coordinator): - await self.coordinator.api.delete_todo_list_item( - self._list.id, uid, existing_item.version + LOGGER.debug( + "Deleting item %s (ID: %s) with version %s", + existing_item.name, + uid, + existing_item.version, ) - LOGGER.debug( - "Successfully deleted item %s (ID: %s) with version %s", - existing_item.name, - uid, - existing_item.version, - ) + async with alexa_api_call(self.coordinator): + await self.coordinator.api.delete_todo_list_item( + self._list.id, uid, existing_item.version + ) + LOGGER.debug( + "Successfully deleted item %s (ID: %s) with version %s", + existing_item.name, + uid, + existing_item.version, + ) + finally: + # A later delete can fail after an earlier one went through + await self.coordinator.refresh_todo_list_items(self._list.id) @override async def async_update_todo_item(self, item: TodoItem) -> None: @@ -166,39 +172,53 @@ class AlexaToDoList(AmazonServiceEntity, TodoListEntity): existing_item = list_items_lookup[item.uid] - if has_completed_changed := ( + has_completed_changed = ( existing_item.status == AmazonListItemStatus.COMPLETE - ) != (item.status == TodoItemStatus.COMPLETED): - # Update the checked status - LOGGER.debug( - "Updating item %s with checked status %s", item.uid, item.status - ) + ) != (item.status == TodoItemStatus.COMPLETED) + has_renamed = existing_item.name != item.summary - async with alexa_api_call(self.coordinator): - await self.coordinator.api.set_todo_list_item_checked_status( - self._list.id, + if not has_completed_changed and not has_renamed: + return + + try: + if has_completed_changed: + # Update the checked status + LOGGER.debug( + "Updating item %s with checked status %s", item.uid, item.status + ) + + async with alexa_api_call(self.coordinator): + await self.coordinator.api.set_todo_list_item_checked_status( + self._list.id, + item.uid, + item.status == TodoItemStatus.COMPLETED, + existing_item.version, + ) + + LOGGER.debug( + "Successfully updated item %s with checked status %s", item.uid, - item.status == TodoItemStatus.COMPLETED, - existing_item.version, + item.status, ) - LOGGER.debug( - "Successfully updated item %s with checked status %s", - item.uid, - item.status, - ) - - if existing_item.name != item.summary: - # Name has changed, update it - LOGGER.debug("Updating item %s with new name %s", item.uid, item.summary) - - # If both have changed -> Increase item version by 1 - version = existing_item.version + int(has_completed_changed) - - async with alexa_api_call(self.coordinator): - await self.coordinator.api.rename_todo_list_item( - self._list.id, item.uid, item.summary, version + if has_renamed: + # Name has changed, update it + LOGGER.debug( + "Updating item %s with new name %s", item.uid, item.summary ) - LOGGER.debug( - "Successfully updated item %s with new name %s", item.uid, item.summary - ) + + # If both have changed -> Increase item version by 1 + version = existing_item.version + int(has_completed_changed) + + async with alexa_api_call(self.coordinator): + await self.coordinator.api.rename_todo_list_item( + self._list.id, item.uid, item.summary, version + ) + LOGGER.debug( + "Successfully updated item %s with new name %s", + item.uid, + item.summary, + ) + finally: + # A rename can fail after the status change went through + await self.coordinator.refresh_todo_list_items(self._list.id) diff --git a/tests/components/alexa_devices/test_todo.py b/tests/components/alexa_devices/test_todo.py index c751975cd765..c3d496d9833c 100644 --- a/tests/components/alexa_devices/test_todo.py +++ b/tests/components/alexa_devices/test_todo.py @@ -1,5 +1,7 @@ """Test Alexa Devices todo entities.""" +import asyncio +from dataclasses import replace from typing import Any from unittest.mock import AsyncMock, patch @@ -26,6 +28,7 @@ from homeassistant.components.todo import ( from homeassistant.config_entries import ConfigEntryState from homeassistant.const import ATTR_ENTITY_ID, Platform from homeassistant.core import HomeAssistant +from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers import entity_registry as er from homeassistant.util import slugify @@ -115,7 +118,7 @@ async def test_all_entities( """Test all entities.""" mock_amazon_devices_client.todo_lists = mock_todo_lists mock_amazon_devices_client.get_todo_list_items = AsyncMock( - side_effect=lambda list_id: mock_todo_items.get(list_id, {}) + side_effect=lambda list_id: dict(mock_todo_items.get(list_id, {})) ) with patch("homeassistant.components.alexa_devices.PLATFORMS", [Platform.TODO]): @@ -132,12 +135,31 @@ async def test_add_todo_item( ) -> None: """Test adding a todo item.""" mock_amazon_devices_client.todo_lists = mock_todo_lists - mock_amazon_devices_client.get_todo_list_items = AsyncMock(return_value={}) + list_items: dict[str, AmazonListItem] = {} + mock_amazon_devices_client.get_todo_list_items = AsyncMock( + side_effect=lambda list_id: dict(list_items) + ) await setup_integration(hass, mock_config_entry) entity_id = MOCK_TODO_LIST_ENTITY_ID + assert hass.states.get(entity_id).state == "0" + + # Amazon has the item from the moment the call returns + mock_amazon_devices_client.add_todo_list_item = AsyncMock( + side_effect=lambda list_id, name: list_items.update( + { + "item_6": AmazonListItem( + id="item_6", + name=name, + status=AmazonListItemStatus.ACTIVE, + version=1, + ) + } + ) + ) + await hass.services.async_call( TODO_DOMAIN, TodoServices.ADD_ITEM, @@ -148,6 +170,132 @@ async def test_add_todo_item( mock_amazon_devices_client.add_todo_list_item.assert_called_once_with( "todo_list_id", "New Task" ) + assert hass.states.get(entity_id).state == "1" + + +async def test_concurrent_writes_keep_the_newest_answer( + hass: HomeAssistant, + mock_amazon_devices_client: AsyncMock, + mock_config_entry: MockConfigEntry, + mock_todo_lists: list[AmazonListInfo], +) -> None: + """Test a slow read of an older list does not overwrite a newer one.""" + mock_amazon_devices_client.todo_lists = mock_todo_lists + list_items: dict[str, AmazonListItem] = {} + mock_amazon_devices_client.get_todo_list_items = AsyncMock( + side_effect=lambda list_id: dict(list_items) + ) + + await setup_integration(hass, mock_config_entry) + + entity_id = MOCK_TODO_LIST_ENTITY_ID + + def add_item(list_id: str, name: str) -> None: + list_items[name] = AmazonListItem( + id=name, name=name, status=AmazonListItemStatus.ACTIVE, version=1 + ) + + mock_amazon_devices_client.add_todo_list_item = AsyncMock(side_effect=add_item) + + # Hold the first read until both items have been written + released = asyncio.Event() + reads = 0 + + async def read_items(list_id: str) -> dict[str, AmazonListItem]: + nonlocal reads + reads += 1 + items = dict(list_items) + if reads == 1: + await released.wait() + return items + + mock_amazon_devices_client.get_todo_list_items = AsyncMock(side_effect=read_items) + + writes = asyncio.gather( + *[ + hass.services.async_call( + TODO_DOMAIN, + TodoServices.ADD_ITEM, + {ATTR_ENTITY_ID: entity_id, "item": item}, + blocking=True, + ) + for item in ("First task", "Second task") + ] + ) + await asyncio.sleep(0) + released.set() + await writes + + assert hass.states.get(entity_id).state == "2" + + +async def test_pushed_event_survives_a_refresh( + hass: HomeAssistant, + mock_amazon_devices_client: AsyncMock, + mock_config_entry: MockConfigEntry, + mock_todo_lists: list[AmazonListInfo], +) -> None: + """Test an event arriving during a read back is not lost by that read.""" + mock_amazon_devices_client.todo_lists = mock_todo_lists + list_items: dict[str, AmazonListItem] = {} + mock_amazon_devices_client.get_todo_list_items = AsyncMock( + side_effect=lambda list_id: dict(list_items) + ) + + await setup_integration(hass, mock_config_entry) + coordinator = mock_config_entry.runtime_data + + entity_id = MOCK_TODO_LIST_ENTITY_ID + + def add_item(list_id: str, name: str) -> None: + list_items[name] = AmazonListItem( + id=name, name=name, status=AmazonListItemStatus.ACTIVE, version=1 + ) + + mock_amazon_devices_client.add_todo_list_item = AsyncMock(side_effect=add_item) + + # Hold the read back until the event has been handled + released = asyncio.Event() + + async def read_items(list_id: str) -> dict[str, AmazonListItem]: + items = dict(list_items) + await released.wait() + return items + + mock_amazon_devices_client.get_todo_list_items = AsyncMock(side_effect=read_items) + + write = asyncio.create_task( + hass.services.async_call( + TODO_DOMAIN, + TodoServices.ADD_ITEM, + {ATTR_ENTITY_ID: entity_id, "item": "Written task"}, + blocking=True, + ) + ) + await asyncio.sleep(0) + + # Alexa reports an item of its own while the read back is in flight + pushed = asyncio.create_task( + coordinator.todo_event_handler( + AmazonListEvent( + list_id="todo_list_id", + item_id="item_6", + type=AmazonListEventType.CREATED, + items=AmazonListItem( + id="item_6", + name="Spoken task", + status=AmazonListItemStatus.ACTIVE, + version=1, + ), + ) + ) + ) + await asyncio.sleep(0) + released.set() + await write + await pushed + + assert hass.states.get(entity_id).state == "2" async def test_delete_todo_item( @@ -160,13 +308,22 @@ async def test_delete_todo_item( """Test deleting a todo item.""" mock_amazon_devices_client.todo_lists = mock_todo_lists mock_amazon_devices_client.get_todo_list_items = AsyncMock( - side_effect=lambda list_id: mock_todo_items.get(list_id, {}) + side_effect=lambda list_id: dict(mock_todo_items.get(list_id, {})) ) await setup_integration(hass, mock_config_entry) entity_id = MOCK_TODO_LIST_ENTITY_ID + assert hass.states.get(entity_id).state == "1" + + # Amazon has dropped the item from the moment the call returns + mock_amazon_devices_client.delete_todo_list_item = AsyncMock( + side_effect=lambda list_id, item_id, version: mock_todo_items[list_id].pop( + item_id + ) + ) + # Delete item_2 await hass.services.async_call( TODO_DOMAIN, @@ -178,6 +335,48 @@ async def test_delete_todo_item( mock_amazon_devices_client.delete_todo_list_item.assert_called_once_with( "todo_list_id", "item_2", 1 ) + assert hass.states.get(entity_id).state == "0" + + +async def test_delete_todo_items_partial_failure( + hass: HomeAssistant, + mock_amazon_devices_client: AsyncMock, + mock_config_entry: MockConfigEntry, + mock_todo_lists: list[AmazonListInfo], + mock_todo_items: dict[str, Any], +) -> None: + """Test a delete that went through is not left in the cache by a later failure.""" + mock_amazon_devices_client.todo_lists = mock_todo_lists + mock_amazon_devices_client.get_todo_list_items = AsyncMock( + side_effect=lambda list_id: dict(mock_todo_items.get(list_id, {})) + ) + + await setup_integration(hass, mock_config_entry) + + entity_id = MOCK_TODO_LIST_ENTITY_ID + + assert hass.states.get(entity_id).state == "1" + + # Amazon drops item_2 and then stops answering + def delete_item(list_id: str, item_id: str, version: int) -> None: + if item_id == "item_3": + raise CannotConnect + mock_todo_items[list_id].pop(item_id) + + mock_amazon_devices_client.delete_todo_list_item = AsyncMock( + side_effect=delete_item + ) + + with pytest.raises(HomeAssistantError): + await hass.services.async_call( + TODO_DOMAIN, + TodoServices.REMOVE_ITEM, + {ATTR_ENTITY_ID: entity_id, "item": ["item_2", "item_3"]}, + blocking=True, + ) + + # Reading the list back worked, so the entity stays usable + assert hass.states.get(entity_id).state == "0" async def test_update_todo_item( @@ -190,7 +389,7 @@ async def test_update_todo_item( """Test updating a todo item.""" mock_amazon_devices_client.todo_lists = mock_todo_lists mock_amazon_devices_client.get_todo_list_items = AsyncMock( - side_effect=lambda list_id: mock_todo_items.get(list_id, {}) + side_effect=lambda list_id: dict(mock_todo_items.get(list_id, {})) ) await setup_integration(hass, mock_config_entry) @@ -254,6 +453,49 @@ async def test_update_todo_item( ) +async def test_update_todo_item_refreshes_state( + hass: HomeAssistant, + mock_amazon_devices_client: AsyncMock, + mock_config_entry: MockConfigEntry, + mock_todo_lists: list[AmazonListInfo], + mock_todo_items: dict[str, Any], +) -> None: + """Test the entity reflects an updated item once the call returns.""" + mock_amazon_devices_client.todo_lists = mock_todo_lists + mock_amazon_devices_client.get_todo_list_items = AsyncMock( + side_effect=lambda list_id: dict(mock_todo_items.get(list_id, {})) + ) + + await setup_integration(hass, mock_config_entry) + + entity_id = MOCK_TODO_LIST_ENTITY_ID + + assert hass.states.get(entity_id).state == "1" + + # Amazon has the item checked from the moment the call returns + def check_item(list_id: str, item_id: str, checked: bool, version: int) -> None: + mock_todo_items[list_id][item_id] = replace( + mock_todo_items[list_id][item_id], status=AmazonListItemStatus.COMPLETE + ) + + mock_amazon_devices_client.set_todo_list_item_checked_status = AsyncMock( + side_effect=check_item + ) + + await hass.services.async_call( + TODO_DOMAIN, + TodoServices.UPDATE_ITEM, + { + ATTR_ENTITY_ID: entity_id, + "item": "item_2", + "status": TodoItemStatus.COMPLETED, + }, + blocking=True, + ) + + assert hass.states.get(entity_id).state == "0" + + @pytest.mark.parametrize( ("initial_lists", "updated_lists"), [