diff --git a/custom_components/nestquest/coordinator.py b/custom_components/nestquest/coordinator.py index 7d8e79b..08f9ad9 100644 --- a/custom_components/nestquest/coordinator.py +++ b/custom_components/nestquest/coordinator.py @@ -25,6 +25,7 @@ the admin header degrades to a harmless zero instead of guessing. """ from __future__ import annotations +import asyncio import datetime from dataclasses import dataclass from zoneinfo import ZoneInfo @@ -97,7 +98,16 @@ class NestQuestSnapshot: class NestQuestCoordinator(DataUpdateCoordinator): - """Shared refresh cycle for every NestQuest entity.""" + """Shared refresh cycle for every NestQuest entity. + + Forced refreshes (the service paths pushing an immediate update + after a completion) are SERIALIZED per entry: overlapping + complete/uncomplete calls must not interleave their snapshot + passes, or an older pass could publish a stale snapshot AFTER a + newer one and leave entities wrong until the next poll. Real + HA's coordinator already serializes internally; the lock keeps + the same guarantee on the stand-in and costs nothing upstream. + """ def __init__( self, @@ -122,6 +132,14 @@ class NestQuestCoordinator(DataUpdateCoordinator): ) self.entry_id = entry_id self.database = database + self._refresh_lock = asyncio.Lock() + + async def async_refresh(self) -> None: + """One refresh at a time; a queued refresh runs AFTER the + in-flight one publishes, so the last snapshot always reflects + the latest mutation.""" + async with self._refresh_lock: + await super().async_refresh() def _local_now(self) -> datetime.datetime: time_zone = ZoneInfo(self.hass.config.time_zone) diff --git a/custom_components/nestquest/services.py b/custom_components/nestquest/services.py index c03150d..f8c322b 100644 --- a/custom_components/nestquest/services.py +++ b/custom_components/nestquest/services.py @@ -242,6 +242,19 @@ def _require_runtime(hass: HomeAssistant, find_runtime: FindRuntime): return runtime +async def _refresh_entities(runtime: Any) -> None: + """Force an immediate coordinator refresh after a service mutation. + + Only the SERVICE paths call this — the scheduled materialize and + rollover internals are direct function calls and never widen this + gate — so entities reflect a completion the moment the service + call returns instead of waiting for the poll interval. + """ + coordinator = getattr(runtime, "coordinator", None) + if coordinator is not None: + await coordinator.async_refresh() + + def _generation_kwargs(hass: HomeAssistant, runtime: Any) -> dict[str, Any]: time_zone = ZoneInfo(hass.config.time_zone) today = datetime.datetime.now(time_zone).date() @@ -293,18 +306,19 @@ def _complete_quest( hass: HomeAssistant, find_runtime: FindRuntime ) -> ServiceHandler: async def _handler(call: Any) -> None: - database = _require_runtime(hass, find_runtime).database + runtime = _require_runtime(hass, find_runtime) instance_id = call.data["instance_id"] # ``appended`` is decided under the same lock as the write, so # a concurrent duplicate call (a double tap) can not produce a # second transition event: the loser appends nothing. result = await completion_layer.complete_instance( - database, + runtime.database, instance_id, **_actor_kwargs(call), ) if result.appended: - await fire_quest_completed(hass, database, instance_id) + await fire_quest_completed(hass, runtime.database, instance_id) + await _refresh_entities(runtime) return _with_errors(_handler) @@ -313,15 +327,18 @@ def _uncomplete_quest( hass: HomeAssistant, find_runtime: FindRuntime ) -> ServiceHandler: async def _handler(call: Any) -> None: - database = _require_runtime(hass, find_runtime).database + runtime = _require_runtime(hass, find_runtime) instance_id = call.data["instance_id"] result = await completion_layer.uncomplete_instance( - database, + runtime.database, instance_id, **_actor_kwargs(call), ) if result.appended: - await fire_quest_uncompleted(hass, database, instance_id) + await fire_quest_uncompleted( + hass, runtime.database, instance_id + ) + await _refresh_entities(runtime) return _with_errors(_handler) diff --git a/tests/test_events.py b/tests/test_events.py index ad019be..829f4ea 100644 --- a/tests/test_events.py +++ b/tests/test_events.py @@ -278,3 +278,102 @@ async def test_day_complete_ignored_for_backdated_completion( # path fires), and the follow-up service call on the now-done # future instance was a no-op: today's single completion stands. assert len(hass.bus.fired(EVENT_QUEST_COMPLETED)) == len(instances) + + +async def test_completion_updates_entities_immediately( + hass, make_entry +) -> None: + """The remaining-today sensor reflects the completion the moment + the service call returns — no manual refresh — and the day-complete + event fired exactly once when the day became clear.""" + from custom_components.nestquest.const import EVENT_CHILD_DAY_COMPLETE + + entry, child, instances = await _setup_and_seed(hass, make_entry) + from conftest import wire_entry_to_registry # noqa: F401 + + coordinator = entry.runtime_data.coordinator + # Entities only exist after one refresh since they were seeded + # after setup; create them, then prove the service path refreshes. + await coordinator.async_refresh() + remaining_before = hass.entities[ + f"nestquest_child_{child.id}_quests_remaining_today" + ].native_value + assert remaining_before == len(instances) + + await hass.services.call( + DOMAIN, + SERVICE_COMPLETE_QUEST, + { + "instance_id": instances[0].id, + "actor": "panel", + "actor_child_id": child.id, + }, + ) + remaining_after = hass.entities[ + f"nestquest_child_{child.id}_quests_remaining_today" + ].native_value + assert remaining_after == len(instances) - 1, ( + "sensor must reflect the completion without a manual refresh" + ) + assert len(hass.bus.fired(EVENT_CHILD_DAY_COMPLETE)) == ( + 1 if len(instances) == 1 else 0 + ) + + +async def test_uncompletion_updates_entities_immediately( + hass, make_entry +) -> None: + entry, child, instances = await _setup_and_seed(hass, make_entry) + coordinator = entry.runtime_data.coordinator + await coordinator.async_refresh() + await hass.services.call( + DOMAIN, + SERVICE_COMPLETE_QUEST, + { + "instance_id": instances[0].id, + "actor": "panel", + "actor_child_id": child.id, + }, + ) + completed_after_complete = hass.entities[ + f"nestquest_child_{child.id}_quests_completed_today" + ].native_value + assert completed_after_complete == 1 + + await hass.services.call( + DOMAIN, + SERVICE_UNCOMPLETE_QUEST, + {"instance_id": instances[0].id, "actor": "user"}, + context=ADMIN_CTX, + ) + completed_after_uncomplete = hass.entities[ + f"nestquest_child_{child.id}_quests_completed_today" + ].native_value + assert completed_after_uncomplete == 0, ( + "sensor must reflect the reversal without a manual refresh" + ) + + +async def test_forced_refreshes_serialize_no_stale_publish( + hass, make_entry +) -> None: + """Overlapping forced refreshes never interleave: a slow pass + pauses mid-snapshot, and the queued pass still publishes AFTER + it, so the last published snapshot reflects the latest mutation.""" + import asyncio + + entry, child, instances = await _setup_and_seed(hass, make_entry) + coordinator = entry.runtime_data.coordinator + order: list[str] = [] + + async def _slow_update(): + order.append("start") + await asyncio.sleep(0.01) + order.append("end") + return coordinator.data + + coordinator._async_update_data = _slow_update + await asyncio.gather(coordinator.async_refresh(), coordinator.async_refresh()) + assert order == ["start", "end", "start", "end"], ( + "overlapping refreshes must serialize: " + str(order) + )