Address PR #142 review: core isolation invariant and shim fixes
- Point recurrence purity scan at custom_components.nestquest.core.recurrence (P2.1); the shim only re-exports, so the real model was no longer scanned. - Make test_const/test_config_flow/test_dao_matrix module scans recursive (rglob) so custom_components/nestquest/core/ is covered (P2.2, P2.3); const.py name exclusion retained. - Add test_core_isolation: AST-scan every core module for homeassistant imports, relative imports climbing out of core (level > 1), absolute imports reaching custom_components.nestquest.<not core>, and bare 'import core'/'from core' (P2.4). - Drop the private re-export of _month_end/_nth_weekday_of_month from the recurrence shim; those are test-only, so move the two test imports to custom_components.nestquest.core.recurrence (P2.1/P3.1). - Fix shim docstrings and core/__init__.py to reference custom_components.nestquest.core.* instead of the unimportable 'core.*' paths (P3.2). - Note the migrations logger namespace move to custom_components.nestquest.core.migrations in RELEASE_NOTES.md (P3.3).
This commit is contained in:
@@ -1,5 +1,19 @@
|
|||||||
# NestQuest Release Notes
|
# NestQuest Release Notes
|
||||||
|
|
||||||
|
## Unreleased
|
||||||
|
|
||||||
|
- **Storage layer extraction (task d0b691d8):** the SQLite storage layer
|
||||||
|
(schema, migrations, db, store, const, recurrence, DAOs) is bundled
|
||||||
|
under `custom_components/nestquest/core/`, with the integration's
|
||||||
|
original module names preserved as re-export shims.
|
||||||
|
- **Logger namespace change (breaking for logging configuration):** the
|
||||||
|
migrations runner now logs under
|
||||||
|
`custom_components.nestquest.core.migrations` instead of
|
||||||
|
`custom_components.nestquest.migrations`. Users who tuned the old
|
||||||
|
logger name (e.g. in `logger.yaml` or `configuration.yaml`) must
|
||||||
|
update the entry to keep that level, as the old name no longer
|
||||||
|
receives migration log output.
|
||||||
|
|
||||||
## Version 0.5.0 — 2026-09-18
|
## Version 0.5.0 — 2026-09-18
|
||||||
|
|
||||||
### Scope
|
### Scope
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
"""Re-export of :mod:`core.const` for the integration package.
|
"""Re-export of :mod:`custom_components.nestquest.core.const` for the integration package.
|
||||||
|
|
||||||
The canonical constants live in :mod:`core.const` (the storage layer
|
The canonical constants live in :mod:`custom_components.nestquest.core.const` (the storage layer
|
||||||
extracted in task d0b691d8); this module re-exports them so existing
|
extracted in task d0b691d8); this module re-exports them so existing
|
||||||
``from .const import ...`` references inside the integration keep
|
``from .const import ...`` references inside the integration keep
|
||||||
working without touching every import line.
|
working without touching every import line.
|
||||||
|
|||||||
@@ -5,7 +5,7 @@ SQLite storage layer: schema DDL, the versioned migration runner, the
|
|||||||
async connection wrapper, the path resolver, the domain constants and
|
async connection wrapper, the path resolver, the domain constants and
|
||||||
recurrence model, and the typed DAOs. Nothing in this package imports
|
recurrence model, and the typed DAOs. Nothing in this package imports
|
||||||
``homeassistant``; the integration wires it into HA at its own layer
|
``homeassistant``; the integration wires it into HA at its own layer
|
||||||
(constructing :class:`core.db.NestQuestDatabase` with an executor
|
(constructing :class:`custom_components.nestquest.core.db.NestQuestDatabase`
|
||||||
adapter backed by ``hass.async_add_executor_job``).
|
with an executor adapter backed by ``hass.async_add_executor_job``).
|
||||||
"""
|
"""
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
"""Re-export of :mod:`core.dao_children` for the integration package.
|
"""Re-export of :mod:`custom_components.nestquest.core.dao_children` for the integration package.
|
||||||
|
|
||||||
The children/admin DAO lives in :mod:`core.dao_children` (extracted in
|
The children/admin DAO lives in
|
||||||
|
:mod:`custom_components.nestquest.core.dao_children` (extracted in
|
||||||
task d0b691d8); this module re-exports it so existing
|
task d0b691d8); this module re-exports it so existing
|
||||||
``from .dao_children import ...`` references inside the integration keep
|
``from .dao_children import ...`` references inside the integration keep
|
||||||
working.
|
working.
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
"""Re-export of :mod:`core.dao_instances` for the integration package.
|
"""Re-export of :mod:`custom_components.nestquest.core.dao_instances` for the integration package.
|
||||||
|
|
||||||
The instances/completion-events DAO lives in :mod:`core.dao_instances`
|
The instances/completion-events DAO lives in
|
||||||
|
:mod:`custom_components.nestquest.core.dao_instances`
|
||||||
(extracted in task d0b691d8); this module re-exports it so existing
|
(extracted in task d0b691d8); this module re-exports it so existing
|
||||||
``from .dao_instances import ...`` references inside the integration keep
|
``from .dao_instances import ...`` references inside the integration keep
|
||||||
working.
|
working.
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
"""Re-export of :mod:`core.dao_meta` for the integration package.
|
"""Re-export of :mod:`custom_components.nestquest.core.dao_meta` for the integration package.
|
||||||
|
|
||||||
The meta-state DAO lives in :mod:`core.dao_meta` (extracted in task
|
The meta-state DAO lives in
|
||||||
|
:mod:`custom_components.nestquest.core.dao_meta` (extracted in task
|
||||||
d0b691d8); this module re-exports it so existing
|
d0b691d8); this module re-exports it so existing
|
||||||
``from .dao_meta import ...`` references inside the integration keep
|
``from .dao_meta import ...`` references inside the integration keep
|
||||||
working.
|
working.
|
||||||
|
|||||||
@@ -1,7 +1,8 @@
|
|||||||
"""Re-export of :mod:`core.dao_presence` for the integration package.
|
"""Re-export of :mod:`custom_components.nestquest.core.dao_presence` for the integration package.
|
||||||
|
|
||||||
The presence DAOs live in :mod:`core.dao_presence` (extracted in task
|
The presence DAOs live in
|
||||||
d0b691d8); this module re-exports them so existing
|
:mod:`custom_components.nestquest.core.dao_presence` (extracted in
|
||||||
|
task d0b691d8); this module re-exports them so existing
|
||||||
``from .dao_presence import ...`` references inside the integration keep
|
``from .dao_presence import ...`` references inside the integration keep
|
||||||
working.
|
working.
|
||||||
"""
|
"""
|
||||||
|
|||||||
@@ -1,9 +1,10 @@
|
|||||||
"""Re-export of :mod:`core.dao_rules` for the integration package.
|
"""Re-export of :mod:`custom_components.nestquest.core.dao_rules` for the integration package.
|
||||||
|
|
||||||
The schedule-rules/quest-definitions DAO lives in
|
The schedule-rules/quest-definitions DAO lives in
|
||||||
:mod:`core.dao_rules` (extracted in task d0b691d8); this module
|
:mod:`custom_components.nestquest.core.dao_rules` (extracted in task
|
||||||
re-exports it so existing ``from .dao_rules import ...`` references
|
d0b691d8); this module re-exports it so existing
|
||||||
inside the integration keep working.
|
``from .dao_rules import ...`` references inside the integration keep
|
||||||
|
working.
|
||||||
"""
|
"""
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
"""Re-export of :mod:`core.db` for the integration package.
|
"""Re-export of :mod:`custom_components.nestquest.core.db` for the integration package.
|
||||||
|
|
||||||
The async SQLite wrapper lives in :mod:`core.db` (extracted in task
|
The async SQLite wrapper lives in :mod:`custom_components.nestquest.core.db` (extracted in task
|
||||||
d0b691d8) and is Home-Assistant-free: :class:`NestQuestDatabase` takes
|
d0b691d8) and is Home-Assistant-free: :class:`NestQuestDatabase` takes
|
||||||
an executor callable rather than a ``hass`` object. This module
|
an executor callable rather than a ``hass`` object. This module
|
||||||
re-exports it and provides :func:`make_database`, a thin adapter that
|
re-exports it and provides :func:`make_database`, a thin adapter that
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
"""Re-export of :mod:`core.migrations` for the integration package.
|
"""Re-export of :mod:`custom_components.nestquest.core.migrations` for the integration package.
|
||||||
|
|
||||||
The versioned migration runner lives in :mod:`core.migrations`
|
The versioned migration runner lives in
|
||||||
|
:mod:`custom_components.nestquest.core.migrations`
|
||||||
(extracted in task d0b691d8); this module re-exports it so existing
|
(extracted in task d0b691d8); this module re-exports it so existing
|
||||||
``from .migrations import ...`` references inside the integration keep
|
``from .migrations import ...`` references inside the integration keep
|
||||||
working.
|
working.
|
||||||
|
|||||||
@@ -1,11 +1,10 @@
|
|||||||
"""Re-export of :mod:`core.recurrence` for the integration package.
|
"""Re-export of :mod:`custom_components.nestquest.core.recurrence` for the integration package.
|
||||||
|
|
||||||
The recurrence model lives in :mod:`core.recurrence` (extracted in task
|
The recurrence model lives in :mod:`custom_components.nestquest.core.recurrence`
|
||||||
d0b691d8); this module re-exports it so existing
|
(extracted in task d0b691d8); this module re-exports it so existing
|
||||||
``from .recurrence import ...`` references inside the integration keep
|
``from .recurrence import ...`` references inside the integration keep
|
||||||
working.
|
working.
|
||||||
"""
|
"""
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
from .core.recurrence import * # noqa: F401,F403
|
from .core.recurrence import * # noqa: F401,F403
|
||||||
from .core.recurrence import _month_end, _nth_weekday_of_month # noqa: F401
|
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
"""Re-export of :mod:`core.schema` for the integration package.
|
"""Re-export of :mod:`custom_components.nestquest.core.schema` for the integration package.
|
||||||
|
|
||||||
The declarative schema DDL lives in :mod:`core.schema` (extracted in
|
The declarative schema DDL lives in
|
||||||
|
:mod:`custom_components.nestquest.core.schema` (extracted in
|
||||||
task d0b691d8); this module re-exports it so existing
|
task d0b691d8); this module re-exports it so existing
|
||||||
``from .schema import ...`` references inside the integration keep
|
``from .schema import ...`` references inside the integration keep
|
||||||
working.
|
working.
|
||||||
|
|||||||
@@ -1,7 +1,9 @@
|
|||||||
"""Re-export of :mod:`core.store` for the integration package.
|
"""Re-export of :mod:`custom_components.nestquest.core.store` for the integration package.
|
||||||
|
|
||||||
The path resolver lives in :mod:`core.store` (extracted in task
|
The path resolver lives in
|
||||||
d0b691d8) and is Home-Assistant-free: :func:`core.store.async_get_db_path`
|
:mod:`custom_components.nestquest.core.store` (extracted in task
|
||||||
|
d0b691d8) and is Home-Assistant-free:
|
||||||
|
:func:`custom_components.nestquest.core.store.async_get_db_path`
|
||||||
takes the full database path. This module re-exports it and provides
|
takes the full database path. This module re-exports it and provides
|
||||||
the original ``async_get_db_path(hass)`` signature so the integration
|
the original ``async_get_db_path(hass)`` signature so the integration
|
||||||
resolves the path from ``hass.config.path`` exactly as before.
|
resolves the path from ``hass.config.path`` exactly as before.
|
||||||
@@ -23,7 +25,8 @@ async def async_get_db_path(hass: HomeAssistant) -> Path:
|
|||||||
"""Return the absolute path to the NestQuest SQLite database file.
|
"""Return the absolute path to the NestQuest SQLite database file.
|
||||||
|
|
||||||
Resolves the database file from ``hass.config.path`` and delegates to
|
Resolves the database file from ``hass.config.path`` and delegates to
|
||||||
:func:`core.store.async_get_db_path`, which ensures the parent
|
:func:`custom_components.nestquest.core.store.async_get_db_path`,
|
||||||
directory exists off the event loop via :func:`asyncio.to_thread`.
|
which ensures the parent directory exists off the event loop via
|
||||||
|
:func:`asyncio.to_thread`.
|
||||||
"""
|
"""
|
||||||
return await _core_async_get_db_path(hass.config.path(SQLITE_DB_FILENAME))
|
return await _core_async_get_db_path(hass.config.path(SQLITE_DB_FILENAME))
|
||||||
|
|||||||
@@ -140,7 +140,7 @@ def test_flow_strings_match_translations(
|
|||||||
def test_no_hardcoded_domain_or_db_filename_in_new_modules() -> None:
|
def test_no_hardcoded_domain_or_db_filename_in_new_modules() -> None:
|
||||||
"""Ensure no module other than const.py hardcodes the domain or db filename."""
|
"""Ensure no module other than const.py hardcodes the domain or db filename."""
|
||||||
pkg_dir = Path(config_flow.__file__).parent
|
pkg_dir = Path(config_flow.__file__).parent
|
||||||
py_files = [f for f in pkg_dir.glob("*.py") if f.name not in ("const.py",)]
|
py_files = [f for f in pkg_dir.rglob("*.py") if f.name not in ("const.py",)]
|
||||||
|
|
||||||
assert len(py_files) > 0, "No python modules found in package to check"
|
assert len(py_files) > 0, "No python modules found in package to check"
|
||||||
|
|
||||||
|
|||||||
+1
-1
@@ -106,7 +106,7 @@ def test_window_clock_ranges_cover_every_window() -> None:
|
|||||||
def test_no_hardcoded_domain_or_db_filename_in_modules() -> None:
|
def test_no_hardcoded_domain_or_db_filename_in_modules() -> None:
|
||||||
"""Ensure no module other than const.py hardcodes DOMAIN string or SQLITE_DB_FILENAME."""
|
"""Ensure no module other than const.py hardcodes DOMAIN string or SQLITE_DB_FILENAME."""
|
||||||
pkg_dir = Path(const.__file__).parent
|
pkg_dir = Path(const.__file__).parent
|
||||||
py_files = [f for f in pkg_dir.glob("*.py") if f.name != "const.py"]
|
py_files = [f for f in pkg_dir.rglob("*.py") if f.name != "const.py"]
|
||||||
|
|
||||||
assert len(py_files) > 0, "No python modules found in package to check"
|
assert len(py_files) > 0, "No python modules found in package to check"
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,119 @@
|
|||||||
|
"""Invariant: the bundled core package is self-contained.
|
||||||
|
|
||||||
|
Every module under ``custom_components/nestquest/core/`` must be free of
|
||||||
|
Home Assistant and must not climb out of the core package: no
|
||||||
|
``homeassistant`` import, no relative import with level > 1, no absolute
|
||||||
|
import of ``custom_components.nestquest.<something other than core>``,
|
||||||
|
and no bare ``import core`` / ``from core``. This guards the
|
||||||
|
storage-layer extraction (task d0b691d8) from regressing into a
|
||||||
|
HA-coupled or integration-coupled bundle.
|
||||||
|
"""
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import ast
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
CORE_PKG = Path(
|
||||||
|
__import__(
|
||||||
|
"custom_components.nestquest.core", fromlist=["__file__"]
|
||||||
|
).__file__
|
||||||
|
).parent
|
||||||
|
|
||||||
|
|
||||||
|
def _module_chain(base: str, name: str) -> str:
|
||||||
|
"""Combined dotted path for ``from <base> import <name>``."""
|
||||||
|
return f"{base}.{name}" if base else name
|
||||||
|
|
||||||
|
|
||||||
|
def _is_homeassistant(module: str) -> bool:
|
||||||
|
return module == "homeassistant" or module.startswith("homeassistant.")
|
||||||
|
|
||||||
|
|
||||||
|
def _climbs_out_of_core(module: str) -> bool:
|
||||||
|
"""An absolute path that reaches the integration package but not core."""
|
||||||
|
if module == "custom_components.nestquest":
|
||||||
|
return True
|
||||||
|
if module.startswith("custom_components.nestquest."):
|
||||||
|
rest = module[len("custom_components.nestquest."):]
|
||||||
|
head = rest.split(".", 1)[0]
|
||||||
|
return head != "core"
|
||||||
|
return False
|
||||||
|
|
||||||
|
|
||||||
|
def _is_bare_core(module: str) -> bool:
|
||||||
|
"""A bare ``core`` / ``core.*`` absolute reference (not the bundle)."""
|
||||||
|
return module == "core" or module.startswith("core.")
|
||||||
|
|
||||||
|
|
||||||
|
def _scan(path: Path) -> list[str]:
|
||||||
|
tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
|
||||||
|
offenders: list[str] = []
|
||||||
|
for node in ast.walk(tree):
|
||||||
|
if isinstance(node, ast.Import):
|
||||||
|
for alias in node.names:
|
||||||
|
mod = alias.name
|
||||||
|
if _is_homeassistant(mod):
|
||||||
|
offenders.append(
|
||||||
|
f"{path.name}:{node.lineno} homeassistant import: import {mod}"
|
||||||
|
)
|
||||||
|
if _is_bare_core(mod):
|
||||||
|
offenders.append(
|
||||||
|
f"{path.name}:{node.lineno} bare core import: import {mod}"
|
||||||
|
)
|
||||||
|
if _climbs_out_of_core(mod):
|
||||||
|
offenders.append(
|
||||||
|
f"{path.name}:{node.lineno} climbs out of core: import {mod}"
|
||||||
|
)
|
||||||
|
elif isinstance(node, ast.ImportFrom):
|
||||||
|
if node.level and node.level > 1:
|
||||||
|
offenders.append(
|
||||||
|
f"{path.name}:{node.lineno} relative import climbs out "
|
||||||
|
f"of core (level {node.level})"
|
||||||
|
)
|
||||||
|
continue
|
||||||
|
if node.level:
|
||||||
|
# relative level 1 stays inside core; nothing to check.
|
||||||
|
continue
|
||||||
|
base = node.module or ""
|
||||||
|
if _is_homeassistant(base):
|
||||||
|
offenders.append(
|
||||||
|
f"{path.name}:{node.lineno} homeassistant import: from {base}"
|
||||||
|
)
|
||||||
|
if _is_bare_core(base):
|
||||||
|
offenders.append(
|
||||||
|
f"{path.name}:{node.lineno} bare core import: from {base}"
|
||||||
|
)
|
||||||
|
if _climbs_out_of_core(base):
|
||||||
|
offenders.append(
|
||||||
|
f"{path.name}:{node.lineno} climbs out of core: from {base}"
|
||||||
|
)
|
||||||
|
for alias in node.names:
|
||||||
|
combined = _module_chain(base, alias.name)
|
||||||
|
if _is_homeassistant(combined):
|
||||||
|
offenders.append(
|
||||||
|
f"{path.name}:{node.lineno} homeassistant import: "
|
||||||
|
f"from {base} import {alias.name}"
|
||||||
|
)
|
||||||
|
if _is_bare_core(combined):
|
||||||
|
offenders.append(
|
||||||
|
f"{path.name}:{node.lineno} bare core import: "
|
||||||
|
f"from {base} import {alias.name}"
|
||||||
|
)
|
||||||
|
if _climbs_out_of_core(combined):
|
||||||
|
offenders.append(
|
||||||
|
f"{path.name}:{node.lineno} climbs out of core: "
|
||||||
|
f"from {base} import {alias.name}"
|
||||||
|
)
|
||||||
|
return offenders
|
||||||
|
|
||||||
|
|
||||||
|
def test_core_package_is_self_contained() -> None:
|
||||||
|
"""No core module imports homeassistant or climbs out of the core package."""
|
||||||
|
py_files = sorted(CORE_PKG.rglob("*.py"))
|
||||||
|
assert py_files, "core package not found under custom_components/nestquest/core"
|
||||||
|
all_offenders: list[str] = []
|
||||||
|
for py_file in py_files:
|
||||||
|
all_offenders.extend(_scan(py_file))
|
||||||
|
assert all_offenders == [], (
|
||||||
|
"core package isolation violated:\n" + "\n".join(all_offenders)
|
||||||
|
)
|
||||||
@@ -441,7 +441,7 @@ def test_completion_events_absolutely_no_mutation_path() -> None:
|
|||||||
)
|
)
|
||||||
offenders = [
|
offenders = [
|
||||||
py.name
|
py.name
|
||||||
for py in sorted(package.glob("*.py"))
|
for py in sorted(package.rglob("*.py"))
|
||||||
if py.name != "__pycache__" and mutation.search(py.read_text())
|
if py.name != "__pycache__" and mutation.search(py.read_text())
|
||||||
]
|
]
|
||||||
assert offenders == []
|
assert offenders == []
|
||||||
@@ -276,7 +276,7 @@ def test_recurrence_module_imports_only_stdlib() -> None:
|
|||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
source = Path(
|
source = Path(
|
||||||
__import__("custom_components.nestquest.recurrence",
|
__import__("custom_components.nestquest.core.recurrence",
|
||||||
fromlist=["__file__"]).__file__
|
fromlist=["__file__"]).__file__
|
||||||
).read_text()
|
).read_text()
|
||||||
offenders = _scan_forbidden_imports(source)
|
offenders = _scan_forbidden_imports(source)
|
||||||
|
|||||||
@@ -9,10 +9,10 @@ from custom_components.nestquest.recurrence import (
|
|||||||
RuleType,
|
RuleType,
|
||||||
RuleValidationError,
|
RuleValidationError,
|
||||||
ScheduleRule,
|
ScheduleRule,
|
||||||
_month_end,
|
|
||||||
occurs_on,
|
occurs_on,
|
||||||
occurrences_between,
|
occurrences_between,
|
||||||
)
|
)
|
||||||
|
from custom_components.nestquest.core.recurrence import _month_end
|
||||||
|
|
||||||
|
|
||||||
def _d(date: str) -> datetime.date:
|
def _d(date: str) -> datetime.date:
|
||||||
|
|||||||
@@ -9,10 +9,10 @@ from custom_components.nestquest.recurrence import (
|
|||||||
RuleType,
|
RuleType,
|
||||||
RuleValidationError,
|
RuleValidationError,
|
||||||
ScheduleRule,
|
ScheduleRule,
|
||||||
_nth_weekday_of_month,
|
|
||||||
occurs_on,
|
occurs_on,
|
||||||
occurrences_between,
|
occurrences_between,
|
||||||
)
|
)
|
||||||
|
from custom_components.nestquest.core.recurrence import _nth_weekday_of_month
|
||||||
|
|
||||||
|
|
||||||
def _d(date: str) -> datetime.date:
|
def _d(date: str) -> datetime.date:
|
||||||
|
|||||||
Reference in New Issue
Block a user