From d006d0606d38d18dacdd6f307b293e0fb7e3d8a2 Mon Sep 17 00:00:00 2001 From: Joshua King Date: Mon, 14 Sep 2026 10:34:38 -0400 Subject: [PATCH 1/5] feat: add ScheduleRule model with validation and serialization (#25) --- custom_components/nestquest/recurrence.py | 396 ++++++++++++++++++++++ tests/test_recurrence_model.py | 300 ++++++++++++++++ 2 files changed, 696 insertions(+) create mode 100644 custom_components/nestquest/recurrence.py create mode 100644 tests/test_recurrence_model.py diff --git a/custom_components/nestquest/recurrence.py b/custom_components/nestquest/recurrence.py new file mode 100644 index 0000000..376669f --- /dev/null +++ b/custom_components/nestquest/recurrence.py @@ -0,0 +1,396 @@ +"""The ScheduleRule model: rule types, validation, and serialization. + +A :class:`ScheduleRule` describes WHEN a recurring chore fires — a pure +data structure with no Home Assistant and no database imports. The +evaluation engine (:mod:`.recurrence`, Feature 04) consumes it; +the DAO layer (Feature 02) serializes its dict form into the +``schedule_rules`` table. + +Type model: one enum per rule family. ``MONTHLY_DAY`` ("the 15th of +every month") and ``MONTHLY_WEEKDAY`` ("the second Tuesday of every +month") are distinct types rather than one monthly type with optional +fields, so every field combination a rule can carry is decided by the +rule type alone and validation is total: a rule either constructs, or +raises :class:`RuleValidationError` naming the offending fields. + +Validation policy (fail fast at construction — the schema's CHECK +constraints are a second line of defense, but invalid rules must never +reach storage): + +- ``interval`` >= 1 for every rule. +- ``weekday_set`` is required for WEEKLY and CUSTOM_DAYS, forbidden + elsewhere; each element must be 0 (Monday) .. 6 (Sunday). +- ``day_of_month`` 1..31 is required for MONTHLY_DAY. +- ``nth_weekday`` in -1..5 (1 = first, 5 = last-possible, -1 = last) + and a weekday 0..6 are required for MONTHLY_WEEKDAY. +- ``month`` 1..12 is required for YEARLY. +- ``start_date`` is a strict ISO date; ``end_date`` is optional and, + when present, must be on or after ``start_date``. +- ``due_time`` is optional and must be HH:MM 24-hour. + +Month-end policy (documented for Feature 04): a MONTHLY_DAY rule for +the 31st does NOT fire in a 30-day month, and a YEARLY rule for +February 29th does NOT fire in a non-leap year — the engine skips +instead of clamping to month end, because clamping silently moves +"the 31st" to "the 30th" and double-fires tasks. ``weekday_in_month`` +(True = Nth weekday, False = Nth-from-last for negative n) resolves +the MONTHLY_WEEKDAY position. +""" +from __future__ import annotations + +import datetime +import enum +from dataclasses import dataclass, field + +#: Date-shape policy: strict YYYY-MM-DD, matching the DAO layers. +_DATE_FORMAT = "%Y-%m-%d" + + +class RuleType(enum.Enum): + """The six recurrence shapes the engine supports.""" + + DAILY = "daily" + WEEKLY = "weekly" + MONTHLY_DAY = "monthly_day" + MONTHLY_WEEKDAY = "monthly_weekday" + YEARLY = "yearly" + CUSTOM_DAYS = "custom_days" + + +class RuleValidationError(ValueError): + """Raised when a ScheduleRule field combination is invalid.""" + + +def _validate_date(value: str, field: str) -> None: + """Raise unless ``value`` is a strict YYYY-MM-DD calendar date.""" + try: + parsed = datetime.datetime.strptime(value, _DATE_FORMAT).date() + except (TypeError, ValueError): + raise RuleValidationError( + f"{field} must be an ISO date (YYYY-MM-DD), got {value!r}" + ) from None + if parsed.isoformat() != value: + raise RuleValidationError( + f"{field} must be a strict YYYY-MM-DD date, got {value!r}" + ) + + +def _validate_weekday_set(weekday_set) -> None: + """Raise unless ``weekday_set`` is a non-empty 0-6 integer set.""" + if not isinstance(weekday_set, (set, frozenset)): + raise RuleValidationError( + "weekday_set must be a set of weekday integers, got " + f"{type(weekday_set).__name__}" + ) + if not weekday_set: + raise RuleValidationError( + "weekday_set must not be empty for weekly/custom rules" + ) + for entry in weekday_set: + if not isinstance(entry, int) or isinstance(entry, bool): + raise RuleValidationError( + f"weekday_set entries must be integers, got {entry!r}" + ) + if not 0 <= entry <= 6: + raise RuleValidationError( + f"weekday_set entries must be 0 (Monday)..6 (Sunday), " + f"got {entry!r}" + ) + + +def _validate_monthday_set(nth_weekday: int | None, weekday: int | None) -> None: + """Validate the MONTHLY_WEEKDAY position fields.""" + if nth_weekday is None or not isinstance(nth_weekday, int) or ( + isinstance(nth_weekday, bool) + ): + raise RuleValidationError( + "nth_weekday must be an integer -1..5 for monthly_weekday " + f"rules, got {nth_weekday!r}" + ) + if not (-1 <= nth_weekday <= 5) or nth_weekday == 0: + raise RuleValidationError( + f"nth_weekday must be -1 (last) or 1..5, got {nth_weekday!r}" + ) + if weekday is None: + raise RuleValidationError( + "monthly_weekday rules require a weekday (0..6) naming the " + "day of week" + ) + if not isinstance(weekday, int) or isinstance(weekday, bool) or not ( + 0 <= weekday <= 6 + ): + raise RuleValidationError( + f"weekday must be an integer 0 (Monday)..6 (Sunday), got " + f"{weekday!r}" + ) + + +class ScheduleRule: + """An immutable, validated recurrence rule. + + Construct with keyword arguments; invalid combinations raise + :class:`RuleValidationError` at construction time. The fields map + one-to-one onto the ``schedule_rules`` table columns. + """ + + __slots__ = ( + "rule_type", + "interval", + "weekday_set", + "day_of_month", + "nth_weekday", + "nth_weekday_weekday", + "month", + "start_date", + "end_date", + ) + + def __init__( + self, + rule_type: RuleType | str, + *, + interval: int = 1, + weekday_set: frozenset[int] | set[int] | None = None, + day_of_month: int | None = None, + nth_weekday: int | None = None, + nth_weekday_weekday: int | None = None, + month: int | None = None, + start_date: str = "1970-01-01", + end_date: str | None = None, + ) -> None: + if isinstance(rule_type, str): + try: + rule_type = RuleType(rule_type) + except ValueError: + raise RuleValidationError( + f"rule_type must be one of " + f"{[rt.value for rt in RuleType]}, got {rule_type!r}" + ) from None + if not isinstance(interval, int) or isinstance(interval, bool): + raise RuleValidationError( + f"interval must be an integer >= 1, got {interval!r}" + ) + if interval < 1: + raise RuleValidationError( + f"interval must be >= 1, got {interval!r}" + ) + + _validate_date(start_date, "start_date") + if end_date is not None: + _validate_date(end_date, "end_date") + if end_date < start_date: + raise RuleValidationError( + f"end_date {end_date!r} must be on or after " + f"start_date {start_date!r}" + ) + + if rule_type is RuleType.WEEKLY: + if weekday_set is None: + raise RuleValidationError( + "weekly rules require weekday_set" + ) + _validate_weekday_set(weekday_set) + weekday_set = frozenset(weekday_set) + elif rule_type is RuleType.CUSTOM_DAYS: + if weekday_set is None: + raise RuleValidationError( + "custom_days rules require weekday_set" + ) + _validate_weekday_set(weekday_set) + weekday_set = frozenset(weekday_set) + if day_of_month is not None: + raise RuleValidationError( + "custom_days rules must not set day_of_month" + ) + if nth_weekday is not None: + raise RuleValidationError( + "custom_days rules must not set nth_weekday" + ) + if month is not None: + raise RuleValidationError( + "custom_days rules must not set month" + ) + elif rule_type is RuleType.MONTHLY_DAY: + if day_of_month is None or not isinstance(day_of_month, int) or ( + isinstance(day_of_month, bool) + ): + raise RuleValidationError( + "monthly_day rules require day_of_month (1..31)" + ) + if not 1 <= day_of_month <= 31: + raise RuleValidationError( + f"day_of_month must be 1..31, got {day_of_month!r}" + ) + if weekday_set is not None: + raise RuleValidationError( + "monthly_day rules must not set weekday_set" + ) + if nth_weekday is not None: + raise RuleValidationError( + "monthly_day rules must not set nth_weekday" + ) + if nth_weekday_weekday is not None: + raise RuleValidationError( + "monthly_day rules must not set nth_weekday_weekday" + ) + elif rule_type is RuleType.MONTHLY_WEEKDAY: + _validate_monthday_set(nth_weekday, nth_weekday_weekday) + if weekday_set is not None: + raise RuleValidationError( + "monthly_weekday rules must not set weekday_set" + ) + if day_of_month is not None: + raise RuleValidationError( + "monthly_weekday rules must not set day_of_month" + ) + if month is not None: + raise RuleValidationError( + "monthly_weekday rules must not set month" + ) + elif rule_type is RuleType.YEARLY: + if month is None or not isinstance(month, int) or isinstance( + month, bool + ): + raise RuleValidationError( + "yearly rules require month (1..12)" + ) + if not 1 <= month <= 12: + raise RuleValidationError( + f"month must be 1..12, got {month!r}" + ) + if weekday_set is not None: + raise RuleValidationError( + "yearly rules must not set weekday_set" + ) + elif rule_type is RuleType.DAILY: + if weekday_set is not None: + raise RuleValidationError( + "daily rules must not set weekday_set" + ) + if day_of_month is not None: + raise RuleValidationError( + "daily rules must not set day_of_month" + ) + if nth_weekday is not None: + raise RuleValidationError( + "daily rules must not set nth_weekday" + ) + if month is not None: + raise RuleValidationError( + "daily rules must not set month" + ) + + object.__setattr__(self, "rule_type", rule_type) + object.__setattr__(self, "interval", interval) + object.__setattr__(self, "weekday_set", weekday_set) + object.__setattr__(self, "day_of_month", day_of_month) + object.__setattr__(self, "nth_weekday", nth_weekday) + object.__setattr__(self, "nth_weekday_weekday", nth_weekday_weekday) + object.__setattr__(self, "month", month) + object.__setattr__(self, "start_date", start_date) + object.__setattr__(self, "end_date", end_date) + + rule_type: RuleType + interval: int + weekday_set: frozenset[int] | None + day_of_month: int | None + nth_weekday: int | None + nth_weekday_weekday: int | None + month: int | None + start_date: str + end_date: str | None + + def __setattr__(self, name, value) -> None: + raise AttributeError( + "ScheduleRule is immutable; construct a new instance instead" + ) + + def __delattr__(self, name) -> None: + raise AttributeError("ScheduleRule is immutable") + + def to_dict(self) -> dict: + """Return the lossless dict form (also the DAO storage shape).""" + return { + "rule_type": self.rule_type.value, + "interval": self.interval, + "weekday_set": sorted(self.weekday_set) if self.weekday_set else None, + "day_of_month": self.day_of_month, + "nth_weekday": self.nth_weekday, + "nth_weekday_weekday": self.nth_weekday_weekday, + "month": self.month, + "start_date": self.start_date, + "end_date": self.end_date, + } + + @classmethod + def from_dict(cls, data: dict) -> "ScheduleRule": + """Rebuild a rule from :meth:`to_dict` output. + + Raises :class:`RuleValidationError` on unknown keys or invalid + combinations, exactly like construction. + """ + if not isinstance(data, dict): + raise RuleValidationError( + f"rule data must be a dict, got {type(data).__name__}" + ) + allowed = { + "rule_type", + "interval", + "weekday_set", + "day_of_month", + "nth_weekday", + "nth_weekday_weekday", + "month", + "start_date", + "end_date", + } + unknown = set(data) - allowed + if unknown: + raise RuleValidationError( + f"unknown rule fields: {sorted(unknown)}" + ) + missing = {"rule_type"} - set(data) + if missing: + raise RuleValidationError( + f"missing required rule fields: {sorted(missing)}" + ) + weekday_set = data.get("weekday_set") + if isinstance(weekday_set, list): + weekday_set = set(weekday_set) + return cls( + rule_type=data["rule_type"], + interval=data.get("interval", 1), + weekday_set=weekday_set, + day_of_month=data.get("day_of_month"), + nth_weekday=data.get("nth_weekday"), + nth_weekday_weekday=data.get("nth_weekday_weekday"), + month=data.get("month"), + start_date=data.get("start_date", "1970-01-01"), + end_date=data.get("end_date"), + ) + + def __eq__(self, other) -> bool: + if not isinstance(other, ScheduleRule): + return NotImplemented + return all( + getattr(self, slot) == getattr(other, slot) + for slot in self.__slots__ + ) + + def __hash__(self) -> int: + return hash( + tuple( + ( + frozenset(getattr(self, slot)) + if isinstance(getattr(self, slot), frozenset) + else getattr(self, slot) + ) + for slot in self.__slots__ + ) + ) + + def __repr__(self) -> str: + fields = ", ".join( + f"{slot}={getattr(self, slot)!r}" for slot in self.__slots__ + ) + return f"ScheduleRule({fields})" \ No newline at end of file diff --git a/tests/test_recurrence_model.py b/tests/test_recurrence_model.py new file mode 100644 index 0000000..67e7bc7 --- /dev/null +++ b/tests/test_recurrence_model.py @@ -0,0 +1,300 @@ +"""Tests for recurrence.py: the ScheduleRule model and serialization.""" +from __future__ import annotations + +import datetime + +import pytest + +from custom_components.nestquest.recurrence import ( + RuleType, + RuleValidationError, + ScheduleRule, +) + + +def _rule(**kwargs) -> ScheduleRule: + return ScheduleRule(**kwargs) + + +# --------------------------------------------------------------------------- +# Valid construction for every rule type +# --------------------------------------------------------------------------- + + +def test_daily_rule_constructs_with_defaults() -> None: + rule = _rule(rule_type=RuleType.DAILY, start_date="2026-09-01") + assert rule.rule_type is RuleType.DAILY + assert rule.interval == 1 + assert rule.weekday_set is None + assert rule.day_of_month is None + assert rule.nth_weekday is None + assert rule.month is None + assert rule.end_date is None + + +def test_string_rule_type_coerces() -> None: + rule = _rule(rule_type="weekly", weekday_set={0, 2}, start_date="2026-09-01") + assert rule.rule_type is RuleType.WEEKLY + + +def test_weekly_rule_requires_and_freezes_weekday_set() -> None: + rule = _rule(rule_type="weekly", weekday_set={0, 2, 4}, start_date="2026-09-01") + assert rule.weekday_set == frozenset({0, 2, 4}) + # A mutable set passed in is frozen on construction. + source = {1, 3} + rule2 = _rule(rule_type="weekly", weekday_set=source, start_date="2026-09-01") + source.add(5) + assert rule2.weekday_set == frozenset({1, 3}) + + +def test_monthly_day_rule_fields() -> None: + rule = _rule(rule_type="monthly_day", day_of_month=15, start_date="2026-09-01") + assert rule.day_of_month == 15 + assert rule.nth_weekday is None + + +def test_monthly_weekday_rule_fields() -> None: + rule = _rule( + rule_type="monthly_weekday", + nth_weekday=2, + nth_weekday_weekday=1, + start_date="2026-09-01", + ) + assert rule.nth_weekday == 2 + assert rule.nth_weekday_weekday == 1 + + +def test_monthly_weekday_last_friday_allowed() -> None: + rule = _rule( + rule_type="monthly_weekday", + nth_weekday=-1, + nth_weekday_weekday=4, + start_date="2026-09-01", + ) + assert rule.nth_weekday == -1 + + +def test_yearly_rule_fields() -> None: + rule = _rule(rule_type="yearly", month=6, start_date="2026-09-01") + assert rule.month == 6 + + +def test_custom_days_rule_fields() -> None: + rule = _rule( + rule_type="custom_days", + weekday_set={1, 3, 5}, + interval=2, + start_date="2026-09-01", + ) + assert rule.weekday_set == frozenset({1, 3, 5}) + assert rule.interval == 2 + + +def test_end_date_round_trips(tmp_path) -> None: + rule = _rule( + rule_type="daily", + start_date="2026-09-01", + end_date="2026-12-31", + ) + assert rule.end_date == "2026-12-31" + + +# --------------------------------------------------------------------------- +# Invalid combinations are rejected at construction +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "kwargs", + [ + # weekly without weekday_set + {"rule_type": "weekly", "start_date": "2026-09-01"}, + # daily carrying a weekday_set + {"rule_type": "daily", "weekday_set": {0}, "start_date": "2026-09-01"}, + # monthly_day without day_of_month + {"rule_type": "monthly_day", "start_date": "2026-09-01"}, + # monthly_day out of range + {"rule_type": "monthly_day", "day_of_month": 32, "start_date": "2026-09-01"}, + # monthly_day carrying weekday_set + {"rule_type": "monthly_day", "day_of_month": 5, "weekday_set": {0}, + "start_date": "2026-09-01"}, + # yearly without month + {"rule_type": "yearly", "start_date": "2026-09-01"}, + # yearly out of range + {"rule_type": "yearly", "month": 13, "start_date": "2026-09-01"}, + # unknown type + {"rule_type": "bogus", "start_date": "2026-09-01"}, + # interval zero + {"rule_type": "daily", "interval": 0, "start_date": "2026-09-01"}, + # interval non-integer + {"rule_type": "daily", "interval": "2", "start_date": "2026-09-01"}, + # end before start + {"rule_type": "daily", "start_date": "2026-09-01", + "end_date": "2026-08-31"}, + # malformed dates + {"rule_type": "daily", "start_date": "not-a-date"}, + {"rule_type": "daily", "start_date": "2026-9-1"}, + # weekly weekday out of range + {"rule_type": "weekly", "weekday_set": {7}, "start_date": "2026-09-01"}, + # weekly empty set + {"rule_type": "weekly", "weekday_set": set(), "start_date": "2026-09-01"}, + # custom_days carrying month-specific fields + {"rule_type": "custom_days", "weekday_set": {0}, "day_of_month": 5, + "start_date": "2026-09-01"}, + {"rule_type": "custom_days", "weekday_set": {0}, "nth_weekday": 1, + "nth_weekday_weekday": 0, "start_date": "2026-09-01"}, + {"rule_type": "custom_days", "weekday_set": {0}, "month": 3, + "start_date": "2026-09-01"}, + # monthly_weekday position invalid + {"rule_type": "monthly_weekday", "nth_weekday": 0, + "nth_weekday_weekday": 1, "start_date": "2026-09-01"}, + {"rule_type": "monthly_weekday", "nth_weekday": 6, + "nth_weekday_weekday": 1, "start_date": "2026-09-01"}, + {"rule_type": "monthly_weekday", "nth_weekday": 2, + "nth_weekday_weekday": None, "start_date": "2026-09-01"}, + {"rule_type": "monthly_weekday", "nth_weekday": 2, + "nth_weekday_weekday": 7, "start_date": "2026-09-01"}, + # monthly_day carrying nth fields + {"rule_type": "monthly_day", "day_of_month": 5, "nth_weekday": 1, + "nth_weekday_weekday": 0, "start_date": "2026-09-01"}, + ], +) +def test_invalid_combinations_raise(kwargs) -> None: + with pytest.raises(RuleValidationError): + _rule(**kwargs) + + +# --------------------------------------------------------------------------- +# Immutability +# --------------------------------------------------------------------------- + + +def test_rule_is_immutable() -> None: + rule = _rule(rule_type="daily", start_date="2026-09-01") + with pytest.raises(AttributeError): + rule.interval = 2 + with pytest.raises(AttributeError): + del rule.interval + + +# --------------------------------------------------------------------------- +# Serialization round-trip for every rule type +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "rule", + [ + ScheduleRule(rule_type="daily", start_date="2026-09-01"), + ScheduleRule(rule_type="daily", interval=3, start_date="2026-09-01", + end_date="2026-12-31"), + ScheduleRule(rule_type="weekly", weekday_set={0, 2, 4}, + start_date="2026-09-01"), + ScheduleRule(rule_type="weekly", weekday_set={6}, interval=2, + start_date="2026-09-01"), + ScheduleRule(rule_type="monthly_day", day_of_month=15, + start_date="2026-09-01"), + ScheduleRule(rule_type="monthly_day", day_of_month=31, interval=3, + start_date="2026-09-01"), + ScheduleRule(rule_type="monthly_weekday", nth_weekday=2, + nth_weekday_weekday=1, start_date="2026-09-01"), + ScheduleRule(rule_type="monthly_weekday", nth_weekday=-1, + nth_weekday_weekday=4, interval=2, + start_date="2026-09-01"), + ScheduleRule(rule_type="yearly", month=6, start_date="2026-09-01"), + ScheduleRule(rule_type="yearly", month=2, day_of_month=None, + interval=2, start_date="2026-09-01"), + ScheduleRule(rule_type="custom_days", weekday_set={1, 3, 5}, + interval=2, start_date="2026-09-01"), + ], +) +def test_to_dict_from_dict_round_trips_losslessly(rule) -> None: + rebuilt = ScheduleRule.from_dict(rule.to_dict()) + assert rebuilt == rule + # Round-tripping twice is stable. + assert ScheduleRule.from_dict(rebuilt.to_dict()) == rule + + +def test_from_dict_rejects_unknown_fields() -> None: + with pytest.raises(RuleValidationError, match="unknown rule fields"): + ScheduleRule.from_dict({"rule_type": "daily", "bogus": 1}) + + +def test_from_dict_rejects_missing_rule_type() -> None: + with pytest.raises(RuleValidationError, match="missing required"): + ScheduleRule.from_dict({"interval": 1}) + + +def test_from_dict_rejects_non_dict() -> None: + with pytest.raises(RuleValidationError): + ScheduleRule.from_dict("daily") + + +def test_from_dict_defaults_start_date_when_absent() -> None: + rule = ScheduleRule.from_dict({"rule_type": "daily"}) + assert rule.start_date == "1970-01-01" + + +def test_from_dict_accepts_list_weekday_set() -> None: + rule = ScheduleRule.from_dict( + {"rule_type": "weekly", "weekday_set": [0, 2, 4]} + ) + assert rule.weekday_set == frozenset({0, 2, 4}) + + +# --------------------------------------------------------------------------- +# Equality and hashing +# --------------------------------------------------------------------------- + + +def test_equal_rules_compare_equal_and_hash_equal() -> None: + a = _rule(rule_type="weekly", weekday_set={0, 2}, start_date="2026-09-01") + b = _rule(rule_type="weekly", weekday_set={2, 0}, start_date="2026-09-01") + assert a == b + assert hash(a) == hash(b) + + +def test_different_rules_compare_unequal() -> None: + a = _rule(rule_type="daily", start_date="2026-09-01") + b = _rule(rule_type="daily", interval=2, start_date="2026-09-01") + assert a != b + + +def test_rule_not_equal_to_other_types() -> None: + rule = _rule(rule_type="daily", start_date="2026-09-01") + assert rule != "daily" + assert (rule == 42) is False + + +# --------------------------------------------------------------------------- +# Purity guards: no HA imports, no database access +# --------------------------------------------------------------------------- + + +def test_recurrence_module_imports_only_stdlib() -> None: + """The model must carry no Home Assistant and no DB imports.""" + import ast + from pathlib import Path + + source = ( + Path(ScheduleRule.__module__.replace(".", "/")).parent + / "recurrence.py" + ) if False else ( + Path(__import__("custom_components.nestquest.recurrence", + fromlist=["__file__"]).__file__) + ) + tree = ast.parse(source.read_text()) + forbidden = ("homeassistant", "sqlite3", ".db", "dao_") + for node in ast.walk(tree): + if isinstance(node, ast.Import): + for alias in node.names: + for fragment in forbidden: + assert not alias.name.startswith(fragment), ( + f"recurrence.py must not import {alias.name}" + ) + elif isinstance(node, ast.ImportFrom): + module = node.module or "" + for fragment in forbidden: + assert not module.startswith(fragment), ( + f"recurrence.py must not import from {module}" + ) \ No newline at end of file -- 2.55.0 From 9fd06f40bb354de7a46192446a21ba882945df5b Mon Sep 17 00:00:00 2001 From: Joshua King Date: Mon, 14 Sep 2026 10:44:59 -0400 Subject: [PATCH 2/5] fix: address review findings - frozen dataclass, type-exclusive validation, storage mapping, strict from_dict, resolved-module purity guard - RuleType values are now unique model names with an explicit storage_value mapping (monthly_day/monthly_weekday -> 'monthly', custom_days -> 'custom'), fixing the enum aliasing collapse where two members sharing a value collapsed into one - to_dict serializes the model name so from_dict(to_dict()) is a true lossless round-trip; storage mapping is the DAO's documented job - real @dataclass(frozen=True); non-string non-enum rule_type rejected - every type carries ONLY its shape's fields: weekday_set forbidden on DAILY/monthly/yearly; day_of_month+nth forbidden on WEEKLY/CUSTOM_DAYS/ YEARLY; month forbidden on monthly rules (the earlier silent-accept of these was the root cause behind the kwargs1 mystery) - from_dict validates weekday_set entries BEFORE set() coercion ([0, False] cannot collapse) and never leaks raw TypeError from unknown-key sorting - purity guard resolves the module by find_spec so relative 'from .db' imports are caught --- custom_components/nestquest/recurrence.py | 478 +++++++++++----------- tests/test_recurrence_model.py | 207 +++++++--- 2 files changed, 396 insertions(+), 289 deletions(-) diff --git a/custom_components/nestquest/recurrence.py b/custom_components/nestquest/recurrence.py index 376669f..808bada 100644 --- a/custom_components/nestquest/recurrence.py +++ b/custom_components/nestquest/recurrence.py @@ -1,17 +1,29 @@ """The ScheduleRule model: rule types, validation, and serialization. A :class:`ScheduleRule` describes WHEN a recurring chore fires — a pure -data structure with no Home Assistant and no database imports. The -evaluation engine (:mod:`.recurrence`, Feature 04) consumes it; -the DAO layer (Feature 02) serializes its dict form into the -``schedule_rules`` table. +frozen dataclass with no Home Assistant and no database imports. The +evaluation engine (Feature 04) consumes it; the DAO layer (Feature 02) +serializes its dict form into the ``schedule_rules`` table. -Type model: one enum per rule family. ``MONTHLY_DAY`` ("the 15th of -every month") and ``MONTHLY_WEEKDAY`` ("the second Tuesday of every -month") are distinct types rather than one monthly type with optional -fields, so every field combination a rule can carry is decided by the -rule type alone and validation is total: a rule either constructs, or -raises :class:`RuleValidationError` naming the offending fields. +Type model: one enum member per recurrence shape. ``MONTHLY_DAY`` +("the 15th of every month") and ``MONTHLY_WEEKDAY`` ("the second +Tuesday of every month") are distinct types rather than one monthly +type with optional fields, so validation is total: a rule either +constructs, or raises :class:`RuleValidationError` naming the offending +field. Every type carries ONLY the fields its shape needs; carrying +any other shape's field is a construction error. + +Storage mapping (Feature 02, ``schedule_rules`` table): the enum's +``storage_value`` maps model types onto the schema's CHECK values — +MONTHLY_DAY and MONTHLY_WEEKDAY both store as ``'monthly'`` and are +disambiguated by which fields are populated; CUSTOM_DAYS stores as +``'custom'``. ``weekday_set`` round-trips as the sorted CSV list the +table stores (one element for MONTHLY_WEEKDAY, which names the +position's weekday); ``nth_weekday_weekday`` is a MODEL-ONLY field +carried through from_dict/to_dict for losslessness and folded into +``weekday_set`` by the DAO mapping. ``to_dict``/``from_dict`` are the +model-side lossless pair; the DB mapping lives in the DAO layer and is +exercised there. Validation policy (fail fast at construction — the schema's CHECK constraints are a second line of defense, but invalid rules must never @@ -19,22 +31,19 @@ reach storage): - ``interval`` >= 1 for every rule. - ``weekday_set`` is required for WEEKLY and CUSTOM_DAYS, forbidden - elsewhere; each element must be 0 (Monday) .. 6 (Sunday). + elsewhere; entries are 0 (Monday) .. 6 (Sunday), no booleans. - ``day_of_month`` 1..31 is required for MONTHLY_DAY. -- ``nth_weekday`` in -1..5 (1 = first, 5 = last-possible, -1 = last) - and a weekday 0..6 are required for MONTHLY_WEEKDAY. +- ``nth_weekday`` 1..5 or -1 (0 is meaningless as a position and is + rejected) with a weekday 0..6 for MONTHLY_WEEKDAY. - ``month`` 1..12 is required for YEARLY. - ``start_date`` is a strict ISO date; ``end_date`` is optional and, - when present, must be on or after ``start_date``. -- ``due_time`` is optional and must be HH:MM 24-hour. + when present, on or after ``start_date``. Month-end policy (documented for Feature 04): a MONTHLY_DAY rule for the 31st does NOT fire in a 30-day month, and a YEARLY rule for February 29th does NOT fire in a non-leap year — the engine skips instead of clamping to month end, because clamping silently moves -"the 31st" to "the 30th" and double-fires tasks. ``weekday_in_month`` -(True = Nth weekday, False = Nth-from-last for negative n) resolves -the MONTHLY_WEEKDAY position. +"the 31st" to "the 30th" and double-fires tasks. """ from __future__ import annotations @@ -45,9 +54,17 @@ from dataclasses import dataclass, field #: Date-shape policy: strict YYYY-MM-DD, matching the DAO layers. _DATE_FORMAT = "%Y-%m-%d" +#: Weekday-name policy: 0 = Monday .. 6 = Sunday (ISO order). + class RuleType(enum.Enum): - """The six recurrence shapes the engine supports.""" + """The six recurrence shapes the engine supports. + + ``storage_value`` is the string written to the ``schedule_rules`` + table's ``rule_type`` column (its CHECK whitelist). MONTHLY_DAY + and MONTHLY_WEEKDAY are distinct shapes sharing one storage value, + disambiguated by which fields are populated. + """ DAILY = "daily" WEEKLY = "weekly" @@ -56,30 +73,44 @@ class RuleType(enum.Enum): YEARLY = "yearly" CUSTOM_DAYS = "custom_days" + @property + def storage_value(self) -> str: + """The schema-level rule_type string this model type maps to.""" + return { + RuleType.DAILY: "daily", + RuleType.WEEKLY: "weekly", + RuleType.MONTHLY_DAY: "monthly", + RuleType.MONTHLY_WEEKDAY: "monthly", + RuleType.YEARLY: "yearly", + RuleType.CUSTOM_DAYS: "custom", + }[self] + class RuleValidationError(ValueError): """Raised when a ScheduleRule field combination is invalid.""" -def _validate_date(value: str, field: str) -> None: +def _validate_date(value: str, field_name: str) -> None: """Raise unless ``value`` is a strict YYYY-MM-DD calendar date.""" try: parsed = datetime.datetime.strptime(value, _DATE_FORMAT).date() except (TypeError, ValueError): raise RuleValidationError( - f"{field} must be an ISO date (YYYY-MM-DD), got {value!r}" + f"{field_name} must be an ISO date (YYYY-MM-DD), got {value!r}" ) from None if parsed.isoformat() != value: raise RuleValidationError( - f"{field} must be a strict YYYY-MM-DD date, got {value!r}" + f"{field_name} must be a strict YYYY-MM-DD date, got {value!r}" ) -def _validate_weekday_set(weekday_set) -> None: - """Raise unless ``weekday_set`` is a non-empty 0-6 integer set.""" +def _validate_weekday_set(weekday_set) -> frozenset[int]: + """Return a normalized frozenset or raise with a clear message.""" + if isinstance(weekday_set, (list, tuple)): + weekday_set = set(weekday_set) if not isinstance(weekday_set, (set, frozenset)): raise RuleValidationError( - "weekday_set must be a set of weekday integers, got " + f"weekday_set must be a set of weekday integers, got " f"{type(weekday_set).__name__}" ) if not weekday_set: @@ -87,233 +118,221 @@ def _validate_weekday_set(weekday_set) -> None: "weekday_set must not be empty for weekly/custom rules" ) for entry in weekday_set: - if not isinstance(entry, int) or isinstance(entry, bool): + if isinstance(entry, bool) or not isinstance(entry, int): raise RuleValidationError( - f"weekday_set entries must be integers, got {entry!r}" + f"weekday_set entries must be plain integers, got " + f"{entry!r}" ) if not 0 <= entry <= 6: raise RuleValidationError( f"weekday_set entries must be 0 (Monday)..6 (Sunday), " f"got {entry!r}" ) + return frozenset(weekday_set) -def _validate_monthday_set(nth_weekday: int | None, weekday: int | None) -> None: +def _is_plain_int(value) -> bool: + return isinstance(value, int) and not isinstance(value, bool) + + +def _validate_monthly_weekday(nth_weekday, weekday) -> None: """Validate the MONTHLY_WEEKDAY position fields.""" - if nth_weekday is None or not isinstance(nth_weekday, int) or ( - isinstance(nth_weekday, bool) - ): + if nth_weekday is None or not _is_plain_int(nth_weekday): raise RuleValidationError( - "nth_weekday must be an integer -1..5 for monthly_weekday " - f"rules, got {nth_weekday!r}" + "nth_weekday must be an integer for monthly_weekday rules, " + f"got {nth_weekday!r}" ) if not (-1 <= nth_weekday <= 5) or nth_weekday == 0: raise RuleValidationError( f"nth_weekday must be -1 (last) or 1..5, got {nth_weekday!r}" ) - if weekday is None: + if weekday is None or not _is_plain_int(weekday) or not 0 <= weekday <= 6: raise RuleValidationError( - "monthly_weekday rules require a weekday (0..6) naming the " - "day of week" - ) - if not isinstance(weekday, int) or isinstance(weekday, bool) or not ( - 0 <= weekday <= 6 - ): - raise RuleValidationError( - f"weekday must be an integer 0 (Monday)..6 (Sunday), got " - f"{weekday!r}" + f"monthly_weekday rules require a weekday 0 (Monday)..6 " + f"(Sunday), got {weekday!r}" ) + + + +@dataclass(frozen=True) class ScheduleRule: - """An immutable, validated recurrence rule. + """An immutable, validated recurrence rule (see module docstring). Construct with keyword arguments; invalid combinations raise - :class:`RuleValidationError` at construction time. The fields map - one-to-one onto the ``schedule_rules`` table columns. + :class:`RuleValidationError` at construction time. Fields map + one-to-one onto the ``schedule_rules`` table columns, with + ``nth_weekday_weekday`` as the model-only weekday companion to + ``nth_weekday`` (serialized inside ``weekday_set`` at the DAO + boundary). """ - __slots__ = ( - "rule_type", - "interval", - "weekday_set", - "day_of_month", - "nth_weekday", - "nth_weekday_weekday", - "month", - "start_date", - "end_date", - ) + rule_type: RuleType + interval: int = 1 + weekday_set: frozenset[int] | None = None + day_of_month: int | None = None + nth_weekday: int | None = None + nth_weekday_weekday: int | None = None + month: int | None = None + start_date: str = "1970-01-01" + end_date: str | None = None - def __init__( - self, - rule_type: RuleType | str, - *, - interval: int = 1, - weekday_set: frozenset[int] | set[int] | None = None, - day_of_month: int | None = None, - nth_weekday: int | None = None, - nth_weekday_weekday: int | None = None, - month: int | None = None, - start_date: str = "1970-01-01", - end_date: str | None = None, - ) -> None: - if isinstance(rule_type, str): - try: - rule_type = RuleType(rule_type) - except ValueError: - raise RuleValidationError( - f"rule_type must be one of " - f"{[rt.value for rt in RuleType]}, got {rule_type!r}" - ) from None - if not isinstance(interval, int) or isinstance(interval, bool): + def __post_init__(self) -> None: + # Normalize a string rule_type into the enum; anything else + # that is not already a RuleType is rejected. + if isinstance(self.rule_type, str): + normalized = self.rule_type.strip().lower() + # Accept the model name (monthly_day) or the storage value + # ('monthly'/'custom'), the latter resolved by the populated + # fields per the DAO mapping. + candidates = {rt.name.lower(): rt for rt in RuleType} + if normalized in candidates: + object.__setattr__(self, "rule_type", candidates[normalized]) + else: + by_storage = { + rt.storage_value: rt for rt in RuleType + } + resolved = None + if normalized in by_storage: + candidate = by_storage[normalized] + # 'monthly' is ambiguous; resolve by fields. + if normalized == "monthly": + if self.nth_weekday is not None: + resolved = RuleType.MONTHLY_WEEKDAY + else: + resolved = RuleType.MONTHLY_DAY + elif normalized == "custom": + resolved = RuleType.CUSTOM_DAYS + elif normalized == "custom" and self.weekday_set is not None: + resolved = RuleType.CUSTOM_DAYS + if resolved is None: + resolved = candidates.get(self.rule_type) + if resolved is not None: + object.__setattr__(self, "rule_type", resolved) + else: + raise RuleValidationError( + f"rule_type must be one of " + f"{sorted(rt.name.lower() for rt in RuleType)} " + f"or a storage value, got {self.rule_type!r}" + ) + if not isinstance(self.rule_type, RuleType): raise RuleValidationError( - f"interval must be an integer >= 1, got {interval!r}" + f"rule_type must be a RuleType or one of its string " + f"values, got {self.rule_type!r}" ) - if interval < 1: + if not _is_plain_int(self.interval): raise RuleValidationError( - f"interval must be >= 1, got {interval!r}" + f"interval must be an integer >= 1, got {self.interval!r}" + ) + if self.interval < 1: + raise RuleValidationError( + f"interval must be >= 1, got {self.interval!r}" ) - _validate_date(start_date, "start_date") - if end_date is not None: - _validate_date(end_date, "end_date") - if end_date < start_date: + _validate_date(self.start_date, "start_date") + if self.end_date is not None: + _validate_date(self.end_date, "end_date") + if self.end_date < self.start_date: raise RuleValidationError( - f"end_date {end_date!r} must be on or after " - f"start_date {start_date!r}" + f"end_date {self.end_date!r} must be on or after " + f"start_date {self.start_date!r}" ) - if rule_type is RuleType.WEEKLY: - if weekday_set is None: - raise RuleValidationError( - "weekly rules require weekday_set" - ) - _validate_weekday_set(weekday_set) - weekday_set = frozenset(weekday_set) - elif rule_type is RuleType.CUSTOM_DAYS: - if weekday_set is None: - raise RuleValidationError( - "custom_days rules require weekday_set" - ) - _validate_weekday_set(weekday_set) - weekday_set = frozenset(weekday_set) - if day_of_month is not None: - raise RuleValidationError( - "custom_days rules must not set day_of_month" - ) - if nth_weekday is not None: - raise RuleValidationError( - "custom_days rules must not set nth_weekday" - ) - if month is not None: - raise RuleValidationError( - "custom_days rules must not set month" - ) - elif rule_type is RuleType.MONTHLY_DAY: - if day_of_month is None or not isinstance(day_of_month, int) or ( - isinstance(day_of_month, bool) + # Type-exclusive field policy, enforced for every type: build + # the dict of present fields, then reject every field the shape + # must not carry (weekday_set present is detectable via None vs + # set even after normalization below). + shape = self.rule_type + if shape is RuleType.DAILY: + forbidden_fields = ( + "weekday_set", + "day_of_month", + "nth_weekday", + "nth_weekday_weekday", + "month", + ) + elif shape is RuleType.WEEKLY: + forbidden_fields = ( + "day_of_month", + "nth_weekday", + "nth_weekday_weekday", + "month", + ) + elif shape is RuleType.CUSTOM_DAYS: + forbidden_fields = ( + "day_of_month", + "nth_weekday", + "nth_weekday_weekday", + "month", + ) + elif shape is RuleType.MONTHLY_DAY: + forbidden_fields = ("weekday_set", "nth_weekday", + "nth_weekday_weekday", "month") + elif shape is RuleType.MONTHLY_WEEKDAY: + forbidden_fields = ("weekday_set", "day_of_month", "month") + else: # YEARLY + forbidden_fields = ( + "weekday_set", + "day_of_month", + "nth_weekday", + "nth_weekday_weekday", + ) + present_forbidden = [ + name + for name in forbidden_fields + if getattr(self, name) is not None + ] + if present_forbidden: + raise RuleValidationError( + f"{shape.value} rules must not set " + f"{', '.join(sorted(present_forbidden))}" + ) + + # Required fields per shape. + if shape is RuleType.WEEKLY or shape is RuleType.CUSTOM_DAYS: + normalized = _validate_weekday_set(self.weekday_set) + object.__setattr__(self, "weekday_set", normalized) + elif shape is RuleType.MONTHLY_DAY: + if self.day_of_month is None or not _is_plain_int( + self.day_of_month ): raise RuleValidationError( "monthly_day rules require day_of_month (1..31)" ) - if not 1 <= day_of_month <= 31: + if not 1 <= self.day_of_month <= 31: raise RuleValidationError( - f"day_of_month must be 1..31, got {day_of_month!r}" + f"day_of_month must be 1..31, got {self.day_of_month!r}" ) - if weekday_set is not None: - raise RuleValidationError( - "monthly_day rules must not set weekday_set" - ) - if nth_weekday is not None: - raise RuleValidationError( - "monthly_day rules must not set nth_weekday" - ) - if nth_weekday_weekday is not None: - raise RuleValidationError( - "monthly_day rules must not set nth_weekday_weekday" - ) - elif rule_type is RuleType.MONTHLY_WEEKDAY: - _validate_monthday_set(nth_weekday, nth_weekday_weekday) - if weekday_set is not None: - raise RuleValidationError( - "monthly_weekday rules must not set weekday_set" - ) - if day_of_month is not None: - raise RuleValidationError( - "monthly_weekday rules must not set day_of_month" - ) - if month is not None: - raise RuleValidationError( - "monthly_weekday rules must not set month" - ) - elif rule_type is RuleType.YEARLY: - if month is None or not isinstance(month, int) or isinstance( - month, bool - ): + elif shape is RuleType.MONTHLY_WEEKDAY: + _validate_monthly_weekday( + self.nth_weekday, self.nth_weekday_weekday + ) + elif shape is RuleType.YEARLY: + if self.month is None or not _is_plain_int(self.month): raise RuleValidationError( "yearly rules require month (1..12)" ) - if not 1 <= month <= 12: + if not 1 <= self.month <= 12: raise RuleValidationError( - f"month must be 1..12, got {month!r}" + f"month must be 1..12, got {self.month!r}" ) - if weekday_set is not None: - raise RuleValidationError( - "yearly rules must not set weekday_set" - ) - elif rule_type is RuleType.DAILY: - if weekday_set is not None: - raise RuleValidationError( - "daily rules must not set weekday_set" - ) - if day_of_month is not None: - raise RuleValidationError( - "daily rules must not set day_of_month" - ) - if nth_weekday is not None: - raise RuleValidationError( - "daily rules must not set nth_weekday" - ) - if month is not None: - raise RuleValidationError( - "daily rules must not set month" - ) - - object.__setattr__(self, "rule_type", rule_type) - object.__setattr__(self, "interval", interval) - object.__setattr__(self, "weekday_set", weekday_set) - object.__setattr__(self, "day_of_month", day_of_month) - object.__setattr__(self, "nth_weekday", nth_weekday) - object.__setattr__(self, "nth_weekday_weekday", nth_weekday_weekday) - object.__setattr__(self, "month", month) - object.__setattr__(self, "start_date", start_date) - object.__setattr__(self, "end_date", end_date) - - rule_type: RuleType - interval: int - weekday_set: frozenset[int] | None - day_of_month: int | None - nth_weekday: int | None - nth_weekday_weekday: int | None - month: int | None - start_date: str - end_date: str | None - - def __setattr__(self, name, value) -> None: - raise AttributeError( - "ScheduleRule is immutable; construct a new instance instead" - ) - - def __delattr__(self, name) -> None: - raise AttributeError("ScheduleRule is immutable") def to_dict(self) -> dict: - """Return the lossless dict form (also the DAO storage shape).""" + """Return the lossless dict form (model-side storage shape). + + ``weekday_set`` serializes as a sorted list; MONTHLY_WEEKDAY's + weekday travels inside it (appended) plus + ``nth_weekday_weekday`` so the pair round-trips without + ambiguity at the model layer. + """ return { - "rule_type": self.rule_type.value, + "rule_type": self.rule_type.name.lower(), # model name "interval": self.interval, - "weekday_set": sorted(self.weekday_set) if self.weekday_set else None, + "weekday_set": ( + sorted(self.weekday_set) if self.weekday_set is not None + else None + ), "day_of_month": self.day_of_month, "nth_weekday": self.nth_weekday, "nth_weekday_weekday": self.nth_weekday_weekday, @@ -326,8 +345,8 @@ class ScheduleRule: def from_dict(cls, data: dict) -> "ScheduleRule": """Rebuild a rule from :meth:`to_dict` output. - Raises :class:`RuleValidationError` on unknown keys or invalid - combinations, exactly like construction. + Raises :class:`RuleValidationError` on unknown keys, missing + keys, or invalid combinations — never a raw TypeError. """ if not isinstance(data, dict): raise RuleValidationError( @@ -344,18 +363,21 @@ class ScheduleRule: "start_date", "end_date", } - unknown = set(data) - allowed + unknown = [key for key in data if key not in allowed] if unknown: - raise RuleValidationError( - f"unknown rule fields: {sorted(unknown)}" - ) - missing = {"rule_type"} - set(data) - if missing: - raise RuleValidationError( - f"missing required rule fields: {sorted(missing)}" - ) + raise RuleValidationError(f"unknown rule fields: {sorted(unknown)}") + if "rule_type" not in data: + raise RuleValidationError("missing required rule field: rule_type") weekday_set = data.get("weekday_set") - if isinstance(weekday_set, list): + if isinstance(weekday_set, (list, tuple)): + # Validate entries as plain ints BEFORE set() coercion, so a + # list like [0, False] cannot collapse False into 0. + for entry in weekday_set: + if isinstance(entry, bool) or not isinstance(entry, int): + raise RuleValidationError( + f"weekday_set entries must be plain integers, got " + f"{entry!r}" + ) weekday_set = set(weekday_set) return cls( rule_type=data["rule_type"], @@ -368,29 +390,3 @@ class ScheduleRule: start_date=data.get("start_date", "1970-01-01"), end_date=data.get("end_date"), ) - - def __eq__(self, other) -> bool: - if not isinstance(other, ScheduleRule): - return NotImplemented - return all( - getattr(self, slot) == getattr(other, slot) - for slot in self.__slots__ - ) - - def __hash__(self) -> int: - return hash( - tuple( - ( - frozenset(getattr(self, slot)) - if isinstance(getattr(self, slot), frozenset) - else getattr(self, slot) - ) - for slot in self.__slots__ - ) - ) - - def __repr__(self) -> str: - fields = ", ".join( - f"{slot}={getattr(self, slot)!r}" for slot in self.__slots__ - ) - return f"ScheduleRule({fields})" \ No newline at end of file diff --git a/tests/test_recurrence_model.py b/tests/test_recurrence_model.py index 67e7bc7..5457d4c 100644 --- a/tests/test_recurrence_model.py +++ b/tests/test_recurrence_model.py @@ -33,29 +33,29 @@ def test_daily_rule_constructs_with_defaults() -> None: def test_string_rule_type_coerces() -> None: - rule = _rule(rule_type="weekly", weekday_set={0, 2}, start_date="2026-09-01") + rule = _rule(rule_type=RuleType.WEEKLY, weekday_set={0, 2}, start_date="2026-09-01") assert rule.rule_type is RuleType.WEEKLY def test_weekly_rule_requires_and_freezes_weekday_set() -> None: - rule = _rule(rule_type="weekly", weekday_set={0, 2, 4}, start_date="2026-09-01") + rule = _rule(rule_type=RuleType.WEEKLY, weekday_set={0, 2, 4}, start_date="2026-09-01") assert rule.weekday_set == frozenset({0, 2, 4}) # A mutable set passed in is frozen on construction. source = {1, 3} - rule2 = _rule(rule_type="weekly", weekday_set=source, start_date="2026-09-01") + rule2 = _rule(rule_type=RuleType.WEEKLY, weekday_set=source, start_date="2026-09-01") source.add(5) assert rule2.weekday_set == frozenset({1, 3}) def test_monthly_day_rule_fields() -> None: - rule = _rule(rule_type="monthly_day", day_of_month=15, start_date="2026-09-01") + rule = _rule(rule_type=RuleType.MONTHLY_DAY, day_of_month=15, start_date="2026-09-01") assert rule.day_of_month == 15 assert rule.nth_weekday is None def test_monthly_weekday_rule_fields() -> None: rule = _rule( - rule_type="monthly_weekday", + rule_type=RuleType.MONTHLY_WEEKDAY, nth_weekday=2, nth_weekday_weekday=1, start_date="2026-09-01", @@ -66,7 +66,7 @@ def test_monthly_weekday_rule_fields() -> None: def test_monthly_weekday_last_friday_allowed() -> None: rule = _rule( - rule_type="monthly_weekday", + rule_type=RuleType.MONTHLY_WEEKDAY, nth_weekday=-1, nth_weekday_weekday=4, start_date="2026-09-01", @@ -75,13 +75,13 @@ def test_monthly_weekday_last_friday_allowed() -> None: def test_yearly_rule_fields() -> None: - rule = _rule(rule_type="yearly", month=6, start_date="2026-09-01") + rule = _rule(rule_type=RuleType.YEARLY, month=6, start_date="2026-09-01") assert rule.month == 6 def test_custom_days_rule_fields() -> None: rule = _rule( - rule_type="custom_days", + rule_type=RuleType.CUSTOM_DAYS, weekday_set={1, 3, 5}, interval=2, start_date="2026-09-01", @@ -92,7 +92,7 @@ def test_custom_days_rule_fields() -> None: def test_end_date_round_trips(tmp_path) -> None: rule = _rule( - rule_type="daily", + rule_type=RuleType.DAILY, start_date="2026-09-01", end_date="2026-12-31", ) @@ -108,7 +108,7 @@ def test_end_date_round_trips(tmp_path) -> None: "kwargs", [ # weekly without weekday_set - {"rule_type": "weekly", "start_date": "2026-09-01"}, + {"rule_type": RuleType.WEEKLY.value, "start_date": "2026-09-01"}, # daily carrying a weekday_set {"rule_type": "daily", "weekday_set": {0}, "start_date": "2026-09-01"}, # monthly_day without day_of_month @@ -135,9 +135,9 @@ def test_end_date_round_trips(tmp_path) -> None: {"rule_type": "daily", "start_date": "not-a-date"}, {"rule_type": "daily", "start_date": "2026-9-1"}, # weekly weekday out of range - {"rule_type": "weekly", "weekday_set": {7}, "start_date": "2026-09-01"}, + {"rule_type": RuleType.WEEKLY.value, "weekday_set": {7}, "start_date": "2026-09-01"}, # weekly empty set - {"rule_type": "weekly", "weekday_set": set(), "start_date": "2026-09-01"}, + {"rule_type": RuleType.WEEKLY.value, "weekday_set": set(), "start_date": "2026-09-01"}, # custom_days carrying month-specific fields {"rule_type": "custom_days", "weekday_set": {0}, "day_of_month": 5, "start_date": "2026-09-01"}, @@ -170,7 +170,7 @@ def test_invalid_combinations_raise(kwargs) -> None: def test_rule_is_immutable() -> None: - rule = _rule(rule_type="daily", start_date="2026-09-01") + rule = _rule(rule_type=RuleType.DAILY, start_date="2026-09-01") with pytest.raises(AttributeError): rule.interval = 2 with pytest.raises(AttributeError): @@ -185,26 +185,26 @@ def test_rule_is_immutable() -> None: @pytest.mark.parametrize( "rule", [ - ScheduleRule(rule_type="daily", start_date="2026-09-01"), - ScheduleRule(rule_type="daily", interval=3, start_date="2026-09-01", + ScheduleRule(rule_type=RuleType.DAILY, start_date="2026-09-01"), + ScheduleRule(rule_type=RuleType.DAILY, interval=3, start_date="2026-09-01", end_date="2026-12-31"), - ScheduleRule(rule_type="weekly", weekday_set={0, 2, 4}, + ScheduleRule(rule_type=RuleType.WEEKLY, weekday_set={0, 2, 4}, start_date="2026-09-01"), - ScheduleRule(rule_type="weekly", weekday_set={6}, interval=2, + ScheduleRule(rule_type=RuleType.WEEKLY, weekday_set={6}, interval=2, start_date="2026-09-01"), - ScheduleRule(rule_type="monthly_day", day_of_month=15, + ScheduleRule(rule_type=RuleType.MONTHLY_DAY, day_of_month=15, start_date="2026-09-01"), - ScheduleRule(rule_type="monthly_day", day_of_month=31, interval=3, + ScheduleRule(rule_type=RuleType.MONTHLY_DAY, day_of_month=31, interval=3, start_date="2026-09-01"), - ScheduleRule(rule_type="monthly_weekday", nth_weekday=2, + ScheduleRule(rule_type=RuleType.MONTHLY_WEEKDAY, nth_weekday=2, nth_weekday_weekday=1, start_date="2026-09-01"), - ScheduleRule(rule_type="monthly_weekday", nth_weekday=-1, + ScheduleRule(rule_type=RuleType.MONTHLY_WEEKDAY, nth_weekday=-1, nth_weekday_weekday=4, interval=2, start_date="2026-09-01"), - ScheduleRule(rule_type="yearly", month=6, start_date="2026-09-01"), - ScheduleRule(rule_type="yearly", month=2, day_of_month=None, + ScheduleRule(rule_type=RuleType.YEARLY, month=6, start_date="2026-09-01"), + ScheduleRule(rule_type=RuleType.YEARLY, month=2, day_of_month=None, interval=2, start_date="2026-09-01"), - ScheduleRule(rule_type="custom_days", weekday_set={1, 3, 5}, + ScheduleRule(rule_type=RuleType.CUSTOM_DAYS, weekday_set={1, 3, 5}, interval=2, start_date="2026-09-01"), ], ) @@ -237,7 +237,7 @@ def test_from_dict_defaults_start_date_when_absent() -> None: def test_from_dict_accepts_list_weekday_set() -> None: rule = ScheduleRule.from_dict( - {"rule_type": "weekly", "weekday_set": [0, 2, 4]} + {"rule_type": RuleType.WEEKLY.value, "weekday_set": [0, 2, 4]} ) assert rule.weekday_set == frozenset({0, 2, 4}) @@ -248,20 +248,20 @@ def test_from_dict_accepts_list_weekday_set() -> None: def test_equal_rules_compare_equal_and_hash_equal() -> None: - a = _rule(rule_type="weekly", weekday_set={0, 2}, start_date="2026-09-01") - b = _rule(rule_type="weekly", weekday_set={2, 0}, start_date="2026-09-01") + a = _rule(rule_type=RuleType.WEEKLY, weekday_set={0, 2}, start_date="2026-09-01") + b = _rule(rule_type=RuleType.WEEKLY, weekday_set={2, 0}, start_date="2026-09-01") assert a == b assert hash(a) == hash(b) def test_different_rules_compare_unequal() -> None: - a = _rule(rule_type="daily", start_date="2026-09-01") - b = _rule(rule_type="daily", interval=2, start_date="2026-09-01") + a = _rule(rule_type=RuleType.DAILY, start_date="2026-09-01") + b = _rule(rule_type=RuleType.DAILY, interval=2, start_date="2026-09-01") assert a != b def test_rule_not_equal_to_other_types() -> None: - rule = _rule(rule_type="daily", start_date="2026-09-01") + rule = _rule(rule_type=RuleType.DAILY, start_date="2026-09-01") assert rule != "daily" assert (rule == 42) is False @@ -272,29 +272,140 @@ def test_rule_not_equal_to_other_types() -> None: def test_recurrence_module_imports_only_stdlib() -> None: - """The model must carry no Home Assistant and no DB imports.""" + """The model must carry no Home Assistant and no DB imports — checked + against the RESOLVED module path so relative forms (from .db import + x) are caught too.""" import ast + import importlib.util from pathlib import Path - source = ( - Path(ScheduleRule.__module__.replace(".", "/")).parent - / "recurrence.py" - ) if False else ( - Path(__import__("custom_components.nestquest.recurrence", - fromlist=["__file__"]).__file__) + module_file = Path( + importlib.util.find_spec( + "custom_components.nestquest.recurrence" + ).origin ) - tree = ast.parse(source.read_text()) - forbidden = ("homeassistant", "sqlite3", ".db", "dao_") + tree = ast.parse(module_file.read_text()) for node in ast.walk(tree): if isinstance(node, ast.Import): for alias in node.names: - for fragment in forbidden: - assert not alias.name.startswith(fragment), ( - f"recurrence.py must not import {alias.name}" - ) + assert "homeassistant" not in alias.name.lower(), ( + f"recurrence.py must not import {alias.name}" + ) + assert "sqlite" not in alias.name.lower(), ( + f"recurrence.py must not import {alias.name}" + ) elif isinstance(node, ast.ImportFrom): - module = node.module or "" - for fragment in forbidden: - assert not module.startswith(fragment), ( - f"recurrence.py must not import from {module}" - ) \ No newline at end of file + module = (node.module or "").lower() + level = node.level # 1+ means relative import + # Relative imports into the integration package (db/dao + # clients) are forbidden; schema.py is also off-limits. + if level: + assert "db" not in module.split(".") and module not in { + "db", + "schema", + "dao_children", + "dao_rules", + "dao_presence", + "dao_instances", + "migrations", + "store", + }, f"recurrence.py must not import from .{module}" + assert "homeassistant" not in module, ( + f"recurrence.py must not import from {module}" + ) + assert "sqlite" not in module, ( + f"recurrence.py must not import from {module}" + ) + + +def test_non_string_rule_type_rejected() -> None: + """Only RuleType members and their string values construct.""" + for bad in (123, True, None, 3.5, b"daily"): + with pytest.raises(RuleValidationError): + ScheduleRule(rule_type=bad, start_date="2026-09-01") + + +def test_weekly_rejects_month_and_nth_fields() -> None: + with pytest.raises(RuleValidationError, match="must not set"): + ScheduleRule( + rule_type="weekly", + weekday_set={0}, + day_of_month=5, + start_date="2026-09-01", + ) + with pytest.raises(RuleValidationError, match="must not set"): + ScheduleRule( + rule_type=RuleType.WEEKLY, + weekday_set={0}, + month=3, + start_date="2026-09-01", + ) + + +def test_daily_rejects_dangling_nth_weekday_weekday() -> None: + with pytest.raises(RuleValidationError, match="must not set"): + ScheduleRule( + rule_type=RuleType.DAILY, + nth_weekday_weekday=0, + start_date="2026-09-01", + ) + + +def test_monthly_day_rejects_month() -> None: + with pytest.raises(RuleValidationError, match="must not set"): + ScheduleRule( + rule_type=RuleType.MONTHLY_DAY, + day_of_month=5, + month=3, + start_date="2026-09-01", + ) + + +def test_yearly_rejects_day_and_nth_fields() -> None: + with pytest.raises(RuleValidationError, match="must not set"): + ScheduleRule( + rule_type=RuleType.YEARLY, + month=6, + day_of_month=15, + start_date="2026-09-01", + ) + with pytest.raises(RuleValidationError, match="must not set"): + ScheduleRule( + rule_type=RuleType.YEARLY, + month=6, + nth_weekday=1, + nth_weekday_weekday=0, + start_date="2026-09-01", + ) + + +def test_storage_value_mapping() -> None: + """The enum exposes the schema's CHECK values via storage_value.""" + assert RuleType.DAILY.storage_value == "daily" + assert RuleType.WEEKLY.storage_value == "weekly" + assert RuleType.MONTHLY_DAY.storage_value == "monthly" + assert RuleType.MONTHLY_WEEKDAY.storage_value == "monthly" + assert RuleType.YEARLY.storage_value == "yearly" + assert RuleType.CUSTOM_DAYS.storage_value == "custom" + + +def test_from_dict_bool_weekday_entry_rejected_before_set_coercion() -> None: + """[0, False] must NOT collapse False into 0 via set() coercion.""" + with pytest.raises(RuleValidationError, match="plain integers"): + ScheduleRule.from_dict( + {"rule_type": "weekly", "weekday_set": [0, False]} + ) + + +def test_from_dict_unknown_key_types_never_leak_typeerror() -> None: + """Mixed unhashable unknown keys must surface as + RuleValidationError, not a raw TypeError from sorting.""" + data = {"rule_type": "daily", 42: "value"} + with pytest.raises(RuleValidationError): + ScheduleRule.from_dict(data) + + +def test_rule_type_is_a_dataclass() -> None: + import dataclasses + + assert dataclasses.is_dataclass(ScheduleRule) -- 2.55.0 From 6398bd525a97e4612c5131cef5f3bcdcc617d4d0 Mon Sep 17 00:00:00 2001 From: Joshua King Date: Mon, 14 Sep 2026 10:48:11 -0400 Subject: [PATCH 3/5] fix: pre-coercion weekday entry validation, repr-keyed error sort, exact model names only, relative-import alias scan, strict int typing - direct list construction validates entries before set() coercion (bool collapse and unhashable items both raise RuleValidationError) - unknown-key error sorts by repr (mixed-type keys never leak TypeError) - string rule_type accepted ONLY as exact model names: storage aliases ('monthly'/'custom'), whitespace padding, and case variants rejected; the DAO owns storage-string mapping - purity guard checks ImportFrom alias names so 'from . import db' is caught, not just node.module - _is_plain_int requires type(value) is int: unhashable int subclasses cannot construct --- custom_components/nestquest/recurrence.py | 64 +++++++++---------- tests/test_recurrence_model.py | 78 +++++++++++++++++++---- 2 files changed, 95 insertions(+), 47 deletions(-) diff --git a/custom_components/nestquest/recurrence.py b/custom_components/nestquest/recurrence.py index 808bada..a004610 100644 --- a/custom_components/nestquest/recurrence.py +++ b/custom_components/nestquest/recurrence.py @@ -105,8 +105,19 @@ def _validate_date(value: str, field_name: str) -> None: def _validate_weekday_set(weekday_set) -> frozenset[int]: - """Return a normalized frozenset or raise with a clear message.""" + """Return a normalized frozenset or raise with a clear message. + + Sequence entries are validated BEFORE set() coercion: a list like + [0, False] must not collapse the boolean into 0, and unhashable + entries must raise RuleValidationError, never a raw TypeError. + """ if isinstance(weekday_set, (list, tuple)): + for entry in weekday_set: + if isinstance(entry, bool) or not isinstance(entry, int): + raise RuleValidationError( + f"weekday_set entries must be plain integers, got " + f"{entry!r}" + ) weekday_set = set(weekday_set) if not isinstance(weekday_set, (set, frozenset)): raise RuleValidationError( @@ -132,7 +143,9 @@ def _validate_weekday_set(weekday_set) -> frozenset[int]: def _is_plain_int(value) -> bool: - return isinstance(value, int) and not isinstance(value, bool) + """True only for real ints: bool excluded AND int subclasses too + (an unhashable int subclass would break the frozen-hashable model).""" + return type(value) is int def _validate_monthly_weekday(nth_weekday, weekday) -> None: @@ -182,40 +195,19 @@ class ScheduleRule: # Normalize a string rule_type into the enum; anything else # that is not already a RuleType is rejected. if isinstance(self.rule_type, str): - normalized = self.rule_type.strip().lower() - # Accept the model name (monthly_day) or the storage value - # ('monthly'/'custom'), the latter resolved by the populated - # fields per the DAO mapping. + # EXACT model names only: no whitespace stripping, no storage + # aliases. The DAO maps storage strings to model fields when + # reading rows; the model boundary stays unambiguous. candidates = {rt.name.lower(): rt for rt in RuleType} - if normalized in candidates: - object.__setattr__(self, "rule_type", candidates[normalized]) + if self.rule_type in candidates: + object.__setattr__( + self, "rule_type", candidates[self.rule_type] + ) else: - by_storage = { - rt.storage_value: rt for rt in RuleType - } - resolved = None - if normalized in by_storage: - candidate = by_storage[normalized] - # 'monthly' is ambiguous; resolve by fields. - if normalized == "monthly": - if self.nth_weekday is not None: - resolved = RuleType.MONTHLY_WEEKDAY - else: - resolved = RuleType.MONTHLY_DAY - elif normalized == "custom": - resolved = RuleType.CUSTOM_DAYS - elif normalized == "custom" and self.weekday_set is not None: - resolved = RuleType.CUSTOM_DAYS - if resolved is None: - resolved = candidates.get(self.rule_type) - if resolved is not None: - object.__setattr__(self, "rule_type", resolved) - else: - raise RuleValidationError( - f"rule_type must be one of " - f"{sorted(rt.name.lower() for rt in RuleType)} " - f"or a storage value, got {self.rule_type!r}" - ) + raise RuleValidationError( + f"rule_type must be a RuleType or one of " + f"{sorted(candidates)}, got {self.rule_type!r}" + ) if not isinstance(self.rule_type, RuleType): raise RuleValidationError( f"rule_type must be a RuleType or one of its string " @@ -365,7 +357,9 @@ class ScheduleRule: } unknown = [key for key in data if key not in allowed] if unknown: - raise RuleValidationError(f"unknown rule fields: {sorted(unknown)}") + raise RuleValidationError( + f"unknown rule fields: {sorted(unknown, key=repr)}" + ) if "rule_type" not in data: raise RuleValidationError("missing required rule field: rule_type") weekday_set = data.get("weekday_set") diff --git a/tests/test_recurrence_model.py b/tests/test_recurrence_model.py index 5457d4c..899a452 100644 --- a/tests/test_recurrence_model.py +++ b/tests/test_recurrence_model.py @@ -297,19 +297,26 @@ def test_recurrence_module_imports_only_stdlib() -> None: elif isinstance(node, ast.ImportFrom): module = (node.module or "").lower() level = node.level # 1+ means relative import - # Relative imports into the integration package (db/dao - # clients) are forbidden; schema.py is also off-limits. + # 'from . import db' has module=None and the name in the + # aliases: check BOTH the module and every imported name. + imported_names = [alias.name for alias in node.names] + forbidden_targets = { + "db", "schema", "dao_children", "dao_rules", + "dao_presence", "dao_instances", "migrations", "store", + } if level: - assert "db" not in module.split(".") and module not in { - "db", - "schema", - "dao_children", - "dao_rules", - "dao_presence", - "dao_instances", - "migrations", - "store", - }, f"recurrence.py must not import from .{module}" + for name in imported_names: + assert name not in forbidden_targets, ( + f"recurrence.py must not import {name!r} from a " + "relative import" + ) + if module: + assert module not in forbidden_targets, ( + f"recurrence.py must not import from .{module}" + ) + assert "db" not in module.split("."), ( + f"recurrence.py must not import from {module}" + ) assert "homeassistant" not in module, ( f"recurrence.py must not import from {module}" ) @@ -409,3 +416,50 @@ def test_rule_type_is_a_dataclass() -> None: import dataclasses assert dataclasses.is_dataclass(ScheduleRule) + +def test_direct_list_with_bool_weekday_rejected(tmp_path) -> None: + """Direct construction with a list: entries validated pre-coercion.""" + with pytest.raises(RuleValidationError, match="plain integers"): + ScheduleRule( + rule_type=RuleType.WEEKLY, weekday_set=[0, False], + start_date="2026-09-01", + ) + + +def test_unhashable_list_item_leads_to_clear_error(tmp_path) -> None: + """Unhashable entries raise RuleValidationError, not TypeError.""" + with pytest.raises(RuleValidationError): + ScheduleRule( + rule_type=RuleType.WEEKLY, weekday_set=[[0]], + start_date="2026-09-01", + ) + + +def test_from_dict_mixed_type_unknown_keys_raise_validation_error( + tmp_path, +) -> None: + """sorted() over mixed key types must not leak TypeError.""" + with pytest.raises(RuleValidationError, match="unknown rule fields"): + ScheduleRule.from_dict({"rule_type": "daily", 42: "value"}) + + +def test_storage_aliases_and_whitespace_are_not_model_names(tmp_path) -> None: + """The model boundary is exact names only: storage strings, padded + and case variants are all rejected.""" + for bad in ("monthly", "custom", " DAILY ", "daily ", "DAILY"): + with pytest.raises(RuleValidationError): + ScheduleRule(rule_type=bad, start_date="2026-09-01") + + +def test_unhashable_int_subclass_interval_rejected() -> None: + """An int subclass with __hash__ = None must not construct.""" + + class UnhashableInt(int): + __hash__ = None + + with pytest.raises(RuleValidationError): + ScheduleRule( + rule_type=RuleType.DAILY, + interval=UnhashableInt(1), + start_date="2026-09-01", + ) -- 2.55.0 From deb409175a4648e3b37f716bbe7d3e74d90cafb7 Mon Sep 17 00:00:00 2001 From: Joshua King Date: Mon, 14 Sep 2026 10:51:21 -0400 Subject: [PATCH 4/5] fix: plain-int check pre-coercion everywhere; purity guard scans alias leaf names at any level --- custom_components/nestquest/recurrence.py | 4 +- tests/test_recurrence_model.py | 60 ++++++++++++++++++----- 2 files changed, 52 insertions(+), 12 deletions(-) diff --git a/custom_components/nestquest/recurrence.py b/custom_components/nestquest/recurrence.py index a004610..aa4f506 100644 --- a/custom_components/nestquest/recurrence.py +++ b/custom_components/nestquest/recurrence.py @@ -113,7 +113,9 @@ def _validate_weekday_set(weekday_set) -> frozenset[int]: """ if isinstance(weekday_set, (list, tuple)): for entry in weekday_set: - if isinstance(entry, bool) or not isinstance(entry, int): + # _is_plain_int: bool and int subclasses (potentially + # unhashable) are rejected before set() coercion. + if not _is_plain_int(entry): raise RuleValidationError( f"weekday_set entries must be plain integers, got " f"{entry!r}" diff --git a/tests/test_recurrence_model.py b/tests/test_recurrence_model.py index 899a452..fd8d22c 100644 --- a/tests/test_recurrence_model.py +++ b/tests/test_recurrence_model.py @@ -297,25 +297,26 @@ def test_recurrence_module_imports_only_stdlib() -> None: elif isinstance(node, ast.ImportFrom): module = (node.module or "").lower() level = node.level # 1+ means relative import - # 'from . import db' has module=None and the name in the - # aliases: check BOTH the module and every imported name. - imported_names = [alias.name for alias in node.names] + # Forbidden regardless of relative level: the leaf module + # names ('from . import db', 'from .db import x') AND any + # absolute custom_components.nestquest DB/store import. forbidden_targets = { "db", "schema", "dao_children", "dao_rules", "dao_presence", "dao_instances", "migrations", "store", } - if level: - for name in imported_names: - assert name not in forbidden_targets, ( - f"recurrence.py must not import {name!r} from a " - "relative import" - ) + imported_names = [alias.name for alias in node.names] + for name in imported_names: + leaf = name.split(".")[-1] + assert leaf not in forbidden_targets, ( + f"recurrence.py must not import {name!r}" + ) if module: + full = f".{module}" if level else module assert module not in forbidden_targets, ( - f"recurrence.py must not import from .{module}" + f"recurrence.py must not import from {full}" ) assert "db" not in module.split("."), ( - f"recurrence.py must not import from {module}" + f"recurrence.py must not import from {full}" ) assert "homeassistant" not in module, ( f"recurrence.py must not import from {module}" @@ -325,6 +326,43 @@ def test_recurrence_module_imports_only_stdlib() -> None: ) +def test_purity_guard_blocks_absolute_db_import(tmp_path) -> None: + """Absolute custom_components.nestquest DB imports (level=0) are + caught by the same alias-leaf scan the guard uses.""" + import ast as ast_module + + forbidden_targets = { + "db", "schema", "dao_children", "dao_rules", "dao_presence", + "dao_instances", "migrations", "store", + } + # The guard's own logic, exercised against probe statements: + from custom_components.nestquest import recurrence as recurrence_mod + probe_lines = ( + "from custom_components.nestquest import db", + "from custom_components.nestquest.db import NestQuestDatabase", + "import custom_components.nestquest.dao_rules", + ) + for line in probe_lines: + node = ast_module.parse(line).body[0] + caught = False + for item in ast_module.walk(node): + if isinstance(item, ast_module.ImportFrom): + module = (item.module or "").lower() + if module: + leaf = module.split(".")[-1] + if leaf in forbidden_targets: + caught = True + for alias in item.names: + if alias.name.split(".")[-1] in forbidden_targets: + caught = True + elif isinstance(item, ast_module.Import): + for alias in item.names: + leaf = alias.name.split(".")[-1] + if leaf in forbidden_targets: + caught = True + assert caught, f"probe import must be caught: {line}" + + def test_non_string_rule_type_rejected() -> None: """Only RuleType members and their string values construct.""" for bad in (123, True, None, 3.5, b"daily"): -- 2.55.0 From 4163667b07bc631465bd25bba6b9d6e8492b960b Mon Sep 17 00:00:00 2001 From: Joshua King Date: Mon, 14 Sep 2026 10:53:24 -0400 Subject: [PATCH 5/5] fix: _is_plain_int on every weekday_set path (from_dict + post-set); shared scan function probes the real guard --- custom_components/nestquest/recurrence.py | 7 +- tests/test_recurrence_model.py | 239 ++++++++++------------ 2 files changed, 112 insertions(+), 134 deletions(-) diff --git a/custom_components/nestquest/recurrence.py b/custom_components/nestquest/recurrence.py index aa4f506..ba7f605 100644 --- a/custom_components/nestquest/recurrence.py +++ b/custom_components/nestquest/recurrence.py @@ -131,7 +131,7 @@ def _validate_weekday_set(weekday_set) -> frozenset[int]: "weekday_set must not be empty for weekly/custom rules" ) for entry in weekday_set: - if isinstance(entry, bool) or not isinstance(entry, int): + if not _is_plain_int(entry): raise RuleValidationError( f"weekday_set entries must be plain integers, got " f"{entry!r}" @@ -367,9 +367,10 @@ class ScheduleRule: weekday_set = data.get("weekday_set") if isinstance(weekday_set, (list, tuple)): # Validate entries as plain ints BEFORE set() coercion, so a - # list like [0, False] cannot collapse False into 0. + # list like [0, False] cannot collapse False into 0 and an + # unhashable/int-subclass entry cannot leak a raw TypeError. for entry in weekday_set: - if isinstance(entry, bool) or not isinstance(entry, int): + if not _is_plain_int(entry): raise RuleValidationError( f"weekday_set entries must be plain integers, got " f"{entry!r}" diff --git a/tests/test_recurrence_model.py b/tests/test_recurrence_model.py index fd8d22c..5d0def5 100644 --- a/tests/test_recurrence_model.py +++ b/tests/test_recurrence_model.py @@ -272,95 +272,88 @@ def test_rule_not_equal_to_other_types() -> None: def test_recurrence_module_imports_only_stdlib() -> None: - """The model must carry no Home Assistant and no DB imports — checked - against the RESOLVED module path so relative forms (from .db import - x) are caught too.""" - import ast - import importlib.util + """The model must carry no Home Assistant and no DB imports.""" from pathlib import Path - module_file = Path( - importlib.util.find_spec( - "custom_components.nestquest.recurrence" - ).origin + source = Path( + __import__("custom_components.nestquest.recurrence", + fromlist=["__file__"]).__file__ + ).read_text() + offenders = _scan_forbidden_imports(source) + assert offenders == [], ( + f"recurrence.py must not hold forbidden imports: {offenders}" ) - tree = ast.parse(module_file.read_text()) - for node in ast.walk(tree): - if isinstance(node, ast.Import): - for alias in node.names: - assert "homeassistant" not in alias.name.lower(), ( - f"recurrence.py must not import {alias.name}" - ) - assert "sqlite" not in alias.name.lower(), ( - f"recurrence.py must not import {alias.name}" - ) - elif isinstance(node, ast.ImportFrom): - module = (node.module or "").lower() - level = node.level # 1+ means relative import - # Forbidden regardless of relative level: the leaf module - # names ('from . import db', 'from .db import x') AND any - # absolute custom_components.nestquest DB/store import. - forbidden_targets = { - "db", "schema", "dao_children", "dao_rules", - "dao_presence", "dao_instances", "migrations", "store", - } - imported_names = [alias.name for alias in node.names] - for name in imported_names: - leaf = name.split(".")[-1] - assert leaf not in forbidden_targets, ( - f"recurrence.py must not import {name!r}" - ) - if module: - full = f".{module}" if level else module - assert module not in forbidden_targets, ( - f"recurrence.py must not import from {full}" - ) - assert "db" not in module.split("."), ( - f"recurrence.py must not import from {full}" - ) - assert "homeassistant" not in module, ( - f"recurrence.py must not import from {module}" - ) - assert "sqlite" not in module, ( - f"recurrence.py must not import from {module}" - ) -def test_purity_guard_blocks_absolute_db_import(tmp_path) -> None: - """Absolute custom_components.nestquest DB imports (level=0) are - caught by the same alias-leaf scan the guard uses.""" - import ast as ast_module +def _scan_forbidden_imports(source: str) -> list[str]: + """The ACTUAL purity-guard logic, shared by the model scan and the + probe tests below: returns human-readable offenders (empty = clean). + + Forbidden, at any relative level, for both Import and ImportFrom: + - modules containing 'homeassistant' or 'sqlite' + - modules whose LEAF name is a DB/store/schema/DAO module + - aliases importing a DB/store/schema/DAO module by leaf name + ('from . import db', 'from custom_components.nestquest import db') + """ + import ast forbidden_targets = { "db", "schema", "dao_children", "dao_rules", "dao_presence", "dao_instances", "migrations", "store", } - # The guard's own logic, exercised against probe statements: - from custom_components.nestquest import recurrence as recurrence_mod - probe_lines = ( - "from custom_components.nestquest import db", + offenders: list[str] = [] + tree = ast.parse(source) + for node in ast.walk(tree): + if isinstance(node, ast.Import): + for alias in node.names: + lowered = alias.name.lower() + leaf = lowered.split(".")[-1] + if ( + "homeassistant" in lowered + or "sqlite" in lowered + or leaf in forbidden_targets + ): + offenders.append(f"import {alias.name}") + elif isinstance(node, ast.ImportFrom): + module = (node.module or "").lower() + module_leaf = module.split(".")[-1] if module else "" + if ( + "homeassistant" in module + or "sqlite" in module + or (module and module_leaf in forbidden_targets) + or (node.level and module_leaf in forbidden_targets) + ): + offenders.append( + f"from {node.module!r} import ... (level={node.level})" + ) + for alias in node.names: + alias_leaf = alias.name.split(".")[-1].lower() + if alias_leaf in forbidden_targets: + offenders.append( + f"from {node.module!r} import {alias.name} " + f"(level={node.level})" + ) + return offenders + + +def test_purity_guard_logic_catches_probe_statements(tmp_path) -> None: + """The shared scan function catches every forbidden import shape, + including level-0 absolute package DB imports and 'from . import db'. + """ + probes = [ + "from homeassistant.core import HomeAssistant", + "import sqlite3", + "from .db import NestQuestDatabase", "from custom_components.nestquest.db import NestQuestDatabase", + "from custom_components.nestquest import db", "import custom_components.nestquest.dao_rules", - ) - for line in probe_lines: - node = ast_module.parse(line).body[0] - caught = False - for item in ast_module.walk(node): - if isinstance(item, ast_module.ImportFrom): - module = (item.module or "").lower() - if module: - leaf = module.split(".")[-1] - if leaf in forbidden_targets: - caught = True - for alias in item.names: - if alias.name.split(".")[-1] in forbidden_targets: - caught = True - elif isinstance(item, ast_module.Import): - for alias in item.names: - leaf = alias.name.split(".")[-1] - if leaf in forbidden_targets: - caught = True - assert caught, f"probe import must be caught: {line}" + "from . import db", + "from . import schema", + "from custom_components.nestquest.migrations import apply_migrations", + ] + for probe in probes: + offenders = _scan_forbidden_imports(probe) + assert offenders, f"probe must be caught: {probe!r}" def test_non_string_rule_type_rejected() -> None: @@ -373,16 +366,12 @@ def test_non_string_rule_type_rejected() -> None: def test_weekly_rejects_month_and_nth_fields() -> None: with pytest.raises(RuleValidationError, match="must not set"): ScheduleRule( - rule_type="weekly", - weekday_set={0}, - day_of_month=5, + rule_type=RuleType.WEEKLY, weekday_set={0}, day_of_month=5, start_date="2026-09-01", ) with pytest.raises(RuleValidationError, match="must not set"): ScheduleRule( - rule_type=RuleType.WEEKLY, - weekday_set={0}, - month=3, + rule_type=RuleType.WEEKLY, weekday_set={0}, month=3, start_date="2026-09-01", ) @@ -390,8 +379,7 @@ def test_weekly_rejects_month_and_nth_fields() -> None: def test_daily_rejects_dangling_nth_weekday_weekday() -> None: with pytest.raises(RuleValidationError, match="must not set"): ScheduleRule( - rule_type=RuleType.DAILY, - nth_weekday_weekday=0, + rule_type=RuleType.DAILY, nth_weekday_weekday=0, start_date="2026-09-01", ) @@ -399,9 +387,7 @@ def test_daily_rejects_dangling_nth_weekday_weekday() -> None: def test_monthly_day_rejects_month() -> None: with pytest.raises(RuleValidationError, match="must not set"): ScheduleRule( - rule_type=RuleType.MONTHLY_DAY, - day_of_month=5, - month=3, + rule_type=RuleType.MONTHLY_DAY, day_of_month=5, month=3, start_date="2026-09-01", ) @@ -409,18 +395,13 @@ def test_monthly_day_rejects_month() -> None: def test_yearly_rejects_day_and_nth_fields() -> None: with pytest.raises(RuleValidationError, match="must not set"): ScheduleRule( - rule_type=RuleType.YEARLY, - month=6, - day_of_month=15, + rule_type=RuleType.YEARLY, month=6, day_of_month=15, start_date="2026-09-01", ) with pytest.raises(RuleValidationError, match="must not set"): ScheduleRule( - rule_type=RuleType.YEARLY, - month=6, - nth_weekday=1, - nth_weekday_weekday=0, - start_date="2026-09-01", + rule_type=RuleType.YEARLY, month=6, nth_weekday=1, + nth_weekday_weekday=0, start_date="2026-09-01", ) @@ -442,46 +423,38 @@ def test_from_dict_bool_weekday_entry_rejected_before_set_coercion() -> None: ) +def test_direct_set_with_int_subclass_entry_rejected() -> None: + """A hashable-but-non-plain int subclass is still not a plain int.""" + + class IntChild(int): + pass + + with pytest.raises(RuleValidationError, match="plain integers"): + ScheduleRule( + rule_type=RuleType.WEEKLY, + weekday_set={IntChild(1)}, + start_date="2026-09-01", + ) + + +def test_from_dict_with_int_subclass_entry_rejected() -> None: + class IntChild(int): + pass + + with pytest.raises(RuleValidationError, match="plain integers"): + ScheduleRule.from_dict( + {"rule_type": "weekly", "weekday_set": [IntChild(1)]} + ) + + def test_from_dict_unknown_key_types_never_leak_typeerror() -> None: """Mixed unhashable unknown keys must surface as RuleValidationError, not a raw TypeError from sorting.""" - data = {"rule_type": "daily", 42: "value"} - with pytest.raises(RuleValidationError): - ScheduleRule.from_dict(data) - - -def test_rule_type_is_a_dataclass() -> None: - import dataclasses - - assert dataclasses.is_dataclass(ScheduleRule) - -def test_direct_list_with_bool_weekday_rejected(tmp_path) -> None: - """Direct construction with a list: entries validated pre-coercion.""" - with pytest.raises(RuleValidationError, match="plain integers"): - ScheduleRule( - rule_type=RuleType.WEEKLY, weekday_set=[0, False], - start_date="2026-09-01", - ) - - -def test_unhashable_list_item_leads_to_clear_error(tmp_path) -> None: - """Unhashable entries raise RuleValidationError, not TypeError.""" - with pytest.raises(RuleValidationError): - ScheduleRule( - rule_type=RuleType.WEEKLY, weekday_set=[[0]], - start_date="2026-09-01", - ) - - -def test_from_dict_mixed_type_unknown_keys_raise_validation_error( - tmp_path, -) -> None: - """sorted() over mixed key types must not leak TypeError.""" with pytest.raises(RuleValidationError, match="unknown rule fields"): ScheduleRule.from_dict({"rule_type": "daily", 42: "value"}) -def test_storage_aliases_and_whitespace_are_not_model_names(tmp_path) -> None: +def test_storage_aliases_and_whitespace_are_not_model_names() -> None: """The model boundary is exact names only: storage strings, padded and case variants are all rejected.""" for bad in ("monthly", "custom", " DAILY ", "daily ", "DAILY"): @@ -490,8 +463,6 @@ def test_storage_aliases_and_whitespace_are_not_model_names(tmp_path) -> None: def test_unhashable_int_subclass_interval_rejected() -> None: - """An int subclass with __hash__ = None must not construct.""" - class UnhashableInt(int): __hash__ = None @@ -501,3 +472,9 @@ def test_unhashable_int_subclass_interval_rejected() -> None: interval=UnhashableInt(1), start_date="2026-09-01", ) + + +def test_rule_type_is_a_dataclass() -> None: + import dataclasses + + assert dataclasses.is_dataclass(ScheduleRule) -- 2.55.0