diff --git a/custom_components/nestquest/__init__.py b/custom_components/nestquest/__init__.py index 91a962d..52a5eeb 100644 --- a/custom_components/nestquest/__init__.py +++ b/custom_components/nestquest/__init__.py @@ -2,19 +2,30 @@ from __future__ import annotations import asyncio +import datetime import inspect import sqlite3 from collections.abc import Callable from dataclasses import dataclass from pathlib import Path from typing import Any +from zoneinfo import ZoneInfo from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant from homeassistant.exceptions import ConfigEntryNotReady +from homeassistant.helpers.event import async_track_time_change -from .const import CONF_ADMIN_USER_IDS, DOMAIN, LOGGER +from .const import ( + CONF_ADMIN_USER_IDS, + CONF_DAY_ROLLOVER_TIME, + DEFAULT_DAY_ROLLOVER_TIME, + DEFAULT_HORIZON_DAYS, + DOMAIN, + LOGGER, +) from .db import NestQuestDatabase +from .materialize import materialize as _materialize_run from .migrations import apply_migrations from .store import async_get_db_path from .admin_allowlist import seed_setup_admin @@ -224,6 +235,7 @@ class NestQuestRuntimeData: options: dict[str, Any] database: NestQuestDatabase remove_update_listener: Callable[[], Any] + remove_time_change_listener: Callable[[], Any] async def _async_owner_user_ids(hass: HomeAssistant) -> list[str]: @@ -248,6 +260,48 @@ async def _async_owner_user_ids(hass: HomeAssistant) -> list[str]: 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``. + + 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`. + """ + rollover = options.get(CONF_DAY_ROLLOVER_TIME, DEFAULT_DAY_ROLLOVER_TIME) + hour, _, minute = rollover.partition(":") + return int(hour), int(minute) + + +def _register_day_rollover_listener( + hass: HomeAssistant, + database: NestQuestDatabase, + options: dict[str, Any], +) -> Callable[[], Any]: + """Register the daily materialization listener and return its remover. + + Fires once per day at the configured local ``day_rollover_time`` and + materializes the rolling horizon ``[today, today + DEFAULT_HORIZON_DAYS]``. + "today" is computed in HA local time (``hass.config.time_zone``) rather + than the system clock, so the horizon tracks the household's own day. + The walk itself runs through :func:`~.materialize.materialize`, whose DB + access is executor-only. + """ + hour, minute = _day_rollover_hour_minute(options) + + async def _run_materialization(_now: datetime.datetime) -> None: + time_zone = ZoneInfo(hass.config.time_zone) + today = datetime.datetime.now(time_zone).date() + start_date = today.isoformat() + end_date = ( + today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS) + ).isoformat() + await _materialize_run(database, start_date, end_date) + + return async_track_time_change( + hass, _run_materialization, hour=hour, minute=minute + ) + + async def async_setup(hass: HomeAssistant, config: dict[str, Any]) -> bool: """Set up the NestQuest integration. YAML configuration is not used, returns True.""" return True @@ -330,6 +384,9 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: owner_ids=owner_ids, ) remove_update_listener = entry.add_update_listener(_async_update_listener) + remove_time_change_listener = _register_day_rollover_listener( + hass, database, dict(entry.options) + ) except BaseException: await database.close() raise @@ -338,6 +395,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: options=dict(entry.options), database=database, remove_update_listener=remove_update_listener, + remove_time_change_listener=remove_time_change_listener, ) entry.runtime_data = runtime_data hass.data[DOMAIN][entry.entry_id] = runtime_data @@ -354,14 +412,29 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: runtime_data = getattr(entry, "runtime_data", None) if runtime_data is not None: remove_update_listener = getattr(runtime_data, "remove_update_listener", None) + remove_time_change_listener = getattr( + runtime_data, "remove_time_change_listener", None + ) unload_error: BaseException | None = None try: - if remove_update_listener is not None: - result = remove_update_listener() - if inspect.isawaitable(result): - await result - except BaseException as err: - unload_error = err + for remove_listener in ( + remove_update_listener, + remove_time_change_listener, + ): + if remove_listener is None: + continue + try: + result = remove_listener() + if inspect.isawaitable(result): + await result + except BaseException as err: + # Cancel EVERY listener even when an earlier remover + # raises: a skipped remover would leave a daily + # callback firing against a closed database after + # unload. The first error is retained and re-raised + # only after both removers (and the DB close) ran. + if unload_error is None: + unload_error = err finally: database = getattr(runtime_data, "database", None) if database is not None: diff --git a/tests/conftest.py b/tests/conftest.py index 5694471..ff8c5d6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -44,6 +44,7 @@ _HA_MODULES = ( "homeassistant.exceptions", "homeassistant.helpers", "homeassistant.helpers.config_validation", + "homeassistant.helpers.event", ) def _find_spec(name: str): @@ -83,6 +84,7 @@ for _parent, _child in ( ("homeassistant", "exceptions"), ("homeassistant", "helpers"), ("homeassistant.helpers", "config_validation"), + ("homeassistant.helpers", "event"), ): setattr(sys.modules[_parent], _child, sys.modules[f"{_parent}.{_child}"]) @@ -242,6 +244,69 @@ class ListenerRegistry: return sum(len(listeners) for listeners in self._listeners.values()) +class TimeChangeRegistry: + """Models HA's time-change listener bookkeeping. + + ``track`` mirrors ``homeassistant.helpers.event.async_track_time_change``: + each call appends a record (action + hour/minute/second) and returns + a remove callable that reverses the registration. Tests inspect + ``registrations`` and exercise the newest action through :meth:`fire`. + """ + + def __init__(self, hass=None): + self._hass = hass + self._registrations: list[dict] = [] + + def track(self, action, hour=None, minute=None, second=None): + record = { + "action": action, + "hour": hour, + "minute": minute, + "second": second, + } + + def _remove(): + self._registrations.remove(record) + + record["remove"] = _remove + self._registrations.append(record) + return _remove + + @property + def registrations(self): + """Return a copy of the live registrations.""" + return list(self._registrations) + + @property + def size(self) -> int: + """Number of live registrations.""" + return len(self._registrations) + + async def fire(self, now=None, index: int = -1): + """Run a registered action as HA's scheduler would, awaiting coroutines.""" + result = self._registrations[index]["action"](now) + if inspect.isawaitable(result): + await result + + +def _async_track_time_change(hass, action, hour=None, minute=None, second=None): + """Mirror homeassistant.helpers.event.async_track_time_change. + + Exact HA 2024.6 signature — no ``local`` keyword: the local-time + wrapper is already this function. Routes to the registry attached to + ``hass`` (created on demand), so tests reach the registrations via + ``hass.time_change``. + """ + registry = getattr(hass, "time_change", None) + if registry is None: + registry = TimeChangeRegistry(hass) + hass.time_change = registry + return registry.track(action, hour=hour, minute=minute, second=second) + + +_ha_mock("homeassistant.helpers.event").async_track_time_change = _async_track_time_change + + def make_config_entry( entry_id: str = "test_entry", options: dict | None = None, @@ -303,6 +368,10 @@ def make_hass() -> tuple: hass.async_add_executor_job = _run_executor_job registry = ListenerRegistry(hass) + hass.time_change = TimeChangeRegistry(hass) + # A valid IANA time zone (the day-rollover listener reads it to + # compute "today" in HA local time). + hass.config.time_zone = "UTC" async def _async_reload(entry_id: str) -> None: registry.reloaded.append(entry_id) diff --git a/tests/test_lifecycle.py b/tests/test_lifecycle.py index c3e0f17..f56864f 100644 --- a/tests/test_lifecycle.py +++ b/tests/test_lifecycle.py @@ -8,7 +8,11 @@ import pytest from conftest import make_config_entry, make_hass, wire_entry_to_registry from custom_components.nestquest import DOMAIN, async_setup_entry, async_unload_entry -from custom_components.nestquest.const import DOMAIN as DOMAIN_CONST +from custom_components.nestquest.const import ( + CONF_DAY_ROLLOVER_TIME, + DEFAULT_HORIZON_DAYS, + DOMAIN as DOMAIN_CONST, +) def _wire(entry, registry): @@ -237,6 +241,128 @@ async def test_unload_closes_database_even_when_listener_removal_raises() -> Non assert database.connected is False, "database leaked despite remover error" assert entry.runtime_data is None assert DOMAIN not in hass.data + assert hass.time_change.size == 0, "time-change listener leaked" + + +async def test_unload_cancels_time_listener_even_when_update_remover_raises() -> None: + """A raising update-listener remover must not skip the time-change cancel. + + Both removers are independent: the update-listener remover raising must + still cancel the daily time-change callback, whose action would otherwise + fire after unload against a closed database. + """ + + class _RemoveBoom(Exception): + pass + + def _remover(): + raise _RemoveBoom() + + entry = make_config_entry(entry_id="update-boom-time-cancel") + entry.add_update_listener = lambda listener: _remover + hass, _registry = make_hass() + await async_setup_entry(hass, entry) + assert hass.time_change.size == 1 + with pytest.raises(_RemoveBoom): + await async_unload_entry(hass, entry) + assert hass.time_change.size == 0, "time-change listener not cancelled" + assert entry.runtime_data is None + assert DOMAIN not in hass.data + + +async def test_setup_registers_day_rollover_listener(hass, make_entry) -> None: + """Setup registers a daily rollover listener and stores its cancel fn.""" + entry = _wire(make_entry(), hass.registry) + assert await async_setup_entry(hass, entry) is True + assert hass.time_change.size == 1 + registration = hass.time_change.registrations[0] + assert registration["hour"] == 0 + assert registration["minute"] == 0 + assert callable(entry.runtime_data.remove_time_change_listener) + + +async def test_setup_registers_day_rollover_at_configured_time(hass, make_entry) -> None: + """The listener fires at the entry option's rollover time, not the default.""" + entry = _wire(make_entry(options={CONF_DAY_ROLLOVER_TIME: "06:30"}), hass.registry) + await async_setup_entry(hass, entry) + registration = hass.time_change.registrations[0] + assert registration["hour"] == 6 + assert registration["minute"] == 30 + + +async def test_unload_cancels_day_rollover_listener(hass, make_entry) -> None: + """Unload removes the time-change listener alongside the update listener.""" + entry = _wire(make_entry(), hass.registry) + await async_setup_entry(hass, entry) + assert hass.time_change.size == 1 + assert await async_unload_entry(hass, entry) is True + assert hass.time_change.size == 0 + + +async def test_rollover_time_change_reregisters_listener_at_new_time( + hass, make_entry +) -> None: + """A rollover-time options change re-registers the listener at the new time.""" + entry = _wire( + make_entry(options={CONF_DAY_ROLLOVER_TIME: "00:00"}), hass.registry + ) + await async_setup_entry(hass, entry) + assert (hass.time_change.registrations[0]["hour"], hass.time_change.registrations[0]["minute"]) == (0, 0) + + # The options flow writes the new rollover time, then HA reloads the + # entry: unload tears down the old listener, setup re-registers it. + entry.options = {CONF_DAY_ROLLOVER_TIME: "06:30"} + await async_unload_entry(hass, entry) + assert hass.time_change.size == 0 + + assert await async_setup_entry(hass, entry) is True + assert hass.time_change.size == 1 + registration = hass.time_change.registrations[0] + assert (registration["hour"], registration["minute"]) == (6, 30) + + +async def test_day_rollover_listener_materializes_ha_local_horizon( + hass, make_entry, monkeypatch +) -> None: + """Firing the listener materializes today..today+horizon in HA local time.""" + import datetime + from unittest.mock import AsyncMock + from zoneinfo import ZoneInfo + + import custom_components.nestquest as nestquest + + entry = _wire(make_entry(), hass.registry) + await async_setup_entry(hass, entry) + database = entry.runtime_data.database + + fake = AsyncMock() + monkeypatch.setattr(nestquest, "_materialize_run", fake) + + time_zone = ZoneInfo(hass.config.time_zone) + today = datetime.datetime.now(time_zone).date() + expected_start = today.isoformat() + expected_end = (today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS)).isoformat() + + await hass.time_change.fire() + + fake.assert_awaited_once_with(database, expected_start, expected_end) + + +def test_day_rollover_time_change_tracker_rejects_local_kwarg() -> None: + """The fake async_track_time_change mirrors HA 2024.6 (no ``local`` kwarg). + + Setup drives the local-time wrapper directly, so passing ``local=`` must + raise TypeError — otherwise a regression in the production call would be + silently masked by the fake. + """ + import inspect as _inspect + + from homeassistant.helpers.event import async_track_time_change + + params = _inspect.signature(async_track_time_change).parameters + assert list(params) == ["hass", "action", "hour", "minute", "second"] + with pytest.raises(TypeError): + async_track_time_change(None, lambda _now: None, hour=0, minute=0, local=True) async def test_setup_applies_all_v1_tables(hass, make_entry) -> None: