diff --git a/custom_components/nestquest/dao_rules.py b/custom_components/nestquest/dao_rules.py index 0d9f748..d994791 100644 --- a/custom_components/nestquest/dao_rules.py +++ b/custom_components/nestquest/dao_rules.py @@ -209,6 +209,36 @@ def _window_from_row(row: tuple) -> QuestDefinitionWindowRecord: ) +#: Storage columns that are forbidden for each rule type. A populated +#: forbidden column means the row's shape is invalid and must raise on +#: read, never be silently dropped. ``weekday_set`` is absent for +#: DAILY/MONTHLY_DAY/YEARLY; MONTHLY_WEEKDAY is the only shape allowed a +#: non-NULL ``nth_weekday``; YEARLY is the only shape allowed ``month``. +_FORBIDDEN_STORAGE_FIELDS: dict[str, tuple[RuleType, ...]] = { + "weekday_set": (RuleType.DAILY, RuleType.MONTHLY_DAY, RuleType.YEARLY), + "day_of_month": ( + RuleType.DAILY, + RuleType.WEEKLY, + RuleType.CUSTOM_DAYS, + RuleType.MONTHLY_WEEKDAY, + ), + "nth_weekday": ( + RuleType.DAILY, + RuleType.WEEKLY, + RuleType.CUSTOM_DAYS, + RuleType.MONTHLY_DAY, + RuleType.YEARLY, + ), + "month": ( + RuleType.DAILY, + RuleType.WEEKLY, + RuleType.CUSTOM_DAYS, + RuleType.MONTHLY_DAY, + RuleType.MONTHLY_WEEKDAY, + ), +} + + def schedule_rule_to_storage(rule: ScheduleRule) -> ScheduleRuleStorage: """Map a validated ScheduleRule onto its storage columns. @@ -261,6 +291,8 @@ def schedule_rule_from_storage(storage: ScheduleRuleStorage) -> ScheduleRule: f"unknown storage rule_type {rule_type!r}" ) + _reject_forbidden_storage_fields(storage, shape) + weekday_set = None nth_weekday_weekday = None if shape is RuleType.WEEKLY or shape is RuleType.CUSTOM_DAYS: @@ -281,6 +313,28 @@ def schedule_rule_from_storage(storage: ScheduleRuleStorage) -> ScheduleRule: ) +def _reject_forbidden_storage_fields( + storage: ScheduleRuleStorage, shape: RuleType +) -> None: + """Raise unless every populated column is permitted for ``shape``. + + The model constructor rejects a forbidden ``day_of_month``, + ``nth_weekday``, or ``month``, but ``weekday_set`` never reaches it + (it is folded for MONTHLY_WEEKDAY and dropped otherwise), so a daily + row carrying ``weekday_set="0"`` would otherwise decode as a valid + daily rule and silently lose the stored data. This validates every + shape column explicitly so a populated-forbidden column raises + instead of being discarded. + """ + for field, forbidden_shapes in _FORBIDDEN_STORAGE_FIELDS.items(): + value = getattr(storage, field) + if value is not None and shape in forbidden_shapes: + raise RuleValidationError( + f"{shape.value} storage rows must not set {field}, " + f"got {value!r}" + ) + + def _disambiguate_monthly(storage: ScheduleRuleStorage) -> RuleType: """Resolve the shared 'monthly' storage value to a model type. @@ -321,16 +375,29 @@ def _weekdays_from_csv(value: str | None) -> frozenset[int]: def _single_weekday_from_csv(value: str | None) -> int: """The one weekday a MONTHLY_WEEKDAY stores in weekday_set. - Raises RuleValidationError when the column is empty or holds more - than one weekday — the position's weekday must be unambiguous. + Parses the CSV into tokens FIRST and requires exactly one token, + so a duplicate pair like ``"1,1"`` is rejected rather than silently + canonicalized to a single weekday on re-storage. An empty or + multi-element column raises RuleValidationError. """ - entries = _weekdays_from_csv(value) - if len(entries) != 1: + if value is None or value == "": raise RuleValidationError( "MONTHLY_WEEKDAY storage weekday_set must hold exactly one " f"weekday, got {value!r}" ) - return next(iter(entries)) + tokens = value.split(",") + if len(tokens) != 1: + raise RuleValidationError( + "MONTHLY_WEEKDAY storage weekday_set must hold exactly one " + f"weekday, got {value!r}" + ) + try: + return int(tokens[0]) + except ValueError: + raise RuleValidationError( + "MONTHLY_WEEKDAY storage weekday_set must be an integer, " + f"got {value!r}" + ) from None class ScheduleRulesDao: diff --git a/tests/test_dao_rules.py b/tests/test_dao_rules.py index ed36f45..34565b3 100644 --- a/tests/test_dao_rules.py +++ b/tests/test_dao_rules.py @@ -608,6 +608,72 @@ def test_schedule_rule_from_storage_rejects_malformed_weekday_csv() -> None: ) +@pytest.mark.parametrize( + "storage", + [ + # DAILY carrying shape columns it must not hold. + ScheduleRuleStorage( + "daily", 1, "0", None, None, None, "2026-09-01", None + ), + ScheduleRuleStorage( + "daily", 1, None, 15, None, None, "2026-09-01", None + ), + ScheduleRuleStorage( + "daily", 1, None, None, 2, None, "2026-09-01", None + ), + ScheduleRuleStorage( + "daily", 1, None, None, None, 6, "2026-09-01", None + ), + # WEEKLY carrying shape columns it must not hold. + ScheduleRuleStorage( + "weekly", 1, "0,2", 15, None, None, "2026-09-01", None + ), + ScheduleRuleStorage( + "weekly", 1, "0,2", None, None, 6, "2026-09-01", None + ), + # CUSTOM_DAYS carrying shape columns it must not hold. + ScheduleRuleStorage( + "custom", 1, "0,2", 15, None, None, "2026-09-01", None + ), + ScheduleRuleStorage( + "custom", 1, "0,2", None, None, 6, "2026-09-01", None + ), + # MONTHLY_DAY carrying weekday_set (folded-only column) or month. + ScheduleRuleStorage( + "monthly", 1, "1", 15, None, None, "2026-09-01", None + ), + ScheduleRuleStorage( + "monthly", 1, None, 15, None, 6, "2026-09-01", None + ), + # MONTHLY_WEEKDAY carrying a forbidden month. + ScheduleRuleStorage( + "monthly", 1, "1", None, 2, 6, "2026-09-01", None + ), + # YEARLY carrying weekday_set or nth_weekday. + ScheduleRuleStorage( + "yearly", 1, "0", None, None, 6, "2026-09-01", None + ), + ScheduleRuleStorage( + "yearly", 1, None, None, 2, 6, "2026-09-01", None + ), + ], +) +def test_schedule_rule_from_storage_rejects_forbidden_fields(storage) -> None: + with pytest.raises(RuleValidationError): + schedule_rule_from_storage(storage) + + +def test_schedule_rule_from_storage_rejects_duplicate_weekday_token() -> None: + """A MONTHLY_WEEKDAY with a duplicate CSV token ("1,1") must raise, + never silently canonicalize to a single weekday on re-storage.""" + with pytest.raises(RuleValidationError): + schedule_rule_from_storage( + ScheduleRuleStorage( + "monthly", 1, "1,1", None, 2, None, "2026-09-01", None + ) + ) + + # --------------------------------------------------------------------------- # quest_definitions: create / get # ---------------------------------------------------------------------------