diff --git a/custom_components/nestquest/dashboard.py b/custom_components/nestquest/dashboard.py index 227f4b2..17bd6cb 100644 --- a/custom_components/nestquest/dashboard.py +++ b/custom_components/nestquest/dashboard.py @@ -1,17 +1,37 @@ """Zero-config panel dashboard registration for NestQuest (Feature 20). On setup the integration registers ONE storage-mode Lovelace dashboard -whose config uses the bundle's ``custom:nestquest-party`` strategy (see +whose CONFIG uses the bundle's ``custom:nestquest-party`` strategy (see ``frontend/src/strategy.ts``): the views themselves are GENERATED at -render time from the household roster, so the stored dashboard carries -nothing but the strategy reference. Registration is IDEMPOTENT — a -dashboard with the stable ``url_path`` (ours or a user's) is never -touched, and the Lovelace dashboards collection being unavailable +render time from the household roster, so the stored dashboard config +carries nothing but the strategy reference. + +Registration mirrors the REAL two-step sequence the lovelace +integration itself uses when it creates the onboarding map dashboard +(``homeassistant/components/lovelace/__init__.py``, +``_create_map_dashboard``): + +1. create the dashboard METADATA through the ``DashboardsCollection`` + (``hass.data["lovelace"]["dashboards_collection"]``) — url_path, + title, icon, sidebar visibility. The strategy reference is NOT part + of the create payload: the collection's create schema allows only + the metadata fields, so a ``strategy`` key there fails the whole + create (this was the silent no-op the first implementation shipped). +2. save the dashboard CONFIG — ``{"strategy": ...}`` — through the + per-url_path CONTENT manager that the collection's change listener + registers under ``hass.data["lovelace"]["dashboards"][url_path]`` + (a ``LovelaceStorage``), exactly the second step HA performs. + +Registration is IDEMPOTENT — a dashboard already under the stable +``url_path`` (ours or a user's) skips BOTH steps and its stored config +is never overwritten — and the Lovelace surfaces being unavailable (YAML mode, lovelace not loaded) is a logged warning, never a setup failure. """ from __future__ import annotations +import asyncio + from homeassistant.core import HomeAssistant from .const import ( @@ -24,19 +44,59 @@ from .const import ( _LOGGER = LOGGER +# Single-flight guard for the check-then-create below: HA can set up +# several config entries CONCURRENTLY, and two overlapping setups would +# both pass the exists-check and race on the url_path create. The lock +# is per hass instance (kept as an attribute on the hass object, since +# hass.data is the integrations' shared registry the unload path +# empties) rather than one module-level asyncio.Lock — a bare +# module-level lock binds to whichever event loop first awaits it and +# raises RuntimeError on any other loop, while the race it must prevent +# only exists between setups running on the SAME hass anyway. +_REGISTRATION_LOCK_ATTR = "_nestquest_dashboard_registration_lock" -def _dashboards_collection(hass: HomeAssistant): - """Return the Lovelace dashboards collection, or None when absent. - The lovelace integration stashes its collections on - ``hass.data["lovelace"]``; a missing key (lovelace not loaded) and a - collection-less LovelaceData both mean "cannot create storage - dashboards here" — callers log and continue. The presence of the - surface is verified by capability (``async_create_item`` callable), - not by type, so no lovelace import is needed. +def _registration_lock(hass: HomeAssistant) -> asyncio.Lock: + """Return the per-hass registration lock, creating it on first use.""" + lock = getattr(hass, _REGISTRATION_LOCK_ATTR, None) + if lock is None: + lock = asyncio.Lock() + setattr(hass, _REGISTRATION_LOCK_ATTR, lock) + return lock + + +def _lovelace_data(hass: HomeAssistant) -> dict | None: + """Return ``hass.data["lovelace"]`` when it is the real dict shape. + + The lovelace integration stores a plain DICT of surfaces (``mode``, + ``dashboards``, ``dashboards_collection``, ``resources``, + ``yaml_dashboards``) under the ``lovelace`` key; anything else + (missing key, unexpected object) means the surfaces below cannot be + reached and registration must warn and continue. """ lovelace_data = hass.data.get("lovelace") - collection = getattr(lovelace_data, "dashboards", None) + if not isinstance(lovelace_data, dict): + return None + return lovelace_data + + +def _dashboards_collection(hass: HomeAssistant): + """Return the Lovelace dashboards METADATA collection, or None. + + The collection lives at ``hass.data["lovelace"] + ["dashboards_collection"]`` — NOT under ``["dashboards"]``, which + maps url_path to the per-dashboard CONTENT managers + (``LovelaceStorage``/``LovelaceYAML``). A missing key (lovelace + not loaded) and a missing/None entry (YAML mode never sets the + collection) both mean "cannot create storage dashboards here" — + callers log and continue. The presence of the surface is verified + by capability (``async_create_item`` callable), not by type, so no + lovelace import is needed. + """ + lovelace_data = _lovelace_data(hass) + if lovelace_data is None: + return None + collection = lovelace_data.get("dashboards_collection") if collection is None or not callable( getattr(collection, "async_create_item", None) ): @@ -44,6 +104,27 @@ def _dashboards_collection(hass: HomeAssistant): return collection +def _dashboard_store(hass: HomeAssistant, url_path: str): + """Return a url_path's dashboard CONTENT manager, or None. + + After a metadata create, the collection's change listener registers + a ``LovelaceStorage`` for the new dashboard under + ``hass.data["lovelace"]["dashboards"][url_path]``; the dashboard + CONFIG (the strategy reference) is saved through THAT manager — + the second step of HA's own dashboard creation. + """ + lovelace_data = _lovelace_data(hass) + if lovelace_data is None: + return None + dashboards = lovelace_data.get("dashboards") + if not isinstance(dashboards, dict): + return None + store = dashboards.get(url_path) + if store is None or not callable(getattr(store, "async_save", None)): + return None + return store + + def _item_url_path(item) -> str | None: """Read a dashboard item's ``url_path`` through either access shape. @@ -72,10 +153,13 @@ async def async_register_dashboard(hass: HomeAssistant) -> bool: Returns True when a dashboard under the stable ``url_path`` exists after the call (pre-existing or created here), False when the - Lovelace dashboards collection was unavailable or refused. A - dashboard already registered under the stable ``url_path`` — whether - ours from a previous setup or a user's own — is left COMPLETELY - alone: never overwritten, never deleted, never a duplicate. + Lovelace surfaces were unavailable or refused. A dashboard already + registered under the stable ``url_path`` — whether ours from a + previous setup or a user's own — skips BOTH steps and is left + COMPLETELY alone: its stored config is never overwritten, it is + never deleted, never a duplicate. The strategy config is saved + ONLY for the dashboard created in THIS call, whose fresh content + store is empty, so the save cannot clobber anything. """ collection = _dashboards_collection(hass) if collection is None: @@ -88,19 +172,37 @@ async def async_register_dashboard(hass: HomeAssistant) -> bool: ) return False try: - items = await collection.async_items() - if _existing_dashboard(items): - return True - await collection.async_create_item( - { - "url_path": DASHBOARD_URL_PATH, - "title": DASHBOARD_TITLE, - "icon": DASHBOARD_ICON, - "show_in_sidebar": True, - "mode": "storage", - "strategy": {"type": DASHBOARD_STRATEGY_TYPE}, - } - ) + # Single-flight: two concurrent entry setups must not both pass + # the exists-check and race on the url_path create. + async with _registration_lock(hass): + # async_items() is a SYNC @callback on the real collection + # (helpers/collection.py ObservableCollection) — not awaited. + if _existing_dashboard(collection.async_items()): + return True + await collection.async_create_item( + { + "url_path": DASHBOARD_URL_PATH, + "title": DASHBOARD_TITLE, + "icon": DASHBOARD_ICON, + "show_in_sidebar": True, + "allow_single_word": True, + } + ) + # Step two (mirroring HA's _create_map_dashboard): the + # strategy CONFIG goes through the fresh dashboard's + # content manager, not through the metadata collection. + store = _dashboard_store(hass, DASHBOARD_URL_PATH) + if store is None: + _LOGGER.warning( + "The NestQuest panel dashboard was registered but its " + "Lovelace content manager never appeared; add the " + "strategy %s to the dashboard manually.", + DASHBOARD_STRATEGY_TYPE, + ) + return False + await store.async_save( + {"strategy": {"type": DASHBOARD_STRATEGY_TYPE}} + ) except Exception: _LOGGER.warning( "Registering the NestQuest panel dashboard in the Lovelace " diff --git a/custom_components/nestquest/manifest.json b/custom_components/nestquest/manifest.json index 157d18a..c4fe61d 100644 --- a/custom_components/nestquest/manifest.json +++ b/custom_components/nestquest/manifest.json @@ -7,7 +7,8 @@ "config_flow": true, "dependencies": [ "frontend", - "http" + "http", + "lovelace" ], "documentation": "https://github.com/Maniacs-Mansion/NestQuest", "integration_type": "hub", diff --git a/tests/test_lifecycle.py b/tests/test_lifecycle.py index 6ced532..e5052d0 100644 --- a/tests/test_lifecycle.py +++ b/tests/test_lifecycle.py @@ -2,7 +2,6 @@ from __future__ import annotations import asyncio -from types import SimpleNamespace import pytest @@ -16,6 +15,7 @@ from custom_components.nestquest.const import ( from custom_components.nestquest.dashboard import ( DASHBOARD_STRATEGY_TYPE, DASHBOARD_URL_PATH, + async_register_dashboard, ) @@ -23,29 +23,86 @@ def _wire(entry, registry): return wire_entry_to_registry(entry, registry) -class FakeDashboardsCollection: - """Stand-in for the Lovelace dashboards collection. +class FakeDashboardStore: + """Stand-in for a per-dashboard CONTENT manager (LovelaceStorage). - Records ``async_create_item`` calls the way the real collection - would persist them, so tests can assert exactly what the setup path - registered — and that a reload registers nothing more. + In the real API the dashboard CONFIG (the strategy reference) is + saved through THIS manager's ``async_save``, NOT through the + metadata collection — so every strategy-save assertion reads here. """ - def __init__(self, items=None): + def __init__(self) -> None: + self.saved: list[dict] = [] + self.config: dict | None = None + + async def async_save(self, config: dict) -> None: + self.saved.append(dict(config)) + self.config = dict(config) + + +class FakeDashboardsCollection: + """Stand-in mirroring the REAL Lovelace dashboards API shapes. + + Faithful to ``homeassistant/components/lovelace`` + + ``helpers/collection.py``: ``async_items()`` is a SYNC ``@callback`` + (ObservableCollection.async_items returns list(data.values())), + ``async_create_item`` IS async, and — mirroring lovelace's + ``storage_dashboard_changed`` listener — a successful create + registers a fresh CONTENT manager under + ``hass.data["lovelace"]["dashboards"][url_path]``, which is where + the dashboard config is then saved. Records ``async_create_item`` + calls so tests can assert exactly what the setup path registered — + and that a reload registers nothing more. + """ + + def __init__(self, hass, items=None) -> None: + self.hass = hass self.items = list(items or []) self.created: list[dict] = [] + #: url_path -> content manager, mirroring the real listener's + # registrations (pre-existing items included). + self.stores: dict[str, FakeDashboardStore] = {} - async def async_items(self): + def async_items(self): + """SYNC, like the real ``ObservableCollection.async_items``.""" return list(self.items) async def async_create_item(self, item_config: dict) -> None: + # The real collection awaits validation and change notification + # around the item insertion; yield first so two concurrent + # callers without a single-flight guard would BOTH get here. + await asyncio.sleep(0) self.created.append(dict(item_config)) self.items.append(dict(item_config)) + self.stores[item_config["url_path"]] = FakeDashboardStore() + self.hass.data["lovelace"]["dashboards"][item_config["url_path"]] = ( + self.stores[item_config["url_path"]] + ) -def _install_dashboard_collection(hass, items=None) -> FakeDashboardsCollection: - collection = FakeDashboardsCollection(items) - hass.data["lovelace"] = SimpleNamespace(dashboards=collection) +def _install_dashboard_collection( + hass, items=None +) -> FakeDashboardsCollection: + """Install the REAL-API-shaped lovelace surfaces on the hass stub. + + Mirrors what lovelace's ``async_setup`` leaves behind in storage + mode: ``hass.data["lovelace"]`` is a DICT with the metadata + collection under ``dashboards_collection`` and the per-url_path + CONTENT managers under ``dashboards``. Any pre-existing item gets + a content manager, exactly as lovelace's change listener would + have registered one when the dashboard was created. + """ + collection = FakeDashboardsCollection(hass, items) + hass.data["lovelace"] = { + "mode": "storage", + "dashboards": {}, + "dashboards_collection": collection, + } + for item in collection.items: + url_path = item.get("url_path") + store = FakeDashboardStore() + collection.stores[url_path] = store + hass.data["lovelace"]["dashboards"][url_path] = store return collection @@ -305,52 +362,76 @@ async def test_unload_shuts_coordinator_even_when_listener_removal_raises() -> N async def test_setup_creates_the_panel_dashboard(hass, make_entry) -> None: - """A fresh install gets the storage-mode strategy dashboard, with the - stable url_path, a title/icon, sidebar visibility and no YAML.""" + """A fresh install gets the storage-mode strategy dashboard via the + REAL two-step Lovelace sequence: the METADATA (url_path, title, + icon, sidebar, single-word allowance — nothing else: the real + create schema rejects unknown keys like ``strategy``) through the + collection, then the dashboard CONFIG (the strategy reference) + through the fresh dashboard's content manager.""" collection = _install_dashboard_collection(hass) entry = _wire(make_entry(), hass.registry) assert await async_setup_entry(hass, entry) is True + # Step 1: the metadata create through the collection. assert len(collection.created) == 1 created = collection.created[0] assert created["url_path"] == DASHBOARD_URL_PATH - assert created["mode"] == "storage" - assert created["strategy"] == {"type": DASHBOARD_STRATEGY_TYPE} assert created["title"] assert created["show_in_sidebar"] is True + assert created["allow_single_word"] is True assert "icon" in created + # The create payload carries METADATA only: a ``strategy`` key is + # rejected by the real collection's create schema (the silent + # no-op the first implementation shipped). + assert "strategy" not in created + assert "mode" not in created + # Step 2: the strategy CONFIG saved through the content manager + # the create registered, with the custom strategy type. + store = hass.data["lovelace"]["dashboards"][DASHBOARD_URL_PATH] + assert store is collection.stores[DASHBOARD_URL_PATH] + assert store.saved == [{"strategy": {"type": DASHBOARD_STRATEGY_TYPE}}] + assert store.config == {"strategy": {"type": DASHBOARD_STRATEGY_TYPE}} async def test_setup_reload_does_not_duplicate_the_dashboard( hass, make_entry ) -> None: """A reload (or a second entry) must not create a second dashboard: - the one under the stable url_path is reused as-is.""" + the one under the stable url_path is reused as-is — BOTH steps are + skipped, so the strategy config is not re-saved either.""" collection = _install_dashboard_collection(hass) entry = _wire(make_entry(), hass.registry) assert await async_setup_entry(hass, entry) is True assert len(collection.created) == 1 + store = hass.data["lovelace"]["dashboards"][DASHBOARD_URL_PATH] + assert len(store.saved) == 1 # A reload: setup again on the same hass (the reload cycle) — # the existing dashboard must be left alone. await async_unload_entry(hass, entry) assert await async_setup_entry(hass, entry) is True assert len(collection.created) == 1 + assert len(store.saved) == 1 # And a second entry on the same install. other = _wire(make_entry(entry_id="other"), hass.registry) assert await async_setup_entry(hass, other) is True assert len(collection.created) == 1 + assert len(store.saved) == 1 async def test_setup_leaves_an_existing_user_dashboard_alone( hass, make_entry ) -> None: """A dashboard already occupying the stable url_path — the user's - own — is never overwritten, deleted or duplicated.""" + own — is never overwritten, deleted or duplicated: no metadata + create AND no strategy save against its content manager.""" users_dashboard = {"url_path": DASHBOARD_URL_PATH, "title": "My Board"} collection = _install_dashboard_collection(hass, items=[users_dashboard]) entry = _wire(make_entry(), hass.registry) assert await async_setup_entry(hass, entry) is True assert collection.created == [] assert collection.items[0] is users_dashboard + users_store = hass.data["lovelace"]["dashboards"][DASHBOARD_URL_PATH] + assert users_store.saved == [] + assert users_store.config is None async def test_setup_succeeds_without_the_dashboards_collection( @@ -376,8 +457,63 @@ async def test_setup_survives_a_failing_dashboards_collection( async def async_create_item(self, item_config): raise PermissionError("not an admin") - collection = _RefusingCollection() - hass.data["lovelace"] = SimpleNamespace(dashboards=collection) + collection = _RefusingCollection(hass) + hass.data["lovelace"] = { + "mode": "storage", + "dashboards": {}, + "dashboards_collection": collection, + } entry = _wire(make_entry(), hass.registry) assert await async_setup_entry(hass, entry) is True assert collection.created == [] + + +async def test_setup_survives_a_failing_content_manager_save( + hass, make_entry +) -> None: + """A content manager that refuses the config save after the + metadata create succeeded (storage write failure) logs a warning + and setup still succeeds — the dashboard exists, the strategy + simply was not persisted through it.""" + class _RefusingStore(FakeDashboardStore): + async def async_save(self, config): + raise OSError("disk full") + + class _StorelessCreateCollection(FakeDashboardsCollection): + async def async_create_item(self, item_config): + await asyncio.sleep(0) + self.created.append(dict(item_config)) + self.items.append(dict(item_config)) + self.stores[item_config["url_path"]] = _RefusingStore() + self.hass.data["lovelace"]["dashboards"][ + item_config["url_path"] + ] = self.stores[item_config["url_path"]] + + collection = _StorelessCreateCollection(hass) + hass.data["lovelace"] = { + "mode": "storage", + "dashboards": {}, + "dashboards_collection": collection, + } + entry = _wire(make_entry(), hass.registry) + assert await async_setup_entry(hass, entry) is True + assert len(collection.created) == 1 + store = hass.data["lovelace"]["dashboards"][DASHBOARD_URL_PATH] + assert store.saved == [] + + +async def test_concurrent_registrations_create_the_dashboard_once(hass) -> None: + """Two CONCURRENT registrations (the race two entry setups run) + must produce exactly ONE dashboard: the single-flight guard makes + the second caller's exists-check run after the first caller's + create completed, so it skips both steps. The fake's create + yields before recording, so without the guard both callers would + pass the exists-check and the dashboard would be created twice.""" + collection = _install_dashboard_collection(hass) + results = await asyncio.gather( + async_register_dashboard(hass), + async_register_dashboard(hass), + ) + assert results == [True, True] + assert len(collection.created) == 1 + assert len(collection.stores[DASHBOARD_URL_PATH].saved) == 1