Apply PR #144 review fixes: per-field settings fallback, strict validation, hardened AST scan (task 4dd8f870)

P2-1: replace the all-or-nothing from_options try/except in setup with
from_options_resilient, which falls back PER FIELD (logging a warning
naming the bad field) so a malformed sibling (e.g. morning_summary_time
'25:00') cannot silently reset horizon_days/day_rollover_time.
P2-2: drop _run_horizon_materialization's legacy horizon_days keyword and
branch; default settings to NestQuestSettings(); remove the legacy paragraph.
P2-3: _run_horizon_materialization now calls settings.horizon_window(today)
so the helper is real and shared; extend tests to run the integration path
(_run_horizon_materialization + _register_day_rollover_listener) with
horizon_days=3 / day_rollover_time='03:30'.
P2-4: harden the AST scan to forbid hass/config_entry names, .hass/.entry/
.config_entry and .config/.options/.data attributes, and getattr/hasattr
calls with those string args; add a negative-control test for each shape.
P2-5: __post_init__ is strict (rejects None for every field); the
None->default mapping lives ONLY in from_options via _option_value.
P3: remove dead shims (_day_rollover_hour_minute, _configured_horizon_days)
and repoint test_lifecycle at from_options; de-dup the two HH:MM
validators into one using a shared TIME_PATTERN in core/const; make
horizon_window's today required; reword the notify_target docstring to
not claim options-flow parity; document _option_value's None-as-absent
semantics; remove unused test helpers.
This commit is contained in:
2026-09-21 15:37:12 -04:00
parent 345bd200e1
commit 33102f8806
6 changed files with 718 additions and 311 deletions
+15 -56
View File
@@ -19,7 +19,6 @@ from homeassistant.helpers.event import async_track_time_change
from .const import ( from .const import (
CONF_ADMIN_USER_IDS, CONF_ADMIN_USER_IDS,
CONF_UPDATE_INTERVAL, CONF_UPDATE_INTERVAL,
DEFAULT_HORIZON_DAYS,
DEFAULT_UPDATE_INTERVAL, DEFAULT_UPDATE_INTERVAL,
DOMAIN, DOMAIN,
LOGGER, LOGGER,
@@ -268,43 +267,10 @@ async def _async_owner_user_ids(hass: HomeAssistant) -> list[str]:
return owners return owners
def _day_rollover_hour_minute(options: dict[str, Any]) -> tuple[int, int]:
"""Return the (hour, minute) the daily rollover fires at from ``options``.
Thin shim over :class:`NestQuestSettings`: the options flow validates
``day_rollover_time`` as a strict ``HH:MM`` string, so a stored value
never needs re-validation here; a missing key falls back to
:data:`~.const.DEFAULT_DAY_ROLLOVER_TIME`.
"""
return NestQuestSettings.from_options(options).day_rollover_hour_minute
def _configured_horizon_days(options: dict[str, Any]) -> int:
"""Return the configured generation horizon, validated, else the default.
Thin shim over :class:`NestQuestSettings`. The settings object's
``from_options`` re-checks ``horizon_days`` — rejecting bools,
non-ints, and sub-1 values, falling back to
:data:`~.const.DEFAULT_HORIZON_DAYS` — so a malformed option can
never shrink or explode the materialized horizon.
"""
try:
return NestQuestSettings.from_options(options).horizon_days
except ValueError:
# A malformed stored option (a bad time string) must not crash
# startup; the horizon itself was already validated above, so a
# ValueError here can only come from a sibling time field — fall
# back to the default horizon the same way the pre-settings path
# tolerated partial options.
return DEFAULT_HORIZON_DAYS
async def _run_horizon_materialization( async def _run_horizon_materialization(
hass: HomeAssistant, hass: HomeAssistant,
database: NestQuestDatabase, database: NestQuestDatabase,
settings: NestQuestSettings | None = None, settings: NestQuestSettings | None = None,
*,
horizon_days: int | None = None,
) -> None: ) -> None:
"""Materialize the rolling horizon ``[today, today + horizon_days]``. """Materialize the rolling horizon ``[today, today + horizon_days]``.
@@ -315,24 +281,18 @@ async def _run_horizon_materialization(
This is the ONE generation path: the startup backfill, the daily rollover This is the ONE generation path: the startup backfill, the daily rollover
listener and the ``regenerate`` service all funnel through it, so there is listener and the ``regenerate`` service all funnel through it, so there is
no duplicated materialization logic. Callers SHOULD pass a no duplicated materialization logic. ``settings`` carries the entry's
:class:`NestQuestSettings` instance built from ``entry.options``; the validated ``horizon_days``; when omitted a default
legacy ``horizon_days`` keyword is kept for callers that have not yet :class:`NestQuestSettings` is used (its horizon is the configured
been converted and overrides the settings value when supplied. When default). The window itself comes from :meth:`settings.horizon_window`
neither is given the window sizes to so the helper is real and shared (not recomputed inline here).
:data:`~.const.DEFAULT_HORIZON_DAYS`.
""" """
if settings is not None: if settings is None:
horizon = settings.horizon_days settings = NestQuestSettings()
else:
horizon = horizon_days if horizon_days is not None else DEFAULT_HORIZON_DAYS
time_zone = ZoneInfo(hass.config.time_zone) time_zone = ZoneInfo(hass.config.time_zone)
today = datetime.datetime.now(time_zone).date() today = datetime.datetime.now(time_zone).date()
start_date = today.isoformat() start, end = settings.horizon_window(today)
end_date = ( await _materialize_run(database, start.isoformat(), end.isoformat(), today=today)
today + datetime.timedelta(days=horizon)
).isoformat()
await _materialize_run(database, start_date, end_date, today=today)
def _register_day_rollover_listener( def _register_day_rollover_listener(
@@ -507,13 +467,12 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
# Build the explicit settings object from entry.options ONCE: the # Build the explicit settings object from entry.options ONCE: the
# horizon materialization and the daily-rollover listener both # horizon materialization and the daily-rollover listener both
# consume it, so core never reads HA config itself. A malformed # consume it, so core never reads HA config itself. A malformed
# stored option falls back to the defaults (the same way the # stored option falls back PER FIELD (via from_options_resilient)
# pre-settings path tolerated a partial options dict) rather than # so a single bad sibling option cannot silently reset the user's
# crashing setup. # horizon_days or day_rollover_time — the two fields core consumes
try: # — rather than the all-or-nothing reset a single try/except would
settings = NestQuestSettings.from_options(entry.options) # cause (a malformed morning_summary_time used to wipe horizon_days).
except ValueError: settings = NestQuestSettings.from_options_resilient(entry.options)
settings = NestQuestSettings()
remove_update_listener = entry.add_update_listener(_async_update_listener) remove_update_listener = entry.add_update_listener(_async_update_listener)
remove_time_change_listener = _register_day_rollover_listener( remove_time_change_listener = _register_day_rollover_listener(
hass, database, settings hass, database, settings
@@ -2,11 +2,19 @@
from __future__ import annotations from __future__ import annotations
import logging import logging
import re
DOMAIN = "nestquest" DOMAIN = "nestquest"
LOGGER_NAME = "custom_components.nestquest" LOGGER_NAME = "custom_components.nestquest"
LOGGER = logging.getLogger(LOGGER_NAME) LOGGER = logging.getLogger(LOGGER_NAME)
#: Strict 24-hour ``HH:MM`` pattern shared by the options flow and the
#: settings object's time validation, so a stored option and a freshly
#: validated form submit parse the same way. Defined here (HA-free) so
#: both the integration's options flow and the core settings module
#: import ONE pattern instead of re-copying it.
TIME_PATTERN = re.compile(r"(?:[01]\d|2[0-3]):[0-5]\d")
# Platforms set up per config entry (Feature 10): the per-child day # Platforms set up per config entry (Feature 10): the per-child day
# sensors and binary sensors. # sensors and binary sensors.
PLATFORMS: list[str] = ["sensor", "binary_sensor"] PLATFORMS: list[str] = ["sensor", "binary_sensor"]
+221 -98
View File
@@ -4,12 +4,13 @@ The integration reads its configuration off a Home Assistant config
entry (``entry.options``); the core package must not. This module entry (``entry.options``); the core package must not. This module
bridges that split: it defines a frozen, validation-carrying bridges that split: it defines a frozen, validation-carrying
:class:`NestQuestSettings` dataclass that the integration constructs :class:`NestQuestSettings` dataclass that the integration constructs
from ``entry.options`` (via :meth:`NestQuestSettings.from_options`) and from ``entry.options`` (via :meth:`NestQuestSettings.from_options`, or
then passes into the core paths that previously read HA config :meth:`NestQuestSettings.from_options_resilient` for the per-field
themselves — the rolling horizon (``horizon_days``) and the daily fallback used at setup) and then passes into the core paths that
rollover time (``day_rollover_time``), plus the Feature 11 notification previously read HA config themselves — the rolling horizon
settings (notify target, the three summary/reminder/report times, and (``horizon_days``) and the daily rollover time (``day_rollover_time``),
the four independent enable toggles). plus the Feature 11 notification settings (notify target, the three
summary/reminder/report times, and the four independent enable toggles).
Nothing in this module imports ``homeassistant`` or touches a config Nothing in this module imports ``homeassistant`` or touches a config
entry; ``from_options`` accepts any plain mapping keyed by the entry; ``from_options`` accepts any plain mapping keyed by the
@@ -19,11 +20,18 @@ rejected), and ``day_rollover_time`` plus the three notification times
must be strict ``HH:MM`` 24-hour strings. Construction therefore can must be strict ``HH:MM`` 24-hour strings. Construction therefore can
never yield a settings object whose horizon could shrink or explode the never yield a settings object whose horizon could shrink or explode the
materialized window, or whose rollover fires at an unparsable time. materialized window, or whose rollover fires at an unparsable time.
The strict resolvers (:func:`_resolve_horizon_days`, :func:`_validate_hhmm`,
:func:`_resolve_bool`, :func:`_resolve_optional_str`) reject ``None``; the
``None``-as-absent mapping lives ONLY in :meth:`from_options` /
:meth:`from_options_resilient` (via :func:`_option_value`), so a hand-built
:class:`NestQuestSettings` cannot smuggle a ``None`` past
:meth:`__post_init__`.
""" """
from __future__ import annotations from __future__ import annotations
import datetime import datetime
import re from collections.abc import Callable
from dataclasses import dataclass from dataclasses import dataclass
from types import MappingProxyType from types import MappingProxyType
from typing import Any, Mapping from typing import Any, Mapping
@@ -45,25 +53,27 @@ from .const import (
DEFAULT_END_OF_DAY_REPORT_TIME, DEFAULT_END_OF_DAY_REPORT_TIME,
DEFAULT_HORIZON_DAYS, DEFAULT_HORIZON_DAYS,
DEFAULT_MORNING_SUMMARY_TIME, DEFAULT_MORNING_SUMMARY_TIME,
LOGGER,
TIME_PATTERN,
) )
#: Strict 24-hour ``HH:MM`` pattern — matches the options flow's
#: ``_TIME_PATTERN`` verbatim so a stored option and a freshly-validated
#: form submit parse the same way.
_HHMM_PATTERN = re.compile(r"(?:[01]\d|2[0-3]):[0-5]\d")
def _option_value( def _option_value(
opts: Mapping[str, Any], key: str, default: Any opts: Mapping[str, Any], key: str, default: Any
) -> Any: ) -> Any:
"""Return ``opts[key]`` when present and non-None, else ``default``. """Return ``opts[key]`` when present and non-None, else ``default``.
A stored ``None`` is treated the same as an absent key — the This is the SOLE place the ``None``-as-absent mapping lives: a stored
historical ``horizon_days`` path mapped a missing-or-None value to ``None`` is treated the same as an absent key — the historical
the default, and this keeps that forgiving semantics uniform across ``horizon_days`` path mapped a missing-or-None value to the default,
every field: a partially-populated ``entry.options`` dict (or one and this keeps that forgiving semantics uniform across every field.
migrated across versions) never raises just because a key carries The per-field resolvers (:func:`_resolve_horizon_days`,
``None``. :func:`_validate_hhmm`, :func:`_resolve_bool`,
:func:`_resolve_optional_str`) are STRICT — they reject ``None`` —
so a partially-populated ``entry.options`` dict (or one migrated
across versions) never raises just because a key carries ``None``,
and a hand-built :class:`NestQuestSettings` cannot smuggle a
``None`` past :meth:`NestQuestSettings.__post_init__`.
""" """
if key not in opts: if key not in opts:
return default return default
@@ -71,31 +81,36 @@ def _option_value(
return default if value is None else value return default if value is None else value
def _parse_hhmm(value: str) -> tuple[int, int]: def _validate_hhmm(value: Any, *, field: str) -> str:
"""Return ``(hour, minute)`` for a strict ``HH:MM`` string. """Validate a strict ``HH:MM`` string and return it; raise naming ``field``.
Raises :class:`ValueError` for anything that is not a string or does Uses the shared :data:`~.const.TIME_PATTERN` (the same compiled
not match :data:`_HHMM_PATTERN` — the same rule the options flow regex the options flow applies), so a stored option and a freshly
applies, surfaced here so a malformed mapping cannot construct a validated form submit parse the same way. ``None`` (and any
settings object with an unparsable time. non-string) is rejected; the ``None``-as-absent mapping lives ONLY
in :meth:`NestQuestSettings.from_options` (via :func:`_option_value`).
""" """
if not isinstance(value, str) or _HHMM_PATTERN.fullmatch(value) is None: if not isinstance(value, str) or TIME_PATTERN.fullmatch(value) is None:
raise ValueError(f"expected a strict HH:MM string, got {value!r}") raise ValueError(
hour_str, _, minute_str = value.partition(":") f"{field} must be a strict HH:MM string, got {value!r}"
return int(hour_str), int(minute_str) )
return value
def _resolve_horizon_days(value: Any) -> int: def _resolve_horizon_days(value: Any) -> int:
"""Validate a horizon value and return the resolved day count. """Validate a horizon value and return the resolved day count.
Mirrors :func:`~.materialize._resolve_horizon_days`: a bool is A bool is rejected (SQLite binds ``True``/``False`` onto an int), a
rejected (SQLite binds ``True``/``False`` onto an int), a non-int non-int is rejected, and a sub-1 value is rejected — so a malformed
is rejected, and a sub-1 value is rejected — so a malformed option option can never shrink or explode the materialized horizon.
can never shrink or explode the materialized horizon. ``None`` ``None`` is rejected here too; the ``None``-as-absent mapping lives
resolves to :data:`~.const.DEFAULT_HORIZON_DAYS`. ONLY in :meth:`NestQuestSettings.from_options` (via
:func:`_option_value`), so a hand-built
:class:`NestQuestSettings(horizon_days=None)` raises in
:meth:`NestQuestSettings.__post_init__` rather than silently
constructing an object whose :meth:`NestQuestSettings.horizon_window`
would later raise ``TypeError`` on ``timedelta(days=None)``.
""" """
if value is None:
return DEFAULT_HORIZON_DAYS
if isinstance(value, bool) or not isinstance(value, int) or value < 1: if isinstance(value, bool) or not isinstance(value, int) or value < 1:
raise ValueError( raise ValueError(
f"horizon_days must be a positive integer, got {value!r}" f"horizon_days must be a positive integer, got {value!r}"
@@ -103,30 +118,32 @@ def _resolve_horizon_days(value: Any) -> int:
return value return value
def _resolve_bool(value: Any, *, default: bool) -> bool: def _resolve_bool(value: Any) -> bool:
"""Return a real bool, falling back to ``default`` when ``value`` is None. """Return a real bool; a non-bool (including ``None``) is rejected.
A non-bool, non-None value is rejected — voluptuous ``bool`` would voluptuous ``bool`` would coerce strings, so the options flow uses a
coerce strings, so the options flow uses ``_strict_bool`` and this strict-bool validator and this mirrors it. The ``None``-as-absent
mirrors it. mapping lives ONLY in :meth:`NestQuestSettings.from_options` (via
:func:`_option_value`).
""" """
if value is None:
return default
if not isinstance(value, bool): if not isinstance(value, bool):
raise ValueError(f"expected a boolean, got {value!r}") raise ValueError(f"expected a boolean, got {value!r}")
return value return value
def _resolve_optional_str(value: Any) -> str: def _resolve_optional_str(value: Any) -> str:
"""Return a stripped notify target string; empty means "not configured". """Return a stripped notify target string; a non-str is rejected.
A non-str value is rejected. An empty (or whitespace-only) value A padded or whitespace-only value is STRIPPED to its trim — the
is kept as the empty string — the shipped default, meaning no shipped default is the empty string, meaning "no notifications are
notifications are configured yet — rather than raising, mirroring configured yet". This is intentionally MORE LENIENT than the options
the options flow's empty-target acceptance. flow, which REJECTS a target that differs from its own trim (so a
padded value never persists): the integration tolerates a stored
padded value (e.g. one migrated from an older schema) by normalizing
it rather than crashing setup. The ``None``-as-absent mapping lives
ONLY in :meth:`NestQuestSettings.from_options` (via
:func:`_option_value`).
""" """
if value is None:
return ""
if not isinstance(value, str): if not isinstance(value, str):
raise ValueError(f"notify_target must be a string, got {value!r}") raise ValueError(f"notify_target must be a string, got {value!r}")
return value.strip() return value.strip()
@@ -137,9 +154,11 @@ class NestQuestSettings:
"""Explicit, HA-free configuration carried into core paths. """Explicit, HA-free configuration carried into core paths.
The integration builds this once from ``entry.options`` (via The integration builds this once from ``entry.options`` (via
:meth:`from_options`) and threads it into the horizon materialization :meth:`from_options`, or :meth:`from_options_resilient` for the
and the daily-rollover scheduling. Core never reads HA config per-field fallback used at setup) and threads it into the horizon
itself; callers pass the resolved horizon and rollover time in. materialization and the daily-rollover scheduling. Core never
reads HA config itself; callers pass the resolved horizon and
rollover time in.
All fields are validated at construction; the dataclass is frozen so All fields are validated at construction; the dataclass is frozen so
a stale settings object cannot be mutated in place after the a stale settings object cannot be mutated in place after the
@@ -158,17 +177,25 @@ class NestQuestSettings:
celebration_enabled: bool = DEFAULT_AUTOMATION_ENABLED celebration_enabled: bool = DEFAULT_AUTOMATION_ENABLED
def __post_init__(self) -> None: def __post_init__(self) -> None:
# Validate at construction so a hand-built instance cannot carry # Validate STRICTLY at construction so a hand-built instance
# an invalid value either: the same rules from_options applies. # cannot carry an invalid value: the same rules from_options
# applies, but WITHOUT the None->default mapping (that lives
# only in from_options via _option_value). A None horizon, time,
# toggle, or notify target raises here rather than later blowing
# up horizon_window or the rollover listener.
_resolve_horizon_days(self.horizon_days) _resolve_horizon_days(self.horizon_days)
_parse_hhmm(self.day_rollover_time) _validate_hhmm(self.day_rollover_time, field="day_rollover_time")
if not isinstance(self.notify_target, str): if not isinstance(self.notify_target, str):
raise ValueError( raise ValueError(
f"notify_target must be a string, got {self.notify_target!r}" f"notify_target must be a string, got {self.notify_target!r}"
) )
_parse_hhmm(self.morning_summary_time) _validate_hhmm(self.morning_summary_time, field="morning_summary_time")
_parse_hhmm(self.afternoon_reminder_time) _validate_hhmm(
_parse_hhmm(self.end_of_day_report_time) self.afternoon_reminder_time, field="afternoon_reminder_time"
)
_validate_hhmm(
self.end_of_day_report_time, field="end_of_day_report_time"
)
for name in ( for name in (
"morning_summary_enabled", "morning_summary_enabled",
"afternoon_reminder_enabled", "afternoon_reminder_enabled",
@@ -184,22 +211,25 @@ class NestQuestSettings:
@property @property
def day_rollover_hour_minute(self) -> tuple[int, int]: def day_rollover_hour_minute(self) -> tuple[int, int]:
"""The (hour, minute) the daily rollover fires at.""" """The (hour, minute) the daily rollover fires at."""
return _parse_hhmm(self.day_rollover_time) # The string is validated at construction, so a plain partition
# is safe and avoids re-running the regex on every access.
hour_str, _, minute_str = self.day_rollover_time.partition(":")
return int(hour_str), int(minute_str)
def horizon_window( def horizon_window(
self, today: datetime.date | None = None self, today: datetime.date
) -> tuple[datetime.date, datetime.date]: ) -> tuple[datetime.date, datetime.date]:
"""Return the rolling horizon window ``[today, today + horizon_days]``. """Return the rolling horizon window ``[today, today + horizon_days]``.
``today`` defaults to the host clock's date ``today`` is REQUIRED (there is no host-clock default): the
(:func:`datetime.date.today`), matching the historical materialize integration passes the HA-local ``hass.config.time_zone`` date so
default. The integration overrides this with the HA-local the horizon tracks the household's own day, and a silent
``hass.config.time_zone`` date so the horizon tracks the host-clock fallback would be a wrong-day footgun (a test or a
household's own day. The returned window is closed/inclusive — caller that forgets to pass ``today`` would otherwise materialize
the same shape :func:`~.materialize.materialize` walks. against the machine's calendar, not the household's). The
returned window is closed/inclusive — the same shape
:func:`~.materialize.materialize` walks.
""" """
if today is None:
today = datetime.date.today()
end = today + datetime.timedelta(days=self.horizon_days) end = today + datetime.timedelta(days=self.horizon_days)
return today, end return today, end
@@ -210,12 +240,16 @@ class NestQuestSettings:
"""Build settings from a CONF_*-keyed mapping (e.g. ``entry.options``). """Build settings from a CONF_*-keyed mapping (e.g. ``entry.options``).
Reads the same keys the options flow persists, applying the Reads the same keys the options flow persists, applying the
matching defaults when a key is absent and validating the same matching defaults when a key is absent OR ``None`` (via
way the options flow does: ``horizon_days`` is a positive int :func:`_option_value`, the SOLE place the ``None``-as-absent
(bool rejected), the four times are strict ``HH:MM``, the four mapping lives) and validating STRICTLY: ``horizon_days`` is a
toggles are real booleans, and ``notify_target`` is a string. positive int (bool rejected), the four times are strict
A ``None`` mapping yields the all-defaults settings — useful for ``HH:MM``, the four toggles are real booleans, and
tests and for a fresh install whose options dict is empty. ``notify_target`` is a string (stripped). A malformed value
RAISES — the integration uses :meth:`from_options_resilient` at
setup to fall back per field instead of crashing. A ``None``
mapping yields the all-defaults settings — useful for tests and
for a fresh install whose options dict is empty.
``options`` is consumed read-only via ``options`` is consumed read-only via
:class:`types.MappingProxyType`, so this method never mutates :class:`types.MappingProxyType`, so this method never mutates
@@ -228,20 +262,24 @@ class NestQuestSettings:
horizon_days=_resolve_horizon_days( horizon_days=_resolve_horizon_days(
_option_value(opts, CONF_HORIZON_DAYS, DEFAULT_HORIZON_DAYS) _option_value(opts, CONF_HORIZON_DAYS, DEFAULT_HORIZON_DAYS)
), ),
day_rollover_time=_parse_hhmm_str( day_rollover_time=_validate_hhmm(
_option_value(opts, CONF_DAY_ROLLOVER_TIME, DEFAULT_DAY_ROLLOVER_TIME), _option_value(
opts, CONF_DAY_ROLLOVER_TIME, DEFAULT_DAY_ROLLOVER_TIME
),
field="day_rollover_time", field="day_rollover_time",
), ),
notify_target=_resolve_optional_str( notify_target=_resolve_optional_str(
_option_value(opts, CONF_NOTIFY_TARGET, "") _option_value(opts, CONF_NOTIFY_TARGET, "")
), ),
morning_summary_time=_parse_hhmm_str( morning_summary_time=_validate_hhmm(
_option_value( _option_value(
opts, CONF_MORNING_SUMMARY_TIME, DEFAULT_MORNING_SUMMARY_TIME opts,
CONF_MORNING_SUMMARY_TIME,
DEFAULT_MORNING_SUMMARY_TIME,
), ),
field="morning_summary_time", field="morning_summary_time",
), ),
afternoon_reminder_time=_parse_hhmm_str( afternoon_reminder_time=_validate_hhmm(
_option_value( _option_value(
opts, opts,
CONF_AFTERNOON_REMINDER_TIME, CONF_AFTERNOON_REMINDER_TIME,
@@ -249,7 +287,7 @@ class NestQuestSettings:
), ),
field="afternoon_reminder_time", field="afternoon_reminder_time",
), ),
end_of_day_report_time=_parse_hhmm_str( end_of_day_report_time=_validate_hhmm(
_option_value( _option_value(
opts, opts,
CONF_END_OF_DAY_REPORT_TIME, CONF_END_OF_DAY_REPORT_TIME,
@@ -260,43 +298,128 @@ class NestQuestSettings:
morning_summary_enabled=_resolve_bool( morning_summary_enabled=_resolve_bool(
_option_value( _option_value(
opts, CONF_MORNING_SUMMARY_ENABLED, DEFAULT_AUTOMATION_ENABLED opts, CONF_MORNING_SUMMARY_ENABLED, DEFAULT_AUTOMATION_ENABLED
), )
default=DEFAULT_AUTOMATION_ENABLED,
), ),
afternoon_reminder_enabled=_resolve_bool( afternoon_reminder_enabled=_resolve_bool(
_option_value( _option_value(
opts, opts,
CONF_AFTERNOON_REMINDER_ENABLED, CONF_AFTERNOON_REMINDER_ENABLED,
DEFAULT_AUTOMATION_ENABLED, DEFAULT_AUTOMATION_ENABLED,
), )
default=DEFAULT_AUTOMATION_ENABLED,
), ),
end_of_day_report_enabled=_resolve_bool( end_of_day_report_enabled=_resolve_bool(
_option_value( _option_value(
opts, opts,
CONF_END_OF_DAY_REPORT_ENABLED, CONF_END_OF_DAY_REPORT_ENABLED,
DEFAULT_AUTOMATION_ENABLED, DEFAULT_AUTOMATION_ENABLED,
), )
default=DEFAULT_AUTOMATION_ENABLED,
), ),
celebration_enabled=_resolve_bool( celebration_enabled=_resolve_bool(
_option_value( _option_value(
opts, CONF_CELEBRATION_ENABLED, DEFAULT_AUTOMATION_ENABLED opts, CONF_CELEBRATION_ENABLED, DEFAULT_AUTOMATION_ENABLED
), )
default=DEFAULT_AUTOMATION_ENABLED,
), ),
) )
@classmethod
def from_options_resilient(
cls, options: Mapping[str, Any] | None = None
) -> "NestQuestSettings":
"""Build settings, falling back PER FIELD on a malformed value.
def _parse_hhmm_str(value: Any, *, field: str) -> str: Unlike :meth:`from_options` (which raises on the first invalid
"""Return a validated ``HH:MM`` string, raising on a malformed value. field), this resolves EACH field in its own try/except: a
malformed value logs a :data:`~.const.LOGGER` warning naming the
field and falls back to that field's default. The integration
uses this at setup so a single bad sibling option (e.g. a
migrated ``morning_summary_time`` of ``'25:00'``) cannot
silently reset the user's ``horizon_days`` or
``day_rollover_time`` — the two fields core actually consumes.
A separate helper from :func:`_parse_hhmm` because :meth:`from_options` An absent or ``None`` key maps to the default via
needs the original string kept (the dataclass stores strings, not :func:`_option_value` WITHOUT a warning (that is the normal
tuples), but still wants the same strict validation. missing-key path); only a present, non-``None``, invalid value
""" triggers a warning and a per-field fallback.
if not isinstance(value, str) or _HHMM_PATTERN.fullmatch(value) is None: """
raise ValueError( opts: Mapping[str, Any] = (
f"{field} must be a strict HH:MM string, got {value!r}" MappingProxyType({}) if options is None else MappingProxyType(options)
) )
return value resolved: dict[str, Any] = {}
def _resolve(
field: str, key: str, default: Any, fn: Callable[[Any], Any]
) -> None:
raw = _option_value(opts, key, default)
try:
resolved[field] = fn(raw)
except ValueError as err:
LOGGER.warning(
"NestQuest option %s is invalid (%s); falling back "
"to the default %r",
field,
err,
default,
)
resolved[field] = default
_resolve(
"horizon_days",
CONF_HORIZON_DAYS,
DEFAULT_HORIZON_DAYS,
_resolve_horizon_days,
)
_resolve(
"day_rollover_time",
CONF_DAY_ROLLOVER_TIME,
DEFAULT_DAY_ROLLOVER_TIME,
lambda v: _validate_hhmm(v, field="day_rollover_time"),
)
_resolve(
"notify_target",
CONF_NOTIFY_TARGET,
"",
_resolve_optional_str,
)
_resolve(
"morning_summary_time",
CONF_MORNING_SUMMARY_TIME,
DEFAULT_MORNING_SUMMARY_TIME,
lambda v: _validate_hhmm(v, field="morning_summary_time"),
)
_resolve(
"afternoon_reminder_time",
CONF_AFTERNOON_REMINDER_TIME,
DEFAULT_AFTERNOON_REMINDER_TIME,
lambda v: _validate_hhmm(v, field="afternoon_reminder_time"),
)
_resolve(
"end_of_day_report_time",
CONF_END_OF_DAY_REPORT_TIME,
DEFAULT_END_OF_DAY_REPORT_TIME,
lambda v: _validate_hhmm(v, field="end_of_day_report_time"),
)
_resolve(
"morning_summary_enabled",
CONF_MORNING_SUMMARY_ENABLED,
DEFAULT_AUTOMATION_ENABLED,
_resolve_bool,
)
_resolve(
"afternoon_reminder_enabled",
CONF_AFTERNOON_REMINDER_ENABLED,
DEFAULT_AUTOMATION_ENABLED,
_resolve_bool,
)
_resolve(
"end_of_day_report_enabled",
CONF_END_OF_DAY_REPORT_ENABLED,
DEFAULT_AUTOMATION_ENABLED,
_resolve_bool,
)
_resolve(
"celebration_enabled",
CONF_CELEBRATION_ENABLED,
DEFAULT_AUTOMATION_ENABLED,
_resolve_bool,
)
return cls(**resolved)
+2 -4
View File
@@ -1,7 +1,6 @@
"""Options flow for NestQuest post-setup settings.""" """Options flow for NestQuest post-setup settings."""
from __future__ import annotations from __future__ import annotations
import re
from typing import Any from typing import Any
import voluptuous as vol import voluptuous as vol
@@ -33,18 +32,17 @@ from .const import (
DEFAULT_UPDATE_INTERVAL, DEFAULT_UPDATE_INTERVAL,
DOMAIN, DOMAIN,
MIN_UPDATE_INTERVAL, MIN_UPDATE_INTERVAL,
TIME_PATTERN,
) )
from .admin_allowlist import list_admin_ids, set_admin_ids from .admin_allowlist import list_admin_ids, set_admin_ids
MIN_HORIZON_DAYS = 1 MIN_HORIZON_DAYS = 1
MIN_PANEL_IDLE_TIMEOUT = 30 MIN_PANEL_IDLE_TIMEOUT = 30
_TIME_PATTERN = re.compile(r"(?:[01]\d|2[0-3]):[0-5]\d")
def _parse_time(value: str) -> bool: def _parse_time(value: str) -> bool:
"""Return True when value is a valid 24-hour HH:MM string.""" """Return True when value is a valid 24-hour HH:MM string."""
return _TIME_PATTERN.fullmatch(value) is not None return TIME_PATTERN.fullmatch(value) is not None
def _async_validate(user_input: dict[str, Any]) -> dict[str, str]: def _async_validate(user_input: dict[str, Any]) -> dict[str, str]:
+455 -141
View File
@@ -1,26 +1,31 @@
"""Tests for core/settings.py: the explicit, HA-free settings object. """Tests for core/settings.py: the explicit, HA-free settings object.
Covers three contracts from task 4dd8f870: Covers the contracts from task 4dd8f870 plus the review fixes:
- ``NestQuestSettings.from_options`` applies defaults when keys are - ``NestQuestSettings.from_options`` applies defaults when keys are
absent and rejects invalid horizon/rollover values. absent and rejects invalid horizon/rollover values; ``from_options``
is strict (raises), while ``from_options_resilient`` falls back PER
FIELD so a malformed sibling cannot reset horizon_days/rollover.
- An AST scan asserts no module under - An AST scan asserts no module under
``custom_components/nestquest/core/`` imports ``homeassistant``, ``custom_components/nestquest/core/`` imports ``homeassistant``,
accesses a ``hass.config`` / config-entry attribute, or reads names ``hass``/``config_entry``, reads a ``.config``/``.options``/
config-entry options. (Docstring mentions are allowed.) ``.data`` attribute, or dynamically reads them via ``getattr``/
``hasattr``. (Docstring mentions are allowed.) A negative-control
test feeds the scanner each bypass shape and asserts it flags them.
- A settings object constructed with NON-default ``horizon_days`` and - A settings object constructed with NON-default ``horizon_days`` and
``day_rollover_time`` drives a materialization whose resulting window ``day_rollover_time`` drives a materialization whose resulting window
matches the non-default horizon. matches the non-default horizon — via BOTH the core ``materialize``
path and the INTEGRATION path (``_run_horizon_materialization`` +
``_register_day_rollover_listener``).
""" """
from __future__ import annotations from __future__ import annotations
import ast import ast
import datetime import datetime
import inspect
from dataclasses import FrozenInstanceError from dataclasses import FrozenInstanceError
from pathlib import Path from pathlib import Path
from types import SimpleNamespace
from typing import Any from typing import Any
from unittest.mock import MagicMock
import pytest import pytest
@@ -206,6 +211,97 @@ def test_from_options_strips_notify_target() -> None:
assert s_empty.notify_target == "" assert s_empty.notify_target == ""
# ---------------------------------------------------------------------------
# from_options_resilient: per-field fallback (P2-1).
# ---------------------------------------------------------------------------
def test_from_options_resilient_none_yields_all_defaults() -> None:
"""A None mapping yields the all-defaults settings (no warnings)."""
s = NestQuestSettings.from_options_resilient(None)
assert s.horizon_days == DEFAULT_HORIZON_DAYS
assert s.day_rollover_time == DEFAULT_DAY_ROLLOVER_TIME
def test_from_options_resilient_reads_valid_keys() -> None:
"""Valid keys are read exactly as from_options would read them."""
s = NestQuestSettings.from_options_resilient(
{CONF_HORIZON_DAYS: 3, CONF_DAY_ROLLOVER_TIME: "03:30"}
)
assert s.horizon_days == 3
assert s.day_rollover_time == "03:30"
def test_from_options_resilient_keeps_horizon_when_sibling_is_bad(
caplog: pytest.LogCaptureFixture,
) -> None:
"""A malformed SIBLING field cannot reset horizon_days or day_rollover_time.
The old all-or-nothing fallback reset EVERY field when from_options
raised for ANY field: ``{horizon_days: 3, morning_summary_time:
'25:00'}`` yielded horizon 14 (was 3). The resilient builder falls
back PER FIELD, so horizon_days=3 and day_rollover_time='03:30'
survive a malformed morning_summary_time, and the bad field alone
falls back to its default with a warning naming it.
"""
s = NestQuestSettings.from_options_resilient(
{
CONF_HORIZON_DAYS: 3,
CONF_DAY_ROLLOVER_TIME: "03:30",
CONF_MORNING_SUMMARY_TIME: "25:00",
}
)
assert s.horizon_days == 3
assert s.day_rollover_time == "03:30"
# The malformed sibling fell back to its default.
assert s.morning_summary_time == DEFAULT_MORNING_SUMMARY_TIME
# A warning was logged naming the bad field.
assert any(
"morning_summary_time" in record.getMessage()
and record.levelname == "WARNING"
for record in caplog.records
)
def test_from_options_resilient_bad_horizon_falls_back_with_warning(
caplog: pytest.LogCaptureFixture,
) -> None:
"""A malformed horizon_days itself falls back to the default + warning."""
s = NestQuestSettings.from_options_resilient({CONF_HORIZON_DAYS: True})
assert s.horizon_days == DEFAULT_HORIZON_DAYS
assert any(
"horizon_days" in record.getMessage() and record.levelname == "WARNING"
for record in caplog.records
)
def test_from_options_resilient_bad_rollover_falls_back_with_warning(
caplog: pytest.LogCaptureFixture,
) -> None:
"""A malformed day_rollover_time falls back to the default + warning."""
s = NestQuestSettings.from_options_resilient(
{CONF_DAY_ROLLOVER_TIME: "99:99"}
)
assert s.day_rollover_time == DEFAULT_DAY_ROLLOVER_TIME
assert any(
"day_rollover_time" in record.getMessage()
and record.levelname == "WARNING"
for record in caplog.records
)
def test_from_options_resilient_none_value_is_not_a_warning(
caplog: pytest.LogCaptureFixture,
) -> None:
"""A None value maps to the default silently (it is absent, not malformed)."""
s = NestQuestSettings.from_options_resilient(
{CONF_HORIZON_DAYS: None, CONF_MORNING_SUMMARY_TIME: None}
)
assert s.horizon_days == DEFAULT_HORIZON_DAYS
assert s.morning_summary_time == DEFAULT_MORNING_SUMMARY_TIME
assert not [r for r in caplog.records if r.levelname == "WARNING"]
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
# Construction + frozenness. # Construction + frozenness.
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@@ -221,6 +317,42 @@ def test_direct_construction_validates() -> None:
NestQuestSettings(morning_summary_enabled="yes") NestQuestSettings(morning_summary_enabled="yes")
@pytest.mark.parametrize(
"field,kwargs",
[
("horizon_days", {"horizon_days": None}),
("day_rollover_time", {"day_rollover_time": None}),
("notify_target", {"notify_target": None}),
("morning_summary_time", {"morning_summary_time": None}),
("afternoon_reminder_time", {"afternoon_reminder_time": None}),
("end_of_day_report_time", {"end_of_day_report_time": None}),
("morning_summary_enabled", {"morning_summary_enabled": None}),
("afternoon_reminder_enabled", {"afternoon_reminder_enabled": None}),
("end_of_day_report_enabled", {"end_of_day_report_enabled": None}),
("celebration_enabled", {"celebration_enabled": None}),
],
)
def test_direct_construction_rejects_none(field: str, kwargs: dict[str, Any]) -> None:
"""__post_init__ is STRICT: None is rejected for every field.
P2-5: the None->default mapping lives ONLY in from_options (via
_option_value). A hand-built NestQuestSettings(field=None) must raise
in __post_init__ rather than constructing and later blowing up
horizon_window (timedelta(days=None) -> TypeError) or the rollover
listener.
"""
with pytest.raises(ValueError):
NestQuestSettings(**kwargs)
@pytest.mark.parametrize("value", [True, False, "5", 1.5])
def test_direct_construction_rejects_non_int_horizon(value: Any) -> None:
"""A bool or non-int horizon is rejected at construction (not bool-as-int)."""
with pytest.raises(ValueError, match="horizon_days"):
NestQuestSettings(horizon_days=value)
def test_settings_is_frozen() -> None: def test_settings_is_frozen() -> None:
"""The dataclass is frozen so a stale settings object cannot be mutated.""" """The dataclass is frozen so a stale settings object cannot be mutated."""
s = NestQuestSettings() s = NestQuestSettings()
@@ -240,17 +372,12 @@ def test_day_rollover_hour_minute() -> None:
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
def test_horizon_window_default_today() -> None:
"""horizon_window returns [today, today + horizon_days] when today is omitted."""
s = NestQuestSettings(horizon_days=7)
start, end = s.horizon_window()
today = datetime.date.today()
assert start == today
assert end == today + datetime.timedelta(days=7)
def test_horizon_window_pinned_today() -> None: def test_horizon_window_pinned_today() -> None:
"""A pinned today is honored so the window is deterministic.""" """A pinned today is honored so the window is deterministic.
``today`` is REQUIRED (no host-clock default) — see
:meth:`NestQuestSettings.horizon_window`.
"""
s = NestQuestSettings(horizon_days=5) s = NestQuestSettings(horizon_days=5)
pinned = datetime.date(2026, 3, 1) pinned = datetime.date(2026, 3, 1)
start, end = s.horizon_window(pinned) start, end = s.horizon_window(pinned)
@@ -258,6 +385,13 @@ def test_horizon_window_pinned_today() -> None:
assert end == pinned + datetime.timedelta(days=5) assert end == pinned + datetime.timedelta(days=5)
def test_horizon_window_requires_today() -> None:
"""horizon_window has no default today: omitting it raises TypeError."""
s = NestQuestSettings(horizon_days=5)
with pytest.raises(TypeError):
s.horizon_window() # type: ignore[call-arg]
def test_horizon_window_non_default_horizon() -> None: def test_horizon_window_non_default_horizon() -> None:
"""A non-default horizon_days widens the window accordingly.""" """A non-default horizon_days widens the window accordingly."""
pinned = datetime.date(2026, 3, 1) pinned = datetime.date(2026, 3, 1)
@@ -283,37 +417,57 @@ def _is_homeassistant(module: str) -> bool:
return module == "homeassistant" or module.startswith("homeassistant.") return module == "homeassistant" or module.startswith("homeassistant.")
def _attribute_chain(node: ast.Attribute) -> list[str]: #: HA-specific identifiers with no legitimate core use. ``entry`` is
"""Flatten an attribute access chain into dotted-name parts (leaf first).""" #: deliberately NOT in this set: core uses ``entry`` as a domain loop
parts: list[str] = [node.attr] #: variable (iterating weekday_set entries in recurrence/presence/
cur: ast.expr = node.value #: quest_definitions), so forbidding the bare name would false-positive.
while isinstance(cur, ast.Attribute): #: The config-entry attribute reads the bare-name check misses are
parts.append(cur.attr) #: caught by _CONFIG_ATTRS and _FORBIDDEN_ATTRS below.
cur = cur.value _FORBIDDEN_NAMES = frozenset({"hass", "config_entry"})
if isinstance(cur, ast.Name):
parts.append(cur.id) #: Attribute names that signal a HA object reference (self.hass,
return parts #: self.entry, obj.config_entry). No core module currently uses any of
#: these as an attribute, so flagging them is false-positive-free.
_FORBIDDEN_ATTRS = frozenset({"hass", "entry", "config_entry"})
#: Config-entry attribute reads — the shapes ``hass.config``,
#: ``config_entry.options``, ``self.entry.options``, ``runtime.options``,
#: ``entry.data``. Flagging the attribute ANYWHERE (regardless of root)
#: is what catches ``runtime.options``, which the old root-only check
#: missed. No core module currently reads any of these attributes.
_CONFIG_ATTRS = frozenset({"config", "options", "data"})
#: Dynamic config-entry reads via getattr/hasattr with a string argument.
_GETATTR_FUNCS = frozenset({"getattr", "hasattr"})
def _dotted_attr(node: ast.Attribute) -> str: def _scan_source_for_ha_config(
"""Return the dotted name of an attribute chain, root first.""" source: str, *, name: str = "<snippet>"
return ".".join(reversed(_attribute_chain(node))) ) -> list[str]:
"""Return offender strings for HA-coupling in ``source``.
Flags (each closes a bypass the old root-only ``hass``/``entry``
chain check missed):
def _scan_module_for_ha_config(path: Path) -> list[str]:
"""Return a list of offender strings for HA-coupling in ``path``.
Flags:
- any ``import homeassistant`` / ``from homeassistant ...``; - any ``import homeassistant`` / ``from homeassistant ...``;
- any attribute access whose chain reaches ``hass.config`` or any - any ``Name``/``arg`` whose id is ``hass`` or ``config_entry``
``.options``/``.data`` access on a name literally named ``entry`` (HA-specific identifiers with no legitimate core use);
(a config-entry attribute read), EXCEPT inside a docstring-only - any attribute access whose ``attr`` is ``hass``/``entry``/
module (no runtime access) — but since AST only sees code, not ``config_entry`` (catches ``self.hass``, ``self.entry``,
docstrings, docstring mentions are structurally exempt. ``obj.config_entry`` — root names the old check could not see);
- any call to ``.options.get(...)`` / ``.data.get(...)`` on a name - any attribute access whose ``attr`` is ``config``/``options``/
literally named ``entry`` (a config-entry options/data read). ``data`` (catches ``hass.config``, ``config_entry.options``,
``self.entry.options``, ``runtime.options`` regardless of root);
- any ``getattr``/``hasattr`` call whose string argument is
``'config'``/``'options'``/``'data'`` (catches the dynamic read
``getattr(hass, 'config')`` the static attribute check cannot see).
Docstring mentions are structurally exempt: AST only sees code, and
none of these node shapes appear inside a string literal. The
existing docstring references to ``hass.config`` in ``core/events.py``
and ``core/store.py`` therefore pass.
""" """
tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) tree = ast.parse(source, filename=name)
offenders: list[str] = [] offenders: list[str] = []
for node in ast.walk(tree): for node in ast.walk(tree):
@@ -321,51 +475,75 @@ def _scan_module_for_ha_config(path: Path) -> list[str]:
for alias in node.names: for alias in node.names:
if _is_homeassistant(alias.name): if _is_homeassistant(alias.name):
offenders.append( offenders.append(
f"{path.name}:{node.lineno} homeassistant import: " f"{name}:{node.lineno} homeassistant import: "
f"import {alias.name}" f"import {alias.name}"
) )
elif isinstance(node, ast.ImportFrom): elif isinstance(node, ast.ImportFrom):
base = node.module or "" base = node.module or ""
if _is_homeassistant(base): if _is_homeassistant(base):
offenders.append( offenders.append(
f"{path.name}:{node.lineno} homeassistant import: from {base}" f"{name}:{node.lineno} homeassistant import: from {base}"
) )
for alias in node.names: for alias in node.names:
combined = ( combined = f"{base}.{alias.name}" if base else alias.name
f"{base}.{alias.name}" if base else alias.name
)
if _is_homeassistant(combined): if _is_homeassistant(combined):
offenders.append( offenders.append(
f"{path.name}:{node.lineno} homeassistant import: " f"{name}:{node.lineno} homeassistant import: "
f"from {base} import {alias.name}" f"from {base} import {alias.name}"
) )
elif isinstance(node, ast.Name):
if node.id in _FORBIDDEN_NAMES:
offenders.append(
f"{name}:{node.lineno} forbidden identifier: {node.id!r}"
)
elif isinstance(node, ast.arg):
if node.arg in _FORBIDDEN_NAMES:
offenders.append(
f"{name}:{node.lineno} forbidden parameter: {node.arg!r}"
)
elif isinstance(node, ast.Attribute): elif isinstance(node, ast.Attribute):
dotted = _dotted_attr(node) if node.attr in _FORBIDDEN_ATTRS:
# hass.config.<anything> — a HA config read.
if dotted == "hass.config" or dotted.startswith("hass.config."):
offenders.append( offenders.append(
f"{path.name}:{node.lineno} hass.config access: {dotted}" f"{name}:{node.lineno} forbidden attribute: .{node.attr}"
) )
# entry.options / entry.data — a config-entry attribute read. if node.attr in _CONFIG_ATTRS:
# Only flag the leaf attribute itself (not every sub-expression),
# and only when the root name is literally 'entry'.
root = _attribute_chain(node)[-1]
if root == "entry" and node.attr in {"options", "data"}:
offenders.append( offenders.append(
f"{path.name}:{node.lineno} config-entry attribute read: " f"{name}:{node.lineno} config-entry attribute read: "
f"{dotted}" f".{node.attr}"
) )
elif isinstance(node, ast.Call):
func = node.func
if isinstance(func, ast.Name) and func.id in _GETATTR_FUNCS:
for arg in node.args:
if (
isinstance(arg, ast.Constant)
and isinstance(arg.value, str)
and arg.value in _CONFIG_ATTRS
):
offenders.append(
f"{name}:{node.lineno} {func.id}() dynamic "
f"config-entry read: {arg.value!r}"
)
return offenders return offenders
def _scan_module_for_ha_config(path: Path) -> list[str]:
"""Scan a core module file for HA-coupling (file-backed scan)."""
return _scan_source_for_ha_config(
path.read_text(encoding="utf-8"), name=path.name
)
def test_core_modules_have_no_ha_config_reads() -> None: def test_core_modules_have_no_ha_config_reads() -> None:
"""No core module imports homeassistant or reads HA config / entry options. """No core module imports homeassistant or reads HA config / entry options.
Docstring mentions are allowed: AST only sees code, and the offenders The scan forbids the HA-specific identifiers ``hass``/``config_entry``,
this scan flags (imports, attribute accesses) never appear in a the HA-object attributes ``.hass``/``.entry``/``.config_entry``, the
docstring. The existing docstring references to ``hass.config`` in config-entry attribute reads ``.config``/``.options``/``.data``, and
``core/events.py`` and ``core/store.py`` therefore pass. dynamic ``getattr``/``hasattr`` reads of them — so a chain rooted at
``self`` or ``runtime`` (which the old root-only check missed) is now
caught. Docstring mentions pass: AST only sees code.
""" """
py_files = sorted(CORE_PKG.rglob("*.py")) py_files = sorted(CORE_PKG.rglob("*.py"))
assert py_files, "core package not found" assert py_files, "core package not found"
@@ -378,6 +556,49 @@ def test_core_modules_have_no_ha_config_reads() -> None:
) )
def test_ast_scan_flags_each_bypass_shape() -> None:
"""Negative control: the scanner flags each bypass shape the old check missed.
The old scan only flagged chains rooted at a literal name ``hass``/
``entry``. Each snippet below is a shape it missed; the new scanner
must flag every one.
"""
snippets: dict[str, str] = {
"self.hass.config": "x = self.hass.config\n",
"config_entry.options": "x = config_entry.options\n",
"self.entry.options": "x = self.entry.options\n",
"getattr(hass,'config')": "x = getattr(hass, 'config')\n",
"runtime.options": "x = runtime.options\n",
"hasattr(obj,'data')": "x = hasattr(obj, 'data')\n",
}
for label, src in snippets.items():
offenders = _scan_source_for_ha_config(src, name=label)
assert offenders, f"scanner missed {label!r}: {src!r}"
def test_ast_scan_allows_legitimate_entry_loop_var() -> None:
"""A bare ``entry`` loop variable (a domain concept) is NOT flagged.
``entry`` is used in core as a loop variable over weekday_set entries
(recurrence.py, presence.py, quest_definitions.py) — not a HA config
entry. Forbidding the bare name would false-positive on those, so
only ``hass``/``config_entry`` are forbidden as bare names; the
config-entry reads via ``entry.options``/``entry.data`` are caught by
the ``_CONFIG_ATTRS`` attribute check instead.
"""
offenders = _scan_source_for_ha_config(
"for entry in items:\n pass\n", name="loop"
)
assert offenders == [], offenders
def test_ast_scan_allows_clean_core_shape() -> None:
"""A clean snippet with no HA coupling is not flagged (no false positives)."""
src = "opts = {'a': 1}\nx = opts.get('a')\nfor entry in items:\n pass\n"
offenders = _scan_source_for_ha_config(src, name="clean")
assert offenders == [], offenders
def test_settings_module_lives_under_core() -> None: def test_settings_module_lives_under_core() -> None:
"""settings.py is part of the core package (not the integration root).""" """settings.py is part of the core package (not the integration root)."""
assert (CORE_PKG / "settings.py").is_file() assert (CORE_PKG / "settings.py").is_file()
@@ -385,56 +606,39 @@ def test_settings_module_lives_under_core() -> None:
import custom_components.nestquest.core.settings as mod import custom_components.nestquest.core.settings as mod
assert hasattr(mod, "NestQuestSettings") assert hasattr(mod, "NestQuestSettings")
assert inspect.isclass(mod.NestQuestSettings) assert isinstance(mod.NestQuestSettings, type)
def test_settings_module_has_no_ha_imports() -> None: def test_settings_module_has_no_ha_imports() -> None:
"""settings.py specifically carries no homeassistant import.""" """settings.py specifically carries no homeassistant import or config read."""
offenders = _scan_module_for_ha_config(CORE_PKG / "settings.py") offenders = _scan_module_for_ha_config(CORE_PKG / "settings.py")
assert offenders == [], offenders assert offenders == [], offenders
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
# Materialization over a non-default horizon. # Materialization over a non-default horizon: core path + integration path.
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
def _make_hass_mock(): async def _prepare(path, executor):
from unittest.mock import AsyncMock """Open and migrate a database backed by ``executor`` (executor-only DB access)."""
hass = MagicMock()
hass.async_add_executor_job = AsyncMock(
side_effect=(lambda fn, *a, **k: fn(*a, **k))
)
return hass
async def _prepare(path):
from custom_components.nestquest.core.db import NestQuestDatabase from custom_components.nestquest.core.db import NestQuestDatabase
from custom_components.nestquest.core.migrations import apply_migrations from custom_components.nestquest.core.migrations import apply_migrations
database = NestQuestDatabase(_make_hass_mock().async_add_executor_job) database = NestQuestDatabase(executor)
await database.open(path) await database.open(path)
await apply_migrations(database) await apply_migrations(database)
return database return database
def _run(coro): async def test_materialize_over_non_default_horizon(hass, tmp_path) -> None:
import asyncio
return asyncio.new_event_loop().run_until_complete(coro)
def test_materialize_over_non_default_horizon(tmp_path) -> None:
"""A settings object with a non-default horizon drives the matching window. """A settings object with a non-default horizon drives the matching window.
Constructs NestQuestSettings with horizon_days=3 and day_rollover_time Constructs NestQuestSettings with horizon_days=3 and day_rollover_time
'03:30' (both non-default), builds the window via horizon_window, and '03:30' (both non-default), builds the window via horizon_window, and
drives materialize over it; asserts the resulting instance rows land drives the CORE ``materialize`` over it; asserts the resulting instance
exactly on the [today, today+3] window and NOT beyond it. rows land exactly on the [today, today+3] window and NOT beyond it.
""" """
import asyncio
from custom_components.nestquest.core.dao_children import ChildrenDao from custom_components.nestquest.core.dao_children import ChildrenDao
from custom_components.nestquest.core.dao_instances import ( from custom_components.nestquest.core.dao_instances import (
QuestInstancesDao, QuestInstancesDao,
@@ -448,57 +652,167 @@ def test_materialize_over_non_default_horizon(tmp_path) -> None:
ScheduleRule, ScheduleRule,
) )
async def _main(): database = await _prepare(tmp_path / "nq.db", hass.async_add_executor_job)
database = await _prepare(tmp_path / "nq.db") try:
settings = NestQuestSettings(
horizon_days=3, day_rollover_time="03:30"
)
assert settings.horizon_days == 3
assert settings.day_rollover_time == "03:30"
today = datetime.date(2026, 3, 1)
start, end = settings.horizon_window(today)
assert start == today
assert end == today + datetime.timedelta(days=3)
NOW = "2026-03-01T12:00:00+00:00"
child = await ChildrenDao(database).create("Ada", NOW)
await create_quest_definition(
database,
"Daily chore",
ScheduleRule(
rule_type=RuleType.DAILY, start_date=today.isoformat()
),
[child.id],
["morning"],
)
count = await materialize(
database,
start.isoformat(),
end.isoformat(),
today=today,
)
# 4 days (today..today+3 inclusive), one instance per day.
assert count == 4
records = await QuestInstancesDao(database).list_by_date_range(
child.id, start.isoformat(), end.isoformat()
)
dates = sorted({r.due_date for r in records})
assert dates == [
(today + datetime.timedelta(days=i)).isoformat()
for i in range(4)
]
# Beyond the configured horizon: no instances.
beyond = (end + datetime.timedelta(days=1)).isoformat()
far = (end + datetime.timedelta(days=10)).isoformat()
assert await QuestInstancesDao(database).list_by_date_range(
child.id, beyond, far
) == []
finally:
await database.close()
def _freeze_nestquest_clock(
monkeypatch, instant: datetime.datetime
) -> None:
"""Freeze the integration's ``datetime`` clock to a fixed aware instant.
Only ``custom_components.nestquest``'s ``datetime`` binding is patched,
so ``_run_horizon_materialization``'s ``datetime.datetime.now`` reads
this instant (localized to ``hass.config.time_zone``) while the test
harness and core keep the real clock. Mirrors the helper in
test_timezones.py.
"""
import custom_components.nestquest as nestquest
frozen = instant.astimezone(datetime.timezone.utc)
class _FrozenDatetime(datetime.datetime):
@classmethod
def now(cls, tz=None):
if tz is None:
return frozen.replace(tzinfo=None)
return frozen.astimezone(tz)
fake_module = SimpleNamespace(
datetime=_FrozenDatetime,
timedelta=datetime.timedelta,
date=datetime.date,
)
monkeypatch.setattr(nestquest, "datetime", fake_module)
async def test_integration_path_threads_settings(
hass, tmp_path, monkeypatch
) -> None:
"""The integration path threads ``settings`` through horizon_window + rollover.
P2-3: ``_run_horizon_materialization`` now calls
``settings.horizon_window(today)`` (so the helper is real and shared,
not recomputed inline), and ``_register_day_rollover_listener``
registers at ``settings.day_rollover_hour_minute``. This runs the
INTEGRATION path with ``horizon_days=3`` and
``day_rollover_time='03:30'`` and asserts the resulting instance
window matches [today, today+3] AND the registered hour/minute are 3/30
— a test the requirement-level core-only check could not fail.
"""
import custom_components.nestquest as nestquest
from custom_components.nestquest.core.dao_children import ChildrenDao
from custom_components.nestquest.core.dao_instances import (
QuestInstancesDao,
)
from custom_components.nestquest.core.quest_definitions import (
create_quest_definition,
)
from custom_components.nestquest.core.recurrence import (
RuleType,
ScheduleRule,
)
# Freeze the integration clock so "today" is deterministic (the
# hass fixture's config.time_zone is UTC, so 2026-03-01 12:00 UTC
# localizes to 2026-03-01).
_freeze_nestquest_clock(
monkeypatch, datetime.datetime(2026, 3, 1, 12, 0, tzinfo=datetime.timezone.utc)
)
database = await _prepare(tmp_path / "nq.db", hass.async_add_executor_job)
try:
settings = NestQuestSettings(horizon_days=3, day_rollover_time="03:30")
today = datetime.date(2026, 3, 1)
NOW = "2026-03-01T12:00:00+00:00"
child = await ChildrenDao(database).create("Ada", NOW)
await create_quest_definition(
database,
"Daily chore",
ScheduleRule(
rule_type=RuleType.DAILY, start_date=today.isoformat()
),
[child.id],
["morning"],
)
# Integration path: the window comes from settings.horizon_window.
await nestquest._run_horizon_materialization(hass, database, settings)
end = today + datetime.timedelta(days=3)
records = await QuestInstancesDao(database).list_by_date_range(
child.id, today.isoformat(), end.isoformat()
)
dates = sorted({r.due_date for r in records})
assert dates == [
(today + datetime.timedelta(days=i)).isoformat()
for i in range(4)
]
beyond = (end + datetime.timedelta(days=1)).isoformat()
far = (end + datetime.timedelta(days=10)).isoformat()
assert await QuestInstancesDao(database).list_by_date_range(
child.id, beyond, far
) == []
# The rollover listener registers at the configured hour/minute.
remove = nestquest._register_day_rollover_listener(
hass, database, settings
)
try: try:
settings = NestQuestSettings( reg = hass.time_change.registrations[-1]
horizon_days=3, day_rollover_time="03:30" assert reg["hour"] == 3
) assert reg["minute"] == 30
assert settings.horizon_days == 3
assert settings.day_rollover_time == "03:30"
today = datetime.date(2026, 3, 1)
start, end = settings.horizon_window(today)
assert start == today
assert end == today + datetime.timedelta(days=3)
NOW = "2026-03-01T12:00:00+00:00"
child = await ChildrenDao(database).create("Ada", NOW)
await create_quest_definition(
database,
"Daily chore",
ScheduleRule(
rule_type=RuleType.DAILY, start_date=today.isoformat()
),
[child.id],
["morning"],
)
count = await materialize(
database,
start.isoformat(),
end.isoformat(),
today=today,
)
# 4 days (today..today+3 inclusive), one instance per day.
assert count == 4
records = await QuestInstancesDao(database).list_by_date_range(
child.id, start.isoformat(), end.isoformat()
)
dates = sorted({r.due_date for r in records})
assert dates == [
(today + datetime.timedelta(days=i)).isoformat()
for i in range(4)
]
# Beyond the configured horizon: no instances.
beyond = (end + datetime.timedelta(days=1)).isoformat()
far = (end + datetime.timedelta(days=10)).isoformat()
assert await QuestInstancesDao(database).list_by_date_range(
child.id, beyond, far
) == []
finally: finally:
await database.close() remove()
finally:
asyncio.new_event_loop().run_until_complete(_main()) await database.close()
+17 -12
View File
@@ -526,24 +526,29 @@ async def test_setup_backfills_configured_horizon_days(hass, make_entry) -> None
).list_by_date_range(child_id, beyond, beyond_end) == [] ).list_by_date_range(child_id, beyond, beyond_end) == []
def test_configured_horizon_days_validates_and_falls_back() -> None: def test_from_options_horizon_validates_and_defaults() -> None:
"""The configured horizon is validated; a malformed value uses the default.""" """from_options returns the configured horizon, defaulting when absent/None.
import custom_components.nestquest as nestquest
assert nestquest._configured_horizon_days({CONF_HORIZON_DAYS: 5}) == 5 Repointed from the removed ``_configured_horizon_days`` shim: a valid
assert nestquest._configured_horizon_days({}) == DEFAULT_HORIZON_DAYS horizon is read, an absent or None key falls back to the default, and
a malformed value RAISES (the integration's ``from_options_resilient``
handles the per-field fall-back at setup, not this strict path).
"""
from custom_components.nestquest.settings import NestQuestSettings
assert NestQuestSettings.from_options({CONF_HORIZON_DAYS: 5}).horizon_days == 5
assert ( assert (
nestquest._configured_horizon_days({CONF_HORIZON_DAYS: True}) NestQuestSettings.from_options({}).horizon_days == DEFAULT_HORIZON_DAYS
== DEFAULT_HORIZON_DAYS
) )
assert ( assert (
nestquest._configured_horizon_days({CONF_HORIZON_DAYS: "5"}) NestQuestSettings.from_options(
== DEFAULT_HORIZON_DAYS {CONF_HORIZON_DAYS: None}
) ).horizon_days
assert (
nestquest._configured_horizon_days({CONF_HORIZON_DAYS: 0})
== DEFAULT_HORIZON_DAYS == DEFAULT_HORIZON_DAYS
) )
for bad in (True, "5", 0):
with pytest.raises(ValueError):
NestQuestSettings.from_options({CONF_HORIZON_DAYS: bad})
async def test_regenerate_service_registered_and_materializes( async def test_regenerate_service_registered_and_materializes(