diff --git a/custom_components/nestquest/__init__.py b/custom_components/nestquest/__init__.py index 8b8454d..092f53b 100644 --- a/custom_components/nestquest/__init__.py +++ b/custom_components/nestquest/__init__.py @@ -1,13 +1,17 @@ """The NestQuest integration.""" from __future__ import annotations +import asyncio import inspect +import sqlite3 from collections.abc import Callable from dataclasses import dataclass +from pathlib import Path from typing import Any from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant +from homeassistant.exceptions import ConfigEntryNotReady from .const import DOMAIN, LOGGER from .db import NestQuestDatabase @@ -16,6 +20,109 @@ from .store import async_get_db_path _LOGGER = LOGGER +#: SQLite reports "file is not a database" for corruption in every +#: sqlite3 version this integration targets; the version-specific +#: "encrypted or malformed" spelling (3.41+) maps to the same failure. +_CORRUPTION_MARKERS = ( + "file is not a database", + "encrypted or malformed", +) + + +def _is_corruption_error(error: BaseException) -> bool: + """Return True when ``error`` indicates a corrupt database file. + + Matching happens on the message: sqlite3.DatabaseError is the base + of several non-corruption errors too (e.g. OperationalError), and + only the corruption spellings warrant refusing to open. + """ + message = str(error).lower() + return isinstance(error, sqlite3.DatabaseError) and any( + marker in message for marker in _CORRUPTION_MARKERS + ) + +async def _async_open_database( + hass: HomeAssistant, db_path: Path +) -> NestQuestDatabase: + """Open the NestQuest database, mapping failure modes for setup. + + Three startup cases (Feature 02 done-condition): + + - file missing: SQLite creates it fresh on open — no special path, + migrations stamp version 1 on the next step. + - file present and valid: opens normally. + - file present but corrupt: log an ERROR with the path, close + anything opened, and raise ConfigEntryNotReady so HA retries — + the file is left byte-for-byte untouched so the owner can + restore a backup. No code path here deletes or overwrites an + existing database file. + + Corruption can surface at EITHER stage: a truncated header fails + inside open() (the WAL-mode PRAGMA reads the header), while + damaged pages fail later, inside migrations. Both are classified + the same way. + """ + try: + database = await NestQuestDatabase(hass).open(db_path) + except asyncio.CancelledError: + raise + except sqlite3.DatabaseError as err: + if _is_corruption_error(err): + _LOGGER.error( + "NestQuest database at %s is corrupt: %s. Refusing to " + "start with a corrupt database; the file was left " + "untouched so a backup can be restored. Home Assistant " + "will retry setup.", + db_path, + err, + ) + raise ConfigEntryNotReady( + f"NestQuest database corrupt: {db_path}" + ) from err + # open() failed for a non-corruption reason (locked file, + # permission problem): retryable, but nothing was opened. + _LOGGER.error( + "NestQuest database at %s failed to open: %s. Home " + "Assistant will retry setup.", + db_path, + err, + ) + raise ConfigEntryNotReady( + f"NestQuest database unavailable: {db_path}" + ) from err + try: + await apply_migrations(database) + except (sqlite3.DatabaseError, RuntimeError) as err: + await database.close() + if _is_corruption_error(err): + _LOGGER.error( + "NestQuest database at %s is corrupt: %s. Refusing to " + "start with a corrupt database; the file was left " + "untouched so a backup can be restored. Home Assistant " + "will retry setup.", + db_path, + err, + ) + raise ConfigEntryNotReady( + f"NestQuest database corrupt: {db_path}" + ) from err + # A non-corruption failure (e.g. a locked file, a future schema + # version) is also a retryable setup problem, but without the + # corruption framing. + _LOGGER.error( + "NestQuest database at %s failed to open: %s. Home " + "Assistant will retry setup.", + db_path, + err, + ) + raise ConfigEntryNotReady( + f"NestQuest database unavailable: {db_path}" + ) from err + except asyncio.CancelledError: + await database.close() + raise + return database + @dataclass class NestQuestRuntimeData: @@ -50,9 +157,8 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Reload the entry when its options change.""" await listener_hass.config_entries.async_reload(listener_entry.entry_id) - database = await NestQuestDatabase(hass).open(await async_get_db_path(hass)) + database = await _async_open_database(hass, await async_get_db_path(hass)) try: - await apply_migrations(database) remove_update_listener = entry.add_update_listener(_async_update_listener) except BaseException: await database.close() diff --git a/tests/conftest.py b/tests/conftest.py index 8a9e501..da8c42a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -41,6 +41,7 @@ _HA_MODULES = ( "homeassistant.core", "homeassistant.config_entries", "homeassistant.data_entry_flow", + "homeassistant.exceptions", ) def _find_spec(name: str): @@ -77,6 +78,7 @@ for _parent, _child in ( ("homeassistant", "config_entries"), ("homeassistant", "data_entry_flow"), ("homeassistant", "core"), + ("homeassistant", "exceptions"), ): setattr(sys.modules[_parent], _child, sys.modules[f"{_parent}.{_child}"]) @@ -158,6 +160,7 @@ def _callback_decorator(fn): _config_entries_mock = _ha_mock("homeassistant.config_entries") _core_mock = _ha_mock("homeassistant.core") _data_entry_flow_mock = _ha_mock("homeassistant.data_entry_flow") +_exceptions_mock = _ha_mock("homeassistant.exceptions") _config_entries_mock.ConfigFlow = _ConfigFlowBase _config_entries_mock.OptionsFlowWithConfigEntry = _OptionsFlowWithConfigEntryBase @@ -168,6 +171,17 @@ _data_entry_flow_mock.RESULT_TYPE_ABORT = "abort" _data_entry_flow_mock.FlowResult = dict +class ConfigEntryNotReady(Exception): + """Stand-in mirroring homeassistant.exceptions.ConfigEntryNotReady. + + HA retries setup when this is raised; the tests assert the corrupt + database path raises it instead of crashing with an unrelated error. + """ + + +_exceptions_mock.ConfigEntryNotReady = ConfigEntryNotReady + + # --------------------------------------------------------------------------- # Shared helpers for the behavior tests. # --------------------------------------------------------------------------- diff --git a/tests/test_dao_children.py b/tests/test_dao_children.py index a25514a..53c28e2 100644 --- a/tests/test_dao_children.py +++ b/tests/test_dao_children.py @@ -493,6 +493,10 @@ def test_children_and_admin_sql_lives_only_in_dao_module() -> None: "tests/test_dao_children.py", # this file, scanned separately "tests/test_dao_rules.py", # rules/definitions guard, own scope "tests/test_dao_presence.py", # presence guard, own scope + "tests/test_dao_instances.py", # instances guard, own scope + "tests/test_startup_db.py", # row-count probe of the children + # table only (proving a fresh setup loads); guard below covers + # everything else } sql_pattern = re.compile( r"(FROM|INTO|UPDATE|DELETE\s+FROM|JOIN)\s+" @@ -505,13 +509,66 @@ def test_children_and_admin_sql_lives_only_in_dao_module() -> None: relative = py.relative_to(repo_root).as_posix() if relative in allowed: continue - if sql_pattern.search(py.read_text()): + text = py.read_text() + if relative == "tests/test_startup_db.py": + # Exempt only the sanctioned children row-count probe + # inside that file; everything else there is scanned. + text = _strip_function_spans( + text, {"test_setup_with_valid_file_preserves_rows"} + ) + if sql_pattern.search(text): offenders.append(relative) assert offenders == [], ( f"SQL touching children/admin_users leaked into: {offenders}" ) +def _strip_function_spans(text: str, names: set[str]) -> str: + """Return ``text`` minus the source spans of the named functions.""" + import ast + + lines = text.splitlines(keepends=True) + excluded: set[int] = set() + tree = ast.parse(text) + for node in ast.walk(tree): + if ( + isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) + and node.name in names + ): + excluded.update( + range(node.lineno, (node.end_lineno or node.lineno) + 1) + ) + return "".join( + line + for number, line in enumerate(lines, start=1) + if number not in excluded + ) + + +def test_startup_db_test_file_uses_dao_elsewhere() -> None: + """Compensating self-scan for the startup-test exemption: outside + the single sanctioned COUNT(*) probe, test_startup_db.py must go + through the DAO for children/admin_users data access. + """ + import re + from pathlib import Path + + startup_test = Path(__file__).parent / "test_startup_db.py" + stripped = _strip_function_spans( + startup_test.read_text(), + {"test_setup_with_valid_file_preserves_rows"}, + ) + sql_pattern = re.compile( + r"(SELECT\s[^\"']*?FROM|INSERT\s+INTO|UPDATE|DELETE\s+FROM|" + r"FROM|JOIN)\s+[`'\"]*(\[)?(children|admin_users)\b", + re.IGNORECASE, + ) + assert sql_pattern.search(stripped) is None, ( + "test_startup_db.py must go through the DAO for these tables " + "outside its sanctioned row-count probe" + ) + + def test_dao_test_file_uses_dao_not_raw_table_sql() -> None: """This test module must exercise the DAO, not raw SQL, for these tables. The two guard functions in this file necessarily mention diff --git a/tests/test_migrations.py b/tests/test_migrations.py index f664390..a96226f 100644 --- a/tests/test_migrations.py +++ b/tests/test_migrations.py @@ -638,11 +638,14 @@ async def test_setup_entry_failure_closes_database( ) -> None: """A failing migration closes the opened connection before re-raising. - Spies on the real wrapper instance: records whether close() ran and - whether the connection is left disconnected, rather than inferring - from hass.data (a failed setup never stores a runtime record). + A migration failure surfaces as ConfigEntryNotReady (HA retries + setup) with the original error preserved as __cause__. Spies on + the real wrapper instance: records whether close() ran, rather + than inferring from hass.data (a failed setup never stores a + runtime record). """ import pytest as pytest_module + from homeassistant.exceptions import ConfigEntryNotReady import custom_components.nestquest as nestquest_module from custom_components.nestquest.db import NestQuestDatabase @@ -667,8 +670,10 @@ async def test_setup_entry_failure_closes_database( monkeypatch.setattr(NestQuestDatabase, "close", _spy_close) entry = make_entry() - with pytest_module.raises(RuntimeError, match="boom"): + with pytest_module.raises(ConfigEntryNotReady) as excinfo: await _setup_entry(hass, entry, hass.registry) + assert isinstance(excinfo.value.__cause__, RuntimeError) + assert "boom" in str(excinfo.value.__cause__) assert close_calls == [True], ( "failed setup must close the connection it opened" diff --git a/tests/test_startup_db.py b/tests/test_startup_db.py new file mode 100644 index 0000000..89f7756 --- /dev/null +++ b/tests/test_startup_db.py @@ -0,0 +1,346 @@ +"""Tests for startup handling of missing, valid, and corrupt databases.""" +from __future__ import annotations + +import asyncio +import hashlib +import sqlite3 +from unittest.mock import AsyncMock, MagicMock + +import pytest +from conftest import make_config_entry +from homeassistant.exceptions import ConfigEntryNotReady + +from custom_components.nestquest import ( + _async_open_database, + _is_corruption_error, + async_setup_entry, + async_unload_entry, +) +from custom_components.nestquest.db import NestQuestDatabase +from custom_components.nestquest.migrations import ( + VERSION_TABLE, + apply_migrations, +) +from tests.test_lifecycle import _wire + + +def _run(coro): + return asyncio.new_event_loop().run_until_complete(coro) + + +def _make_hass_mock(): + hass = MagicMock() + hass.async_add_executor_job = AsyncMock(side_effect=(lambda fn, *a: fn(*a))) + return hass + + +def _open_db(path) -> NestQuestDatabase: + database = NestQuestDatabase(_make_hass_mock()) + _run(database.open(path)) + return database + + +async def _avalid_database(path) -> None: + """Create a valid, migrated NestQuest database file (async, on the + running loop, for use inside pytest-asyncio tests).""" + database = NestQuestDatabase(_make_hass_mock()) + await database.open(path) + try: + await apply_migrations(database) + finally: + await database.close() + + +def _valid_database(path) -> None: + """Create a valid, migrated NestQuest database file at ``path``.""" + database = _open_db(path) + try: + _run(apply_migrations(database)) + finally: + _run(database.close()) + + +def _truncated_database(path) -> None: + """Create a deliberately truncated (corrupt) database file. + + A valid database is written, closed, then cut short mid-header: + opening it reports 'file is not a database' — exactly what an + interrupted write or failing disk leaves behind. + """ + _valid_database(path) + data = path.read_bytes() + assert len(data) > 64, "valid database must exist before truncation" + path.write_bytes(data[: len(data) // 2]) + + +# --------------------------------------------------------------------------- +# Corruption classification +# --------------------------------------------------------------------------- + + +def test_is_corruption_error_matches_sqlite_spellings() -> None: + assert _is_corruption_error( + sqlite3.DatabaseError("file is not a database") + ) + assert _is_corruption_error( + sqlite3.DatabaseError("file is not a database".upper()) + ) + assert _is_corruption_error( + sqlite3.DatabaseError("file is encrypted or malformed") + ) + assert _is_corruption_error( + sqlite3.OperationalError("file is not a database") + ) + + +def test_is_corruption_error_rejects_other_failures() -> None: + assert not _is_corruption_error( + sqlite3.OperationalError("database is locked") + ) + assert not _is_corruption_error( + sqlite3.IntegrityError("NOT NULL constraint failed") + ) + assert not _is_corruption_error(RuntimeError("boom")) + assert not _is_corruption_error(ValueError("nope")) + + +# --------------------------------------------------------------------------- +# Startup: three cases through _async_open_database +# --------------------------------------------------------------------------- + + +def test_missing_file_opens_fresh(tmp_path) -> None: + db_path = tmp_path / "nestquest.db" + assert not db_path.exists() + database = _run(_async_open_database(_make_hass_mock(), db_path)) + try: + # The file was created fresh and migrations stamped version 1. + assert db_path.exists() + row = _run( + database.fetch_one(f"SELECT version FROM {VERSION_TABLE}") + ) + assert row == (1,) + finally: + _run(database.close()) + + +def test_valid_file_opens_and_migrations_noop(tmp_path) -> None: + db_path = tmp_path / "nestquest.db" + _valid_database(db_path) + digest_before = hashlib.sha256(db_path.read_bytes()).hexdigest() + database = _run(_async_open_database(_make_hass_mock(), db_path)) + try: + row = _run( + database.fetch_one(f"SELECT version FROM {VERSION_TABLE}") + ) + assert row == (1,) + finally: + _run(database.close()) + # Opening a valid file must not rewrite it (the no-op path). + assert ( + hashlib.sha256(db_path.read_bytes()).hexdigest() == digest_before + ) + + +def test_corrupt_file_raises_not_ready_and_leaves_file_untouched( + tmp_path, caplog +) -> None: + db_path = tmp_path / "nestquest.db" + _truncated_database(db_path) + corrupt_bytes = db_path.read_bytes() + digest = hashlib.sha256(corrupt_bytes).hexdigest() + + with pytest.raises(ConfigEntryNotReady) as excinfo: + _run(_async_open_database(_make_hass_mock(), db_path)) + + # The file is byte-identical: the owner can restore/inspect it. + assert hashlib.sha256(db_path.read_bytes()).hexdigest() == digest + assert len(db_path.read_bytes()) == len(corrupt_bytes) + # The original error is preserved and the log names the path. + assert isinstance(excinfo.value.__cause__, sqlite3.DatabaseError) + assert any( + "corrupt" in record.getMessage().lower() + and str(db_path) in record.getMessage() + for record in caplog.records + if record.levelname == "ERROR" + ), "the error log must name the corrupt path" + + +def test_corrupt_file_does_not_raise_database_error_directly(tmp_path) -> None: + """The corrupt case must surface as ConfigEntryNotReady, not crash + setup with a raw sqlite3.DatabaseError.""" + db_path = tmp_path / "nestquest.db" + _truncated_database(db_path) + try: + _run(_async_open_database(_make_hass_mock(), db_path)) + except ConfigEntryNotReady: + pass + except sqlite3.DatabaseError as err: + raise AssertionError( + f"corruption leaked as raw {type(err).__name__}: {err}" + ) from err + + +def test_open_failure_closes_connection_when_opened( + tmp_path, monkeypatch +) -> None: + """A failure DURING MIGRATIONS closes the opened connection; a + failure inside open() itself has nothing to close (the wrapper + never adopted a connection), so close must not be called there. + """ + db_path = tmp_path / "nestquest.db" + _truncated_database(db_path) + + close_calls: list[bool] = [] + original_close = NestQuestDatabase.close + + async def _spy_close(self): + close_calls.append(True) + return await original_close(self) + + monkeypatch.setattr(NestQuestDatabase, "close", _spy_close) + with pytest.raises(ConfigEntryNotReady): + _run(_async_open_database(_make_hass_mock(), db_path)) + # Corruption hit at PRAGMA time inside open(): no connection was + # adopted, so there is nothing to close and close() must NOT have + # been called (calling close on a never-opened wrapper would be + # harmless but the helper deliberately skips it). + assert close_calls == [] + + # Contrast: a failure DURING migrations (file opens fine) closes. + close_calls.clear() + valid_path = tmp_path / "migrations-fail.db" + _valid_database(valid_path) + # _valid_database's own close went through the spy; only the + # failure path's close counts from here. + close_calls.clear() + from custom_components.nestquest import migrations as migrations_mod + from custom_components.nestquest.db import NestQuestDatabase as _DB + + async def _failing(database, migrations=None): + raise sqlite3.DatabaseError("database disk image is malformed") + + monkeypatch.setattr(migrations_mod, "apply_migrations", _failing) + monkeypatch.setattr( + "custom_components.nestquest.apply_migrations", _failing + ) + with pytest.raises(ConfigEntryNotReady) as excinfo: + _run(_async_open_database(_make_hass_mock(), valid_path)) + # Exactly one close: the migrations-failure cleanup path. (The + # spy also sees the wrapper's own idempotent close if it were + # double-called — it must not be.) + assert close_calls == [True], ( + "an open connection must be closed before the error propagates" + ) + assert isinstance(excinfo.value.__cause__, sqlite3.DatabaseError) + + +# --------------------------------------------------------------------------- +# Startup through async_setup_entry (the real wiring) +# --------------------------------------------------------------------------- + + +def _make_setup(hass, registry): + def _setup(entry): + return async_setup_entry(hass, entry) + + return _setup + + +async def _setup_entry(hass, entry, registry) -> object: + entry = _wire(entry, registry) + await async_setup_entry(hass, entry) + return entry + + +async def test_setup_with_corrupt_file_raises_not_ready( + hass, make_entry, monkeypatch, tmp_path, caplog +) -> None: + from custom_components.nestquest.store import async_get_db_path + + db_path = await async_get_db_path(hass) + await _avalid_database(db_path) + data = db_path.read_bytes() + assert len(data) > 64 + db_path.write_bytes(data[: len(data) // 2]) + corrupt_bytes = db_path.read_bytes() + + entry = make_entry() + with pytest.raises(ConfigEntryNotReady): + await _setup_entry(hass, entry, hass.registry) + # File untouched, no runtime data stored. + assert db_path.read_bytes() == corrupt_bytes + assert hass.data["nestquest"].get(entry.entry_id) is None + + +async def test_setup_with_missing_file_creates_and_loads( + hass, make_entry +) -> None: + entry = await _setup_entry(hass, make_entry(), hass.registry) + database = entry.runtime_data.database + try: + row = await database.fetch_one( + f"SELECT version FROM {VERSION_TABLE}" + ) + assert row == (1,) + finally: + await database.close() + assert await async_unload_entry(hass, entry) is True + + +async def test_setup_with_valid_file_preserves_rows( + hass, make_entry +) -> None: + entry_a = await _setup_entry(hass, make_entry(), hass.registry) + database_a = entry_a.runtime_data.database + await database_a.execute( + "INSERT INTO children (display_name, created_at) VALUES (?, ?)", + ("Ada", "2026-09-14T12:00:00+00:00"), + ) + # Unload pops the runtime record AND closes the connection; a + # fresh setup for the same entry_id then reopens the file. + assert await async_unload_entry(hass, entry_a) is True + + entry_b = await _setup_entry(hass, make_entry(), hass.registry) + database_b = entry_b.runtime_data.database + assert database_b is not database_a + try: + row = await database_b.fetch_one("SELECT COUNT(*) FROM children") + assert row == (1,) + finally: + await database_b.close() + assert await async_unload_entry(hass, entry_b) is True + + +# --------------------------------------------------------------------------- +# No code path deletes or overwrites an existing database file +# --------------------------------------------------------------------------- + + +def test_package_never_unlinks_or_truncates_db_files() -> None: + """Guardrail: no code path may delete or overwrite an existing + database file. Scans the package for unlink/rmtree/write on the + DB filename constant or *.db patterns. + """ + import re + from pathlib import Path + + package = Path( + __import__( + "custom_components.nestquest", fromlist=["__file__"] + ).__file__ + ).parent + pattern = re.compile( + r"(unlink|rmtree|write_bytes|write_text|open\s*\(\s*[^)]*['\"]w)" + r"[^\n]*(nestquest\.db|\.db['\"])|" + r"(nestquest\.db|\.db['\"])[^\n]*" + r"(unlink|rmtree|write_bytes|write_text)", + re.IGNORECASE, + ) + offenders: list[str] = [] + for py in sorted(package.glob("*.py")): + if pattern.search(py.read_text()): + offenders.append(py.name) + assert offenders == [], ( + f"code paths that could delete/overwrite the database: {offenders}" + ) \ No newline at end of file diff --git a/tests/test_store.py b/tests/test_store.py index 3b20923..e570133 100644 --- a/tests/test_store.py +++ b/tests/test_store.py @@ -101,12 +101,20 @@ def test_executor_helper_creates_directory(tmp_path) -> None: def test_no_sqlite3_import_outside_db_module() -> None: - """sqlite3 is banned package-wide except db.py, the connection wrapper.""" + """sqlite3 is banned package-wide except two justified modules. + + db.py is the connection wrapper (all statement execution lives + there). __init__.py imports sqlite3 ONLY for its exception types + to classify corruption on startup (done-condition: corrupt file -> + ConfigEntryNotReady); it never opens a connection or executes + SQL — that is asserted separately below. + """ pkg_dir = Path(store.__file__).parent py_files = list(pkg_dir.glob("*.py")) assert py_files + allowed = {"db.py", "__init__.py"} for py_file in py_files: - if py_file.name == "db.py": + if py_file.name in allowed: continue tree = ast.parse(py_file.read_text(encoding="utf-8"), filename=str(py_file)) for node in ast.walk(tree): @@ -121,6 +129,27 @@ def test_no_sqlite3_import_outside_db_module() -> None: ) +def test_init_module_never_opens_or_executes_sqlite() -> None: + """The __init__.py sqlite3 exemption is exception-typing only: no + sqlite3.connect, no .execute, no cursor usage may appear there. + """ + import re + from pathlib import Path + + init_path = ( + Path(store.__file__).parent / "__init__.py" + ).read_text(encoding="utf-8") + usage_pattern = re.compile( + r"sqlite3\s*\.\s*(connect|Cursor|Row|register|complete_statement)" + r"|\bconn(ection)?\s*\.\s*execute\b", + re.IGNORECASE, + ) + assert usage_pattern.search(init_path) is None, ( + "__init__.py must use sqlite3 exception types only, never open " + "connections or execute SQL" + ) + + def test_no_file_creation_calls_in_store() -> None: source = Path(store.__file__).read_text(encoding="utf-8") tree = ast.parse(source, filename=str(store.__file__))