From 775a9906e7ae7a7a6fb4081d8e8c0332f7d9ed8b Mon Sep 17 00:00:00 2001 From: Joshua Date: Fri, 18 Sep 2026 06:47:45 -0400 Subject: [PATCH] complete_instance (Feature 08, task 3) (#90) --- custom_components/nestquest/completion.py | 128 +++++++++++- tests/test_completion.py | 244 +++++++++++++++++++++- 2 files changed, 364 insertions(+), 8 deletions(-) diff --git a/custom_components/nestquest/completion.py b/custom_components/nestquest/completion.py index 94bbb46..d87bb31 100644 --- a/custom_components/nestquest/completion.py +++ b/custom_components/nestquest/completion.py @@ -4,11 +4,11 @@ Sits between the Feature 09 service gate and the typed :class:`~.dao_instances.CompletionEventsDao`: callers get actor-shape validation and the UTC occurred-at stamp here, and never talk to the DAO or SQL directly. This module is the only Feature-08 caller of -:meth:`~.dao_instances.CompletionEventsDao.append`. Completing and -un-completing an instance (no-op-if-already-done) arrive in later -tasks. Instance current state is derived from the latest event via -:func:`instance_state` (``open`` / ``done``); there is no status -column. +:meth:`~.dao_instances.CompletionEventsDao.append`. Completing an +instance (no-op if already done) is :func:`complete_instance`; +un-completing arrives in a later task. Instance current state is +derived from the latest event via :func:`instance_state` +(``open`` / ``done``); there is no status column. Actor policy matches the schema CHECKs (D-008): ``actor_source`` is ``'user'`` or ``'panel'`` only — never ``'service'``. ``'user'`` @@ -25,9 +25,10 @@ is no permission check here (that is Feature 09). """ from __future__ import annotations +import asyncio import datetime -from .dao_children import _connection_lock +from .dao_children import _CONNECTION_LOCKS, _connection_lock from .dao_instances import ( EVENT_COMPLETED, EVENT_UNCOMPLETED, @@ -62,6 +63,63 @@ def _stamp_occurred_at(now: datetime.datetime | None) -> str: ) +class _TaskReentrantLock: + """Shared connection lock the owning task may re-enter.""" + + def __init__(self, inner: asyncio.Lock) -> None: + self._inner = inner + self._task: asyncio.Task | None = None + self._depth = 0 + + def locked(self) -> bool: + return self._inner.locked() + + async def acquire(self) -> bool: + task = asyncio.current_task() + if self._task is task: + self._depth += 1 + return True + await self._inner.acquire() + self._task = task + self._depth = 1 + return True + + def release(self) -> None: + if self._task is not asyncio.current_task(): + raise RuntimeError("lock released by a non-owner") + self._depth -= 1 + if self._depth == 0: + self._task = None + self._inner.release() + + async def __aenter__(self) -> _TaskReentrantLock: + await self.acquire() + return self + + async def __aexit__(self, *exc: object) -> None: + self.release() + + +def _reentrant_connection_lock(database) -> _TaskReentrantLock: + """Return the shared connection lock, made task-reentrant. + + ``CompletionEventsDao.append`` also acquires this lock, so + check+append in :func:`complete_instance` must re-enter on the + same task. The wrapper is stored in the shared dict so the DAO + sees the same object. + """ + loop = asyncio.get_running_loop() + key = (id(database), id(loop)) + lock = _CONNECTION_LOCKS.get(key) + if isinstance(lock, _TaskReentrantLock): + return lock + wrapped = _TaskReentrantLock( + lock if lock is not None else asyncio.Lock() + ) + _CONNECTION_LOCKS[key] = wrapped + return wrapped + + def _validate_int_id(value: object, field: str) -> int: """Reject non-int ids (bools included) before any lookup.""" if isinstance(value, bool) or not isinstance(value, int): @@ -198,3 +256,61 @@ async def instance_state( f"event_type must be '{EVENT_COMPLETED}' or " f"'{EVENT_UNCOMPLETED}', got {latest.event_type!r}" ) + + +async def complete_instance( + database: NestQuestDatabase, + instance_id: int, + *, + actor_source: str, + actor_user_id: str | None = None, + actor_child_id: int | None = None, + now: datetime.datetime | None = None, + today: datetime.date | None = None, +) -> str: + """Append a completed event unless the instance is already done. + + Returns the derived state ``done``. An already-done instance is a + no-op: nothing is appended. Unknown ``instance_id`` raises + ValueError naming the field. Existence, latest-event, and the + append run under one connection lock so a concurrent complete + cannot double-append. + + ``was_on_time`` is recorded as True — a placeholder until the + on-time task computes it from ``now`` / ``today`` against the + instance due_date and optional due_time. ``today`` is accepted so + callers can thread the HA-local date through without a signature + change; it is unused here. + """ + instance_id = _validate_int_id(instance_id, "instance_id") + actor_source, actor_user_id, actor_child_id = _validate_actor( + actor_source, actor_user_id, actor_child_id + ) + async with _reentrant_connection_lock(database): + instance = await QuestInstancesDao(database).get_by_id(instance_id) + if instance is None: + raise ValueError( + f"instance_id: quest instance {instance_id} does not exist" + ) + latest = await CompletionEventsDao(database).get_latest_for_instance( + instance_id + ) + if latest is not None and latest.event_type == EVENT_COMPLETED: + return "done" + if latest is not None and latest.event_type != EVENT_UNCOMPLETED: + raise ValueError( + f"event_type must be '{EVENT_COMPLETED}' or " + f"'{EVENT_UNCOMPLETED}', got {latest.event_type!r}" + ) + await append_event( + database, + instance.id, + instance.child_id, + EVENT_COMPLETED, + actor_source=actor_source, + actor_user_id=actor_user_id, + actor_child_id=actor_child_id, + was_on_time=True, + now=now, + ) + return "done" diff --git a/tests/test_completion.py b/tests/test_completion.py index 5412db5..e9e2ff4 100644 --- a/tests/test_completion.py +++ b/tests/test_completion.py @@ -8,7 +8,11 @@ import re import pytest -from custom_components.nestquest.completion import append_event, instance_state +from custom_components.nestquest.completion import ( + append_event, + complete_instance, + instance_state, +) from custom_components.nestquest.dao_children import ChildrenDao from custom_components.nestquest.dao_instances import ( EVENT_COMPLETED, @@ -333,7 +337,7 @@ def test_module_never_exposes_update_or_delete() -> None: ) assert "append_event" in public assert "instance_state" in public - assert "complete_instance" not in public + assert "complete_instance" in public assert "uncomplete_instance" not in public source = inspect.getsource(completion) assert "UPDATE" not in source @@ -444,3 +448,239 @@ def test_instance_state_three_events_returns_done_and_preserves_rows( assert len(after) == 3 _with_db(tmp_path, "state-three.db")(_body) + + +# --------------------------------------------------------------------------- +# complete_instance: append completed, no-op if already done +# --------------------------------------------------------------------------- + + +def test_complete_instance_appends_and_returns_done(tmp_path) -> None: + async def _body(database, child, instance): + events = CompletionEventsDao(database) + assert await events.list_by_instance(instance.id) == [] + state = await complete_instance( + database, + instance.id, + actor_source="user", + actor_user_id="user-1", + ) + assert state == "done" + assert await instance_state(database, instance.id) == "done" + rows = await events.list_by_instance(instance.id) + assert len(rows) == 1 + assert rows[0].event_type == EVENT_COMPLETED + assert rows[0].actor_source == "user" + assert rows[0].actor_user_id == "user-1" + assert rows[0].was_on_time is True + + _with_db(tmp_path, "complete-open.db")(_body) + + +def test_complete_instance_already_done_is_noop(tmp_path) -> None: + async def _body(database, child, instance): + first = await complete_instance( + database, + instance.id, + actor_source="user", + actor_user_id="user-1", + ) + events = CompletionEventsDao(database) + before = await events.list_by_instance(instance.id) + assert first == "done" + assert len(before) == 1 + second = await complete_instance( + database, + instance.id, + actor_source="user", + actor_user_id="user-2", + ) + after = await events.list_by_instance(instance.id) + assert second == "done" + assert after == before + assert after[0].actor_user_id == "user-1" + + _with_db(tmp_path, "complete-noop.db")(_body) + + +def test_complete_instance_unknown_instance_id_raises(tmp_path) -> None: + async def _body(database, child, instance): + with pytest.raises(ValueError, match="instance_id"): + await complete_instance( + database, + instance.id + 999, + actor_source="user", + actor_user_id="user-1", + ) + + _with_db(tmp_path, "complete-unknown.db")(_body) + + +def test_complete_instance_rejects_non_integer_id() -> None: + async def _body(): + with pytest.raises(ValueError, match="instance_id"): + await complete_instance( + object(), + "1", + actor_source="user", + actor_user_id="user-1", + ) + with pytest.raises(ValueError, match="instance_id"): + await complete_instance( + object(), + True, + actor_source="user", + actor_user_id="user-1", + ) + + _run(_body()) + + +def test_complete_instance_after_uncompleted_appends(tmp_path) -> None: + async def _body(database, child, instance): + await append_event( + database, + instance.id, + child.id, + EVENT_COMPLETED, + actor_source="user", + actor_user_id="user-1", + was_on_time=True, + ) + await append_event( + database, + instance.id, + child.id, + EVENT_UNCOMPLETED, + actor_source="user", + actor_user_id="user-1", + ) + events = CompletionEventsDao(database) + before = await events.list_by_instance(instance.id) + assert await instance_state(database, instance.id) == "open" + state = await complete_instance( + database, + instance.id, + actor_source="panel", + actor_child_id=child.id, + ) + assert state == "done" + after = await events.list_by_instance(instance.id) + assert len(after) == len(before) + 1 + assert after[-1].event_type == EVENT_COMPLETED + assert after[-1].actor_source == "panel" + assert after[-1].was_on_time is True + + _with_db(tmp_path, "complete-reopen.db")(_body) + + +def test_complete_instance_now_pins_occurred_at(tmp_path) -> None: + async def _body(database, child, instance): + pinned = datetime.datetime( + 2026, 9, 18, 8, 0, 0, tzinfo=datetime.timezone.utc + ) + await complete_instance( + database, + instance.id, + actor_source="user", + actor_user_id="user-1", + now=pinned, + ) + rows = await CompletionEventsDao(database).list_by_instance( + instance.id + ) + assert rows[0].occurred_at == "2026-09-18T08:00:00+00:00" + + _with_db(tmp_path, "complete-pinned.db")(_body) + + +def _gated_hass(armed: dict, gate_open: asyncio.Event, a_started: asyncio.Event): + """A hass mock whose executor gates the FIRST ``_execute`` job after + arming — the INSERT of whichever ``complete_instance`` starts first + — proving that caller holds the connection lock and the second + caller must queue behind it.""" + from unittest.mock import MagicMock + + hass = MagicMock() + + async def _gated_executor(fn, *args): + if armed["active"]: + if getattr(fn, "__name__", "") == "_execute" and not armed["gated"]: + armed["gated"] = True + a_started.set() + await gate_open.wait() + return fn(*args) + + hass.async_add_executor_job = _gated_executor + return hass + + +def test_complete_instance_concurrent_second_call_is_noop(tmp_path) -> None: + """Two racing completes cannot both append. + + Deterministic gate: task A is paused at its INSERT (it holds the + connection lock); task B is then started and must queue. A + finishes, B sees done and appends nothing. + """ + async def _main(): + armed = {"active": False, "gated": False} + gate_open = asyncio.Event() + a_started = asyncio.Event() + hass = _gated_hass(armed, gate_open, a_started) + database = NestQuestDatabase(hass) + await database.open(tmp_path / "complete-race.db") + try: + await apply_migrations(database) + children = ChildrenDao(database) + rules = ScheduleRulesDao(database) + definitions = QuestDefinitionsDao(database) + instances = QuestInstancesDao(database) + child = await children.create("Ada", _now_stamp()) + rule = await rules.create("daily", D1) + definition = await definitions.create( + "Brush teeth", + rule.id, + _now_stamp(), + assignee_child_ids=[child.id], + ) + instance = await instances.upsert( + definition.id, + child.id, + D1, + _now_stamp(), + window="morning", + ) + + armed["active"] = True + task_a = asyncio.ensure_future( + complete_instance( + database, + instance.id, + actor_source="user", + actor_user_id="user-1", + ) + ) + await a_started.wait() + armed["active"] = False + task_b = asyncio.ensure_future( + complete_instance( + database, + instance.id, + actor_source="user", + actor_user_id="user-2", + ) + ) + await asyncio.sleep(0) + gate_open.set() + state_a, state_b = await asyncio.gather(task_a, task_b) + assert state_a == "done" + assert state_b == "done" + rows = await CompletionEventsDao(database).list_by_instance( + instance.id + ) + assert len(rows) == 1 + assert rows[0].actor_user_id == "user-1" + finally: + await database.close() + + _run(_main())