diff --git a/custom_components/nestquest/dao_rules.py b/custom_components/nestquest/dao_rules.py index d994791..ae5dc5a 100644 --- a/custom_components/nestquest/dao_rules.py +++ b/custom_components/nestquest/dao_rules.py @@ -400,6 +400,37 @@ def _single_weekday_from_csv(value: str | None) -> int: ) from None +async def _insert_rule_row( + database: NestQuestDatabase, storage: ScheduleRuleStorage +) -> int: + """Insert one ``schedule_rules`` row and return its new id. + + Runs inside the caller's transaction (no lock or transaction of its + own) so a multi-table write can persist the rule together with the + rows that reference it. The caller is responsible for validating + the storage fields — :meth:`ScheduleRulesDao.create` validates the + date shape, and :meth:`QuestDefinitionsDao.create_with_rule_and_windows` + receives storage already mapped from a validated + :class:`~.recurrence.ScheduleRule`. + """ + result = await database.execute( + "INSERT INTO schedule_rules (rule_type, interval, weekday_set, " + "day_of_month, nth_weekday, month, start_date, end_date) " + "VALUES (?, ?, ?, ?, ?, ?, ?, ?)", + ( + storage.rule_type, + storage.interval, + storage.weekday_set, + storage.day_of_month, + storage.nth_weekday, + storage.month, + storage.start_date, + storage.end_date, + ), + ) + return result.lastrowid + + class ScheduleRulesDao: """Typed async access to the ``schedule_rules`` table.""" @@ -430,22 +461,20 @@ class ScheduleRulesDao: _validate_date(start_date, "start_date") if end_date is not None: _validate_date(end_date, "end_date") - result = await self._database.execute( - "INSERT INTO schedule_rules (rule_type, interval, weekday_set, " - "day_of_month, nth_weekday, month, start_date, end_date) " - "VALUES (?, ?, ?, ?, ?, ?, ?, ?)", - ( - rule_type, - interval, - weekday_set, - day_of_month, - nth_weekday, - month, - start_date, - end_date, + rule_id = await _insert_rule_row( + self._database, + ScheduleRuleStorage( + rule_type=rule_type, + interval=interval, + weekday_set=weekday_set, + day_of_month=day_of_month, + nth_weekday=nth_weekday, + month=month, + start_date=start_date, + end_date=end_date, ), ) - rule = await self.get(result.lastrowid) + rule = await self.get(rule_id) assert rule is not None return rule @@ -605,6 +634,71 @@ class QuestDefinitionsDao: assert definition is not None return definition + async def create_with_rule_and_windows( + self, + title: str, + schedule_rule: ScheduleRuleStorage, + created_at: str, + assignee_child_ids: list[int], + windows: list[tuple[str, str | None]], + *, + description: str | None = None, + icon: str | None = None, + ) -> QuestDefinitionRecord: + """Insert a rule, definition, assignees and windows atomically. + + The whole write — the schedule rule row, the definition row, + every assignee link and every window declaration — runs inside + ONE transaction under the connection lock, so a validation or + write failure rolls the lot back together (all-or-nothing): the + caller can never observe a definition with a missing rule, + assignee or window, and a rejected create leaves no partial + rows. + + The rule arrives as pre-validated storage fields (mapped from a + :class:`~.recurrence.ScheduleRule` via + :func:`schedule_rule_to_storage`); its dates are re-validated + here so a hand-built storage can never persist a malformed date. + The definition-level ``due_time`` column is deliberately left + NULL — per-window due times supersede it (D-008). Assignees are + validated as existing-and-active at insert time, and windows are + validated for name and strict HH:MM due time, matching + :meth:`upsert_window`. + """ + _validate_date(schedule_rule.start_date, "start_date") + if schedule_rule.end_date is not None: + _validate_date(schedule_rule.end_date, "end_date") + async with _connection_lock(self._database): + async with self._database.transaction(): + for child_id in assignee_child_ids: + await self._validate_assignable_child(child_id) + rule_id = await _insert_rule_row(self._database, schedule_rule) + result = await self._database.execute( + "INSERT INTO quest_definitions (title, description, " + "icon, schedule_rule_id, due_time, is_active, " + "created_at) VALUES (?, ?, ?, ?, ?, ?, ?)", + (title, description, icon, rule_id, None, 1, created_at), + ) + definition_id = result.lastrowid + for child_id in assignee_child_ids: + await self._database.execute( + "INSERT INTO quest_definition_assignees " + "(definition_id, child_id) VALUES (?, ?)", + (definition_id, child_id), + ) + for window, due_time in windows: + _validate_window(window) + if due_time is not None: + _validate_time(due_time, "due_time") + await self._database.execute( + "INSERT INTO quest_definition_windows " + "(definition_id, window, due_time) VALUES (?, ?, ?)", + (definition_id, window, due_time), + ) + definition = await self.get(definition_id) + assert definition is not None + return definition + async def _validate_rule_exists(self, schedule_rule_id: int) -> None: """Raise ValueError unless the rule exists. diff --git a/custom_components/nestquest/quest_definitions.py b/custom_components/nestquest/quest_definitions.py new file mode 100644 index 0000000..3cf8b36 --- /dev/null +++ b/custom_components/nestquest/quest_definitions.py @@ -0,0 +1,202 @@ +"""Business layer for creating quest definitions (Feature 06). + +Sits between the Feature 09 service gate and the typed +:class:`~.dao_rules.QuestDefinitionsDao`: callers get validation and the +all-or-nothing create here, and never touch the DAO SQL directly. +:func:`create_quest_definition` is the create path; there is no +permission check here (that is Feature 09). + +Validation policy mirrors :mod:`.children`: + +- ``title`` is required and trimmed; empty or whitespace-only raises + ValueError. +- ``rule`` must be an already-validated :class:`~.recurrence.ScheduleRule` + (its constructor is total — rule validation is NOT re-implemented + here). +- ``assignee_child_ids`` is non-empty; every id is a plain int (bools + and floats rejected, since SQLite would bind them onto a real child) + and must reference an existing ACTIVE child (the DAO enforces the + active/exists check inside the same transaction). +- ``windows`` is non-empty; each entry is a window name from + :data:`~.const.QUEST_WINDOWS` — either a bare name (no due time) or a + ``(name, due_time)`` pair whose due time is a strict 24-hour HH:MM + string. + +The rule, definition, assignees and windows are persisted in ONE +transaction (the DAO's :meth:`~.dao_rules.QuestDefinitionsDao.create_with_rule_and_windows`), +so a rejected or failed create persists nothing. The definition-level +``due_time`` column is superseded by per-window due times (D-008) and is +never set here. ``created_at`` is stamped UTC ISO-8601 at second +precision, matching :mod:`.children`. +""" +from __future__ import annotations + +import datetime +from dataclasses import dataclass + +from .dao_children import ChildRecord +from .dao_rules import ( + QuestDefinitionRecord, + QuestDefinitionWindowRecord, + QuestDefinitionsDao, + _validate_time, + _validate_window, + schedule_rule_from_storage, + schedule_rule_to_storage, +) +from .db import NestQuestDatabase +from .recurrence import ScheduleRule + + +@dataclass(frozen=True) +class CreatedQuestDefinition: + """The persisted result of :func:`create_quest_definition`. + + ``definition`` is the stored row; ``rule`` is the schedule rule + decoded back through the storage mapping; ``assignees`` and + ``windows`` are the definition's linked child records and window + declarations, read through the DAO. + """ + + definition: QuestDefinitionRecord + rule: ScheduleRule + assignees: list[ChildRecord] + windows: list[QuestDefinitionWindowRecord] + + +def _now_stamp() -> str: + """Return the module's strict UTC second-precision stamp.""" + return datetime.datetime.now(datetime.timezone.utc).isoformat( + timespec="seconds" + ) + + +def _validate_text( + value: object, field: str, *, required: bool = False +) -> str | None: + """Validate a free-text field and return it trimmed. + + ``None`` raises when ``required`` and otherwise returns None. A + non-string value is rejected; an empty result is rejected for + required fields. + """ + if value is None: + if required: + raise ValueError(f"{field} is required") + return None + if not isinstance(value, str): + raise ValueError(f"{field} must be a string, got {value!r}") + trimmed = value.strip() + if required and not trimmed: + raise ValueError(f"{field} is required") + return trimmed + + +def _validate_assignee_ids(value: object) -> list[int]: + """Return the assignee ids as plain ints, or raise naming the field. + + The list must be non-empty; each entry must be a real int (bools and + floats rejected) so SQLite cannot bind ``True`` onto child 1 or a + float onto a neighbouring profile. + """ + if not isinstance(value, (list, tuple)): + raise ValueError( + "assignee_child_ids must be a non-empty list of child ids" + ) + if not value: + raise ValueError("assignee_child_ids must not be empty") + ids: list[int] = [] + for entry in value: + if isinstance(entry, bool) or not isinstance(entry, int): + raise ValueError( + "assignee_child_ids must contain integer child ids, " + f"got {entry!r}" + ) + ids.append(entry) + return ids + + +def _normalize_windows( + value: object, +) -> list[tuple[str, str | None]]: + """Normalize the windows argument to ``(name, due_time)`` pairs. + + Each entry is a bare window name (due time None) or a + ``(name, due_time)`` pair. The name must be a + :data:`~.const.QUEST_WINDOWS` spelling and a present due time must be + a strict 24-hour HH:MM, both validated up front so a malformed + window is rejected before any write. + """ + if not isinstance(value, (list, tuple)): + raise ValueError( + "windows must be a non-empty list of window names or " + "(name, due_time) pairs" + ) + if not value: + raise ValueError("windows must not be empty") + normalized: list[tuple[str, str | None]] = [] + for index, entry in enumerate(value): + if isinstance(entry, str): + name: object = entry + due_time: object = None + elif isinstance(entry, (tuple, list)) and len(entry) == 2: + name, due_time = entry[0], entry[1] + else: + raise ValueError( + f"windows[{index}] must be a window name or a " + f"(name, due_time) pair, got {entry!r}" + ) + _validate_window(name) + if due_time is not None: + _validate_time(due_time, "due_time") + normalized.append((name, due_time)) + return normalized + + +async def create_quest_definition( + database: NestQuestDatabase, + title: str, + rule: ScheduleRule, + assignee_child_ids: list[int], + windows: list[str | tuple[str, str | None]], + *, + description: str | None = None, + icon: str | None = None, +) -> CreatedQuestDefinition: + """Create a quest definition (rule + assignees + windows) atomically. + + Validates the arguments up front, then persists the schedule rule, + the definition, its assignees and its windows in one transaction via + the DAO. Returns the stored definition together with its decoded + rule, assignees and windows. Raises ValueError on any rejected + argument; a rejected or failed create persists nothing. + """ + name = _validate_text(title, "title", required=True) + assert name is not None + if not isinstance(rule, ScheduleRule): + raise ValueError(f"rule must be a ScheduleRule, got {rule!r}") + assignee_ids = _validate_assignee_ids(assignee_child_ids) + window_specs = _normalize_windows(windows) + description_value = _validate_text(description, "description") + icon_value = _validate_text(icon, "icon") + + storage = schedule_rule_to_storage(rule) + dao = QuestDefinitionsDao(database) + definition = await dao.create_with_rule_and_windows( + name, + storage, + _now_stamp(), + assignee_ids, + window_specs, + description=description_value, + icon=icon_value, + ) + decoded_rule = schedule_rule_from_storage(storage) + assignees = await dao.list_assignees(definition.id) + window_records = await dao.list_windows(definition.id) + return CreatedQuestDefinition( + definition=definition, + rule=decoded_rule, + assignees=assignees, + windows=window_records, + ) diff --git a/tests/test_quest_definitions.py b/tests/test_quest_definitions.py new file mode 100644 index 0000000..35e2d01 --- /dev/null +++ b/tests/test_quest_definitions.py @@ -0,0 +1,353 @@ +"""Tests for quest_definitions.py: the Feature 06 create business layer.""" +from __future__ import annotations + +import asyncio +import re + +import pytest + +from custom_components.nestquest.dao_children import ChildrenDao +from custom_components.nestquest.dao_rules import ( + QuestDefinitionsDao, + ScheduleRulesDao, +) +from custom_components.nestquest.db import NestQuestDatabase +from custom_components.nestquest.migrations import apply_migrations +from custom_components.nestquest.quest_definitions import ( + CreatedQuestDefinition, + create_quest_definition, +) +from custom_components.nestquest.recurrence import RuleType, ScheduleRule + + +def _run(coro): + return asyncio.new_event_loop().run_until_complete(coro) + + +def _make_hass_mock(): + from unittest.mock import AsyncMock, MagicMock + + hass = MagicMock() + hass.async_add_executor_job = AsyncMock(side_effect=(lambda fn, *a: fn(*a))) + return hass + + +NOW = "2026-09-14T12:00:00+00:00" + + +async def _prepare(path) -> tuple: + database = NestQuestDatabase(_make_hass_mock()) + await database.open(path) + await apply_migrations(database) + children = ChildrenDao(database) + rules = ScheduleRulesDao(database) + definitions = QuestDefinitionsDao(database) + child = await children.create("Ada", NOW) + return database, children, rules, definitions, child + + +def _with_db(tmp_path, name): + """Run an async body against a prepared database, closing it after.""" + + def _run_test(body): + async def _main(): + database, children, rules, definitions, child = await _prepare( + tmp_path / name + ) + try: + return await body(database, children, rules, definitions, child) + finally: + await database.close() + + return _run(_main()) + + return _run_test + + +def _daily_rule() -> ScheduleRule: + return ScheduleRule(rule_type=RuleType.DAILY, start_date="2026-09-14") + + +# --------------------------------------------------------------------------- +# create_quest_definition: successful creates +# --------------------------------------------------------------------------- + + +def test_create_one_child_one_window(tmp_path) -> None: + async def _body(database, children, rules, definitions, child): + rule = _daily_rule() + result = await create_quest_definition( + database, + " Brush teeth ", + rule, + [child.id], + ["morning"], + description=" Twice daily ", + icon=" mdi:tooth ", + ) + assert isinstance(result, CreatedQuestDefinition) + definition = result.definition + assert definition.title == "Brush teeth" + assert definition.description == "Twice daily" + assert definition.icon == "mdi:tooth" + assert definition.is_active is True + # D-008: the definition-level due_time column is never set here. + assert definition.due_time is None + # Timestamp is stamped UTC at second precision by the business layer. + assert re.fullmatch( + r"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\+00:00", + definition.created_at, + ) + # The rule round-trips through the storage mapping. + assert result.rule == rule + assert definition.schedule_rule_id is not None + rule_row = await rules.get(definition.schedule_rule_id) + assert rule_row is not None + assert rule_row.rule_type == "daily" + # Assignees and windows are read back through the DAO. + assert [c.id for c in result.assignees] == [child.id] + assert [w.window for w in result.windows] == ["morning"] + assert result.windows[0].due_time is None + return result + + _with_db(tmp_path, "create-one.db")(_body) + + +def test_create_several_children_several_windows_with_due_times( + tmp_path, +) -> None: + async def _body(database, children, rules, definitions, child): + bo = await children.create("Bo", NOW) + cleo = await children.create("Cleo", NOW) + rule = ScheduleRule( + rule_type=RuleType.WEEKLY, + weekday_set={0, 2, 4}, + start_date="2026-09-14", + ) + result = await create_quest_definition( + database, + "Chore", + rule, + [child.id, bo.id, cleo.id], + [("morning", "09:00"), ("evening", "19:30")], + ) + assert sorted(c.id for c in result.assignees) == sorted( + [child.id, bo.id, cleo.id] + ) + by_name = {w.window: w.due_time for w in result.windows} + assert by_name == {"morning": "09:00", "evening": "19:30"} + assert result.rule == rule + stored = await definitions.get(result.definition.id) + assert stored is not None + assert stored.due_time is None + return result + + _with_db(tmp_path, "create-many.db")(_body) + + +# --------------------------------------------------------------------------- +# create_quest_definition: argument rejections +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("bad", ["", " ", "\t\n", None]) +def test_create_rejects_empty_title(tmp_path, bad) -> None: + async def _body(database, children, rules, definitions, child): + with pytest.raises(ValueError, match="title is required"): + await create_quest_definition( + database, bad, _daily_rule(), [child.id], ["morning"] + ) + return None + + _with_db(tmp_path, "create-bad-title.db")(_body) + + +def test_create_rejects_non_string_title(tmp_path) -> None: + async def _body(database, children, rules, definitions, child): + with pytest.raises(ValueError, match="title must be a string"): + await create_quest_definition( + database, 42, _daily_rule(), [child.id], ["morning"] + ) + return None + + _with_db(tmp_path, "create-bad-title-type.db")(_body) + + +@pytest.mark.parametrize("bad", [None, "daily", 42, {"rule_type": "daily"}]) +def test_create_rejects_non_schedule_rule(tmp_path, bad) -> None: + async def _body(database, children, rules, definitions, child): + with pytest.raises(ValueError, match="rule must be a ScheduleRule"): + await create_quest_definition( + database, "Brush teeth", bad, [child.id], ["morning"] + ) + return None + + _with_db(tmp_path, "create-bad-rule.db")(_body) + + +@pytest.mark.parametrize("bad", [[], ()]) +def test_create_rejects_empty_assignees(tmp_path, bad) -> None: + async def _body(database, children, rules, definitions, child): + with pytest.raises( + ValueError, match="assignee_child_ids must not be empty" + ): + await create_quest_definition( + database, "Brush teeth", _daily_rule(), bad, ["morning"] + ) + return None + + _with_db(tmp_path, "create-empty-assignees.db")(_body) + + +@pytest.mark.parametrize("bad", [True, False, 1.5, "1", None]) +def test_create_rejects_non_int_assignee(tmp_path, bad) -> None: + async def _body(database, children, rules, definitions, child): + with pytest.raises( + ValueError, match="assignee_child_ids must contain integer" + ): + await create_quest_definition( + database, "Brush teeth", _daily_rule(), [bad], ["morning"] + ) + return None + + _with_db(tmp_path, "create-bad-assignee.db")(_body) + + +def test_create_rejects_inactive_child(tmp_path) -> None: + async def _body(database, children, rules, definitions, child): + inactive = await children.create("Bo", NOW) + await children.set_active(inactive.id, False) + with pytest.raises(ValueError, match="inactive"): + await create_quest_definition( + database, + "Brush teeth", + _daily_rule(), + [inactive.id], + ["morning"], + ) + return None + + _with_db(tmp_path, "create-inactive-child.db")(_body) + + +def test_create_rejects_unknown_child(tmp_path) -> None: + async def _body(database, children, rules, definitions, child): + with pytest.raises(ValueError, match="does not exist"): + await create_quest_definition( + database, "Brush teeth", _daily_rule(), [999], ["morning"] + ) + return None + + _with_db(tmp_path, "create-unknown-child.db")(_body) + + +@pytest.mark.parametrize("bad", [[], ()]) +def test_create_rejects_empty_windows(tmp_path, bad) -> None: + async def _body(database, children, rules, definitions, child): + with pytest.raises(ValueError, match="windows must not be empty"): + await create_quest_definition( + database, "Brush teeth", _daily_rule(), [child.id], bad + ) + return None + + _with_db(tmp_path, "create-empty-windows.db")(_body) + + +@pytest.mark.parametrize("bad", [["noon"], [("MORNING", None)], [("", None)]]) +def test_create_rejects_invalid_window(tmp_path, bad) -> None: + async def _body(database, children, rules, definitions, child): + with pytest.raises(ValueError, match="window must be one of"): + await create_quest_definition( + database, "Brush teeth", _daily_rule(), [child.id], bad + ) + return None + + _with_db(tmp_path, "create-bad-window.db")(_body) + + +@pytest.mark.parametrize("bad", ["9:30", "25:00", "09:60", "0900"]) +def test_create_rejects_malformed_due_time(tmp_path, bad) -> None: + async def _body(database, children, rules, definitions, child): + with pytest.raises(ValueError, match="HH:MM"): + await create_quest_definition( + database, + "Brush teeth", + _daily_rule(), + [child.id], + [("morning", bad)], + ) + return None + + _with_db(tmp_path, "create-bad-due-time.db")(_body) + + +# --------------------------------------------------------------------------- +# create_quest_definition: a rejected create persists nothing +# --------------------------------------------------------------------------- + + +async def _assert_only_baseline_rows( + rules, definitions, child, baseline_definition +) -> None: + """Assert the baseline rule/definition/assignee/window rows survive + unchanged and no second row of any of them appeared. + + A rejected create must leave each table's row count exactly as it + was: still one rule, one definition, one assignee, one window — + checked through the DAO (id 2 must be absent everywhere, id 1 + intact), since the leak guard forbids raw SQL in this test file. + """ + assert await rules.get(1) is not None + assert await rules.get(2) is None + assert await definitions.get(1) == baseline_definition + assert await definitions.get(2) is None + assert [c.id for c in await definitions.list_assignees(1)] == [child.id] + assert await definitions.list_assignees(2) == [] + assert [w.window for w in await definitions.list_windows(1)] == [ + "morning" + ] + assert await definitions.list_windows(2) == [] + + +def test_rejected_create_writes_nothing(tmp_path) -> None: + """A rejected create leaves every quest row count unchanged. + + Two rejection paths are forced: an in-transaction rejection (an + inactive assignee fails validation inside the transaction and must + roll the rule/definition back together) and a pre-write rejection + (an invalid window is caught in the business layer before any write). + Both must leave the baseline single-rule/single-definition state + untouched, with no second row in any of the four tables. + """ + async def _body(database, children, rules, definitions, child): + baseline_rule = await rules.create("daily", "2026-09-14") + baseline_definition = await definitions.create( + "Baseline", baseline_rule.id, NOW, assignee_child_ids=[child.id] + ) + await definitions.upsert_window(baseline_definition.id, "morning") + inactive = await children.create("Bo", NOW) + await children.set_active(inactive.id, False) + rule = _daily_rule() + + # In-transaction rejection: the inactive child fails validation + # inside the transaction, rolling the rule and definition back. + with pytest.raises(ValueError, match="inactive"): + await create_quest_definition( + database, "Brush teeth", rule, [inactive.id], ["morning"] + ) + await _assert_only_baseline_rows( + rules, definitions, child, baseline_definition + ) + + # Pre-write rejection: an invalid window is caught up front, so + # no transaction ever opens. + with pytest.raises(ValueError, match="window must be one of"): + await create_quest_definition( + database, "Brush teeth", rule, [child.id], [("noon", None)] + ) + await _assert_only_baseline_rows( + rules, definitions, child, baseline_definition + ) + return None + + _with_db(tmp_path, "create-rejected-nothing.db")(_body)