fix: validate horizon_days before delete + wrapper forwarding tests (NQ-86-001, NQ-86-002)
This commit is contained in:
@@ -84,6 +84,31 @@ def _now_stamp() -> str:
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _resolve_horizon_days(horizon_days: int | None) -> int:
|
||||||
|
"""Validate a regeneration horizon and return the resolved day count.
|
||||||
|
|
||||||
|
``None`` resolves to :data:`~.const.DEFAULT_HORIZON_DAYS`. A bool
|
||||||
|
(SQLite would happily bind ``True``/``False`` onto an int), a non-int,
|
||||||
|
or a value below 1 raises ValueError naming the field — the validation
|
||||||
|
runs BEFORE any future-instance delete so a malformed horizon can never
|
||||||
|
delete rows it then fails to rebuild. A negative horizon would
|
||||||
|
otherwise delete first and only then be rejected by
|
||||||
|
:func:`materialize`'s inverted-range check; ``0``/``False`` would
|
||||||
|
silently shrink the window to a single day.
|
||||||
|
"""
|
||||||
|
if horizon_days is None:
|
||||||
|
return DEFAULT_HORIZON_DAYS
|
||||||
|
if (
|
||||||
|
isinstance(horizon_days, bool)
|
||||||
|
or not isinstance(horizon_days, int)
|
||||||
|
or horizon_days < 1
|
||||||
|
):
|
||||||
|
raise ValueError(
|
||||||
|
f"horizon_days must be a positive integer, got {horizon_days!r}"
|
||||||
|
)
|
||||||
|
return horizon_days
|
||||||
|
|
||||||
|
|
||||||
def _decode_rule(snapshot) -> ScheduleRule:
|
def _decode_rule(snapshot) -> ScheduleRule:
|
||||||
"""Decode a snapshot's rule record back into a ScheduleRule.
|
"""Decode a snapshot's rule record back into a ScheduleRule.
|
||||||
|
|
||||||
@@ -255,11 +280,12 @@ async def regenerate_for_definition(
|
|||||||
so a household time zone behind the host around midnight stays
|
so a household time zone behind the host around midnight stays
|
||||||
consistent. ``horizon_days`` sizes the re-materialization window;
|
consistent. ``horizon_days`` sizes the re-materialization window;
|
||||||
when omitted it falls back to :data:`~.const.DEFAULT_HORIZON_DAYS`.
|
when omitted it falls back to :data:`~.const.DEFAULT_HORIZON_DAYS`.
|
||||||
|
A bool, non-int, or sub-1 value raises ValueError BEFORE any delete.
|
||||||
Returns the number of instances the re-materialization
|
Returns the number of instances the re-materialization
|
||||||
upserted across the whole snapshot (idempotent — other active
|
upserted across the whole snapshot (idempotent — other active
|
||||||
definitions' tuples refresh in place, never duplicate).
|
definitions' tuples refresh in place, never duplicate).
|
||||||
"""
|
"""
|
||||||
horizon = DEFAULT_HORIZON_DAYS if horizon_days is None else horizon_days
|
horizon = _resolve_horizon_days(horizon_days)
|
||||||
today_date = _resolve_today(today)
|
today_date = _resolve_today(today)
|
||||||
start_date = today_date.isoformat()
|
start_date = today_date.isoformat()
|
||||||
end_date = (
|
end_date = (
|
||||||
@@ -323,14 +349,15 @@ async def regenerate_for_child(
|
|||||||
into the delete cutoff and the re-materialization for the same
|
into the delete cutoff and the re-materialization for the same
|
||||||
host-vs-HA time-zone consistency. ``horizon_days`` sizes the
|
host-vs-HA time-zone consistency. ``horizon_days`` sizes the
|
||||||
re-materialization window; when omitted it falls back to
|
re-materialization window; when omitted it falls back to
|
||||||
:data:`~.const.DEFAULT_HORIZON_DAYS`. Returns the number of instances
|
:data:`~.const.DEFAULT_HORIZON_DAYS`. A bool, non-int, or sub-1 value
|
||||||
|
raises ValueError BEFORE any delete. Returns the number of instances
|
||||||
the re-materialization upserted for the child across the whole
|
the re-materialization upserted for the child across the whole
|
||||||
snapshot.
|
snapshot.
|
||||||
"""
|
"""
|
||||||
if isinstance(child_id, bool) or not isinstance(child_id, int):
|
if isinstance(child_id, bool) or not isinstance(child_id, int):
|
||||||
raise ValueError(f"child_id must be an integer, got {child_id!r}")
|
raise ValueError(f"child_id must be an integer, got {child_id!r}")
|
||||||
|
|
||||||
horizon = DEFAULT_HORIZON_DAYS if horizon_days is None else horizon_days
|
horizon = _resolve_horizon_days(horizon_days)
|
||||||
today_date = _resolve_today(today)
|
today_date = _resolve_today(today)
|
||||||
|
|
||||||
instances = QuestInstancesDao(database)
|
instances = QuestInstancesDao(database)
|
||||||
|
|||||||
@@ -1193,6 +1193,79 @@ def test_regenerate_for_child_respects_horizon_days(tmp_path) -> None:
|
|||||||
_with_db(tmp_path, "regenerate-child-horizon.db")(_body)
|
_with_db(tmp_path, "regenerate-child-horizon.db")(_body)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("bad", [-1, 0, True, False, "5", 1.5])
|
||||||
|
def test_regenerate_for_definition_rejects_invalid_horizon_before_delete(
|
||||||
|
tmp_path, bad
|
||||||
|
) -> None:
|
||||||
|
"""A malformed horizon fails BEFORE any future-instance delete."""
|
||||||
|
async def _body(database):
|
||||||
|
children = ChildrenDao(database)
|
||||||
|
child = await children.create("Ada", NOW)
|
||||||
|
|
||||||
|
today = datetime.date.today()
|
||||||
|
start_iso = today.isoformat()
|
||||||
|
horizon_end = today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS)
|
||||||
|
|
||||||
|
created = await create_quest_definition(
|
||||||
|
database,
|
||||||
|
"Daily chore",
|
||||||
|
ScheduleRule(rule_type=RuleType.DAILY, start_date=start_iso),
|
||||||
|
[child.id],
|
||||||
|
[("morning", "09:00")],
|
||||||
|
)
|
||||||
|
dao = QuestInstancesDao(database)
|
||||||
|
await materialize(database, start_iso, horizon_end.isoformat())
|
||||||
|
|
||||||
|
with pytest.raises(ValueError, match="horizon_days"):
|
||||||
|
await regenerate_for_definition(
|
||||||
|
database, created.definition.id, horizon_days=bad
|
||||||
|
)
|
||||||
|
|
||||||
|
# The refused call deleted nothing: the seeded horizon survives.
|
||||||
|
records = await dao.list_by_date_range(
|
||||||
|
child.id, start_iso, horizon_end.isoformat()
|
||||||
|
)
|
||||||
|
assert len(records) == DEFAULT_HORIZON_DAYS + 1
|
||||||
|
return None
|
||||||
|
|
||||||
|
_with_db(tmp_path, "regenerate-def-horizon-invalid.db")(_body)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("bad", [-1, 0, True, False, "5", 1.5])
|
||||||
|
def test_regenerate_for_child_rejects_invalid_horizon_before_delete(
|
||||||
|
tmp_path, bad
|
||||||
|
) -> None:
|
||||||
|
"""A malformed child horizon fails BEFORE any future-instance delete."""
|
||||||
|
async def _body(database):
|
||||||
|
children = ChildrenDao(database)
|
||||||
|
child = await children.create("Ada", NOW)
|
||||||
|
|
||||||
|
today = datetime.date.today()
|
||||||
|
start_iso = today.isoformat()
|
||||||
|
horizon_end = today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS)
|
||||||
|
|
||||||
|
await create_quest_definition(
|
||||||
|
database,
|
||||||
|
"Daily chore",
|
||||||
|
ScheduleRule(rule_type=RuleType.DAILY, start_date=start_iso),
|
||||||
|
[child.id],
|
||||||
|
[("morning", "09:00")],
|
||||||
|
)
|
||||||
|
dao = QuestInstancesDao(database)
|
||||||
|
await materialize(database, start_iso, horizon_end.isoformat())
|
||||||
|
|
||||||
|
with pytest.raises(ValueError, match="horizon_days"):
|
||||||
|
await regenerate_for_child(database, child.id, horizon_days=bad)
|
||||||
|
|
||||||
|
records = await dao.list_by_date_range(
|
||||||
|
child.id, start_iso, horizon_end.isoformat()
|
||||||
|
)
|
||||||
|
assert len(records) == DEFAULT_HORIZON_DAYS + 1
|
||||||
|
return None
|
||||||
|
|
||||||
|
_with_db(tmp_path, "regenerate-child-horizon-invalid.db")(_body)
|
||||||
|
|
||||||
|
|
||||||
#: Fixed end-to-end anchor: 2026-06-01 is a Monday AND the 1st of its month,
|
#: Fixed end-to-end anchor: 2026-06-01 is a Monday AND the 1st of its month,
|
||||||
#: so the two-week custody cycle is week-aligned and the yearly rule's
|
#: so the two-week custody cycle is week-aligned and the yearly rule's
|
||||||
#: "1st of the anchor month" is GUARANTEED to fire inside the 28-day window
|
#: "1st of the anchor month" is GUARANTEED to fire inside the 28-day window
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ import re
|
|||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
from custom_components.nestquest.dao_children import ChildrenDao
|
from custom_components.nestquest.dao_children import ChildrenDao
|
||||||
|
from custom_components.nestquest.const import DEFAULT_HORIZON_DAYS
|
||||||
from custom_components.nestquest.dao_instances import (
|
from custom_components.nestquest.dao_instances import (
|
||||||
CompletionEventsDao,
|
CompletionEventsDao,
|
||||||
QuestInstancesDao,
|
QuestInstancesDao,
|
||||||
@@ -1938,3 +1939,71 @@ def test_full_lifecycle_preserves_completed_and_history(tmp_path) -> None:
|
|||||||
return reactivated
|
return reactivated
|
||||||
|
|
||||||
_with_db(tmp_path, "full-lifecycle.db")(_body)
|
_with_db(tmp_path, "full-lifecycle.db")(_body)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"mode",
|
||||||
|
["edit", "assign", "unassign", "set_active"],
|
||||||
|
)
|
||||||
|
def test_wrappers_forward_horizon_days(tmp_path, mode) -> None:
|
||||||
|
"""The four change wrappers forward a non-default horizon_days.
|
||||||
|
|
||||||
|
Each wrapper threads ``horizon_days`` into the regeneration it triggers;
|
||||||
|
passing 5 here must bound the regenerated window to today..today+5 —
|
||||||
|
proving the forwarding argument is actually honored, not dropped.
|
||||||
|
"""
|
||||||
|
async def _body(database, children, rules, definitions, child):
|
||||||
|
created = await create_quest_definition(
|
||||||
|
database,
|
||||||
|
"Chore",
|
||||||
|
ScheduleRule(rule_type=RuleType.DAILY, start_date=_today_iso()),
|
||||||
|
[child.id],
|
||||||
|
["morning"],
|
||||||
|
)
|
||||||
|
definition_id = created.definition.id
|
||||||
|
instances = QuestInstancesDao(database)
|
||||||
|
|
||||||
|
if mode == "edit":
|
||||||
|
await edit_quest_definition(
|
||||||
|
database,
|
||||||
|
definition_id,
|
||||||
|
windows=[("morning", "10:30")],
|
||||||
|
horizon_days=5,
|
||||||
|
)
|
||||||
|
tracked = child
|
||||||
|
elif mode == "assign":
|
||||||
|
bo = await children.create("Bo", NOW)
|
||||||
|
await assign_child(database, definition_id, bo.id, horizon_days=5)
|
||||||
|
tracked = bo
|
||||||
|
elif mode == "unassign":
|
||||||
|
# The remaining assignee (``child``) must be regenerated over
|
||||||
|
# the 5-day window after ``bo`` is removed.
|
||||||
|
bo = await children.create("Bo", NOW)
|
||||||
|
await assign_child(database, definition_id, bo.id)
|
||||||
|
await unassign_child(
|
||||||
|
database, definition_id, bo.id, horizon_days=5
|
||||||
|
)
|
||||||
|
tracked = child
|
||||||
|
else: # set_active
|
||||||
|
await set_quest_definition_active(
|
||||||
|
database, definition_id, True, horizon_days=5
|
||||||
|
)
|
||||||
|
tracked = child
|
||||||
|
|
||||||
|
today = datetime.date.today()
|
||||||
|
end = (today + datetime.timedelta(days=5)).isoformat()
|
||||||
|
records = await instances.list_by_date_range(
|
||||||
|
tracked.id, today.isoformat(), end
|
||||||
|
)
|
||||||
|
assert len(records) == 6
|
||||||
|
|
||||||
|
beyond = (today + datetime.timedelta(days=6)).isoformat()
|
||||||
|
beyond_end = (
|
||||||
|
today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS)
|
||||||
|
).isoformat()
|
||||||
|
assert await instances.list_by_date_range(
|
||||||
|
tracked.id, beyond, beyond_end
|
||||||
|
) == []
|
||||||
|
return None
|
||||||
|
|
||||||
|
_with_db(tmp_path, f"wrapper-horizon-{mode}.db")(_body)
|
||||||
|
|||||||
Reference in New Issue
Block a user