From c12a8ebcd5266d7ded4bf4ff9b15ee174f9eaa8e Mon Sep 17 00:00:00 2001 From: NicoM701 Date: Sun, 30 Aug 2026 23:13:33 +0200 Subject: [PATCH] feat: prune leftover PlantRun device-registry shells Home Assistant refused to delete empty leftover run devices because PlantRun never implemented device removal. Allow registry delete for identifiers that are not in current storage.runs, and drop those shells on setup or reload. Co-authored-by: Cursor --- custom_components/plantrun/__init__.py | 30 +++ custom_components/plantrun/device_cleanup.py | 44 +++++ tests/test_orphan_device_shells.py | 184 +++++++++++++++++++ tests/test_stability_lifecycle.py | 114 +++++++++++- 4 files changed, 371 insertions(+), 1 deletion(-) create mode 100644 custom_components/plantrun/device_cleanup.py create mode 100644 tests/test_orphan_device_shells.py diff --git a/custom_components/plantrun/__init__.py b/custom_components/plantrun/__init__.py index 410cf83..22e76c2 100644 --- a/custom_components/plantrun/__init__.py +++ b/custom_components/plantrun/__init__.py @@ -36,6 +36,11 @@ UNSUPPORTED_BINDING_METRIC_TYPES, ) from .coordinator import PlantRunCoordinator +from .device_cleanup import ( + async_prune_orphan_devices, + device_may_be_removed, + live_run_ids, +) from .domain import DomainError from .history_context import build_binding_history_context from .models import Binding, CultivarSnapshot, Note, Phase, RunData @@ -542,6 +547,16 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: "y" if removed_legacy_entities == 1 else "ies", ) + removed_orphan_devices = async_prune_orphan_devices( + hass, entry, live_run_ids(storage) + ) + if removed_orphan_devices: + _LOGGER.info( + "Removed %s leftover PlantRun device-registry shell%s during setup.", + removed_orphan_devices, + "" if removed_orphan_devices == 1 else "s", + ) + await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) entry.async_on_unload(entry.add_update_listener(async_reload_entry)) @@ -1111,6 +1126,21 @@ def register_service(name: str, handler: Any, schema: vol.Schema) -> None: return True +async def async_remove_config_entry_device( + hass: HomeAssistant, + config_entry: ConfigEntry, + device_entry: Any, +) -> bool: + """Allow deleting leftover run devices without unloading the config entry.""" + runtime = hass.data.get(DOMAIN, {}).get(config_entry.entry_id) + if not isinstance(runtime, dict): + return False + storage = runtime.get("storage") + if storage is None: + return False + return device_may_be_removed(device_entry, live_run_ids(storage)) + + async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Unload a config entry.""" unload_ok = await hass.config_entries.async_unload_platforms(entry, PLATFORMS) diff --git a/custom_components/plantrun/device_cleanup.py b/custom_components/plantrun/device_cleanup.py new file mode 100644 index 0000000..7704826 --- /dev/null +++ b/custom_components/plantrun/device_cleanup.py @@ -0,0 +1,44 @@ +"""Drop leftover PlantRun device-registry shells that no longer match a live run.""" + +from __future__ import annotations + +from typing import Any + +from homeassistant.core import HomeAssistant +from homeassistant.helpers import device_registry as dr + +from .const import DOMAIN + + +def live_run_ids(storage: Any) -> set[str]: + """Return current storage.runs ids only. Never the hidden v2 legacy bucket.""" + return {run.id for run in getattr(storage, "runs", []) if getattr(run, "id", None)} + + +def _plantrun_identifiers(device: Any) -> list[str]: + identifiers = getattr(device, "identifiers", ()) or () + return [ident for domain, ident in identifiers if domain == DOMAIN] + + +def device_may_be_removed(device: Any, live_ids: set[str]) -> bool: + """Return True when every PlantRun identifier is absent from current storage.runs.""" + identifiers = _plantrun_identifiers(device) + if not identifiers: + return False + return all(ident not in live_ids for ident in identifiers) + + +def async_prune_orphan_devices( + hass: HomeAssistant, + entry: Any, + live_ids: set[str], +) -> int: + """Remove config-entry devices whose PlantRun identifiers are not live run ids.""" + registry = dr.async_get(hass) + removed = 0 + for device in dr.async_entries_for_config_entry(registry, entry.entry_id): + if not device_may_be_removed(device, live_ids): + continue + registry.async_remove_device(device.id) + removed += 1 + return removed diff --git a/tests/test_orphan_device_shells.py b/tests/test_orphan_device_shells.py new file mode 100644 index 0000000..0296bb1 --- /dev/null +++ b/tests/test_orphan_device_shells.py @@ -0,0 +1,184 @@ +import importlib.util +import sys +import types +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +PLANTRUN_DIR = ROOT / "custom_components" / "plantrun" + +DOMAIN = "plantrun" +LIVE_RUN_ID = "run_2df887ffb2a4456389d1c05a3c66125c" +ORPHAN_ID = "0b46f385bb4541c0a862f1fdf70570be" +OTHER_ORPHAN_ID = "68b5f27b118a48e2a2cf302d0f3c5e72" + + +def _load_module(name: str, path: Path): + spec = importlib.util.spec_from_file_location(name, path) + assert spec and spec.loader + module = importlib.util.module_from_spec(spec) + sys.modules[name] = module + spec.loader.exec_module(module) + return module + + +class FakeDevice: + def __init__(self, device_id: str, identifiers, config_entries): + self.id = device_id + self.identifiers = set(identifiers) + self.config_entries = set(config_entries) + + +class FakeDeviceRegistry: + def __init__(self, devices=None): + self.devices = {device.id: device for device in (devices or [])} + self.removed = [] + + def async_remove_device(self, device_id: str) -> None: + if device_id in self.devices: + self.removed.append(device_id) + self.devices.pop(device_id, None) + + +class FakeStorage: + def __init__(self, runs): + self.runs = runs + self.legacy_v2 = {"runs": [{"id": "legacy-hidden-run"}]} + + +class FakeRun: + def __init__(self, run_id: str): + self.id = run_id + + +def _install_stubs() -> None: + ha = types.ModuleType("homeassistant") + sys.modules.setdefault("homeassistant", ha) + + core = types.ModuleType("homeassistant.core") + core.HomeAssistant = object + sys.modules["homeassistant.core"] = core + + helpers = types.ModuleType("homeassistant.helpers") + helpers.__path__ = [] + device_registry_mod = types.ModuleType("homeassistant.helpers.device_registry") + + def async_get(hass): + return hass.device_registry + + def async_entries_for_config_entry(registry, entry_id): + return [ + device + for device in registry.devices.values() + if entry_id in device.config_entries + ] + + device_registry_mod.async_get = async_get + device_registry_mod.async_entries_for_config_entry = async_entries_for_config_entry + helpers.device_registry = device_registry_mod + sys.modules["homeassistant.helpers"] = helpers + sys.modules["homeassistant.helpers.device_registry"] = device_registry_mod + + custom_components = types.ModuleType("custom_components") + custom_components.__path__ = [str(ROOT / "custom_components")] + sys.modules.setdefault("custom_components", custom_components) + plantrun_pkg = types.ModuleType("custom_components.plantrun") + plantrun_pkg.__path__ = [str(PLANTRUN_DIR)] + sys.modules["custom_components.plantrun"] = plantrun_pkg + + +_install_stubs() +_load_module("custom_components.plantrun.const", PLANTRUN_DIR / "const.py") +CLEANUP = _load_module( + "custom_components.plantrun.device_cleanup", + PLANTRUN_DIR / "device_cleanup.py", +) + + +def _orphan_device(entry_id="entry-1"): + return FakeDevice( + "0ba35f45b515117a8d3145580e6e6aeb", + {(DOMAIN, ORPHAN_ID)}, + {entry_id}, + ) + + +def _live_device(entry_id="entry-1"): + return FakeDevice( + "live-device", + {(DOMAIN, LIVE_RUN_ID)}, + {entry_id}, + ) + + +def _foreign_device(entry_id="entry-1"): + return FakeDevice( + "foreign-device", + {("hue", "abc123")}, + {entry_id}, + ) + + +class OrphanDeviceShellTests(unittest.TestCase): + def test_orphan_bare_uuid_may_be_removed(self): + live_ids = CLEANUP.live_run_ids(FakeStorage([FakeRun(LIVE_RUN_ID)])) + self.assertTrue(CLEANUP.device_may_be_removed(_orphan_device(), live_ids)) + + def test_live_run_device_must_not_be_removed(self): + live_ids = CLEANUP.live_run_ids(FakeStorage([FakeRun(LIVE_RUN_ID)])) + self.assertFalse(CLEANUP.device_may_be_removed(_live_device(), live_ids)) + + def test_hidden_legacy_bucket_does_not_keep_a_shell(self): + storage = FakeStorage([FakeRun(LIVE_RUN_ID)]) + live_ids = CLEANUP.live_run_ids(storage) + self.assertNotIn("legacy-hidden-run", live_ids) + self.assertTrue( + CLEANUP.device_may_be_removed( + FakeDevice("legacy-shell", {(DOMAIN, "legacy-hidden-run")}, {"entry-1"}), + live_ids, + ) + ) + + def test_foreign_domain_device_must_not_be_removed(self): + live_ids = CLEANUP.live_run_ids(FakeStorage([FakeRun(LIVE_RUN_ID)])) + self.assertFalse(CLEANUP.device_may_be_removed(_foreign_device(), live_ids)) + + def test_prune_removes_orphan_shells_and_keeps_live_and_foreign_devices(self): + entry = types.SimpleNamespace(entry_id="entry-1") + hass = types.SimpleNamespace( + device_registry=FakeDeviceRegistry( + [ + _orphan_device(), + FakeDevice( + "2add613e0385d5f9695f1501312d94fc", + {(DOMAIN, OTHER_ORPHAN_ID)}, + {entry.entry_id}, + ), + _live_device(), + _foreign_device(), + FakeDevice( + "other-entry-orphan", + {(DOMAIN, "dead-on-other-entry")}, + {"other-entry"}, + ), + ] + ) + ) + + removed = CLEANUP.async_prune_orphan_devices( + hass, + entry, + CLEANUP.live_run_ids(FakeStorage([FakeRun(LIVE_RUN_ID)])), + ) + + self.assertEqual(removed, 2) + self.assertEqual( + hass.device_registry.removed, + [ + "0ba35f45b515117a8d3145580e6e6aeb", + "2add613e0385d5f9695f1501312d94fc", + ], + ) + self.assertIn("live-device", hass.device_registry.devices) + self.assertIn("foreign-device", hass.device_registry.devices) + self.assertIn("other-entry-orphan", hass.device_registry.devices) diff --git a/tests/test_stability_lifecycle.py b/tests/test_stability_lifecycle.py index 14cec39..96fdf4e 100644 --- a/tests/test_stability_lifecycle.py +++ b/tests/test_stability_lifecycle.py @@ -189,13 +189,29 @@ def async_get(hass): def async_entries_for_config_entry(registry, entry_id): return [entry for entry in registry.entries.values() if entry.config_entry_id == entry_id] + device_registry_mod = types.ModuleType("homeassistant.helpers.device_registry") + + def async_get_devices(hass): + return hass.device_registry + + def async_device_entries_for_config_entry(registry, entry_id): + return [ + device + for device in registry.devices.values() + if entry_id in device.config_entries + ] + helpers_mod.__path__ = [] helpers_mod.entity_registry = entity_registry_mod + helpers_mod.device_registry = device_registry_mod sys.modules["homeassistant.helpers"] = helpers_mod sys.modules["homeassistant.helpers.selector"] = selector_mod entity_registry_mod.async_get = async_get entity_registry_mod.async_entries_for_config_entry = async_entries_for_config_entry sys.modules["homeassistant.helpers.entity_registry"] = entity_registry_mod + device_registry_mod.async_get = async_get_devices + device_registry_mod.async_entries_for_config_entry = async_device_entries_for_config_entry + sys.modules["homeassistant.helpers.device_registry"] = device_registry_mod aiohttp_client = types.ModuleType("homeassistant.helpers.aiohttp_client") aiohttp_client._session = object() @@ -217,9 +233,10 @@ class ServiceValidationError(Exception): class FakeStorage: instances = [] + seed_runs = [] def __init__(self, _hass=None): - self.runs = [] + self.runs = list(FakeStorage.seed_runs) self.active_run_id = None self.saved_runs = [] self.calls = [] @@ -328,6 +345,24 @@ def async_remove(self, entity_id: str) -> None: self.entries.pop(entity_id, None) +class FakeDevice: + def __init__(self, device_id: str, identifiers, config_entries): + self.id = device_id + self.identifiers = set(identifiers) + self.config_entries = set(config_entries) + + +class FakeDeviceRegistry: + def __init__(self, devices=None): + self.devices = {device.id: device for device in (devices or [])} + self.removed = [] + + def async_remove_device(self, device_id: str) -> None: + if device_id in self.devices: + self.removed.append(device_id) + self.devices.pop(device_id, None) + + class StabilityLifecycleTests(unittest.TestCase): @classmethod def setUpClass(cls): @@ -395,6 +430,7 @@ async def async_fetch_cultivar_image(_detail_url, _strain_name=None, session=Non def setUp(self): self.providers.calls.clear() FakeStorage.instances.clear() + FakeStorage.seed_runs = [] self.tmpdir = Path(tempfile.mkdtemp(prefix="plantrun-test-")) def tearDown(self): @@ -416,6 +452,7 @@ async def async_add_executor_job(func): config_entries=FakeConfigEntries(), async_add_executor_job=async_add_executor_job, entity_registry=FakeEntityRegistry(), + device_registry=FakeDeviceRegistry(), ) hass._executor_calls = executor_calls return hass @@ -499,6 +536,81 @@ def test_setup_entry_removes_only_known_legacy_singleton_entities(self): self.assertIn("sensor.plantrun_active_phase_other_entry", hass.entity_registry.entries) self.assertIn("sensor.plantrun_active_phase_run123", hass.entity_registry.entries) + def test_remove_config_entry_device_allows_orphan_and_refuses_live(self): + hass = self._build_hass() + entry = sys.modules["homeassistant.config_entries"].ConfigEntry("entry-devices") + live_run_id = "run_2df887ffb2a4456389d1c05a3c66125c" + FakeStorage.seed_runs = [ + self.models.RunData( + friendly_name="Diesel Auto RQS", + start_time="2026-08-25T00:00:00+00:00", + id=live_run_id, + ) + ] + asyncio.run(self.integration.async_setup_entry(hass, entry)) + + orphan = FakeDevice( + "0ba35f45b515117a8d3145580e6e6aeb", + {("plantrun", "0b46f385bb4541c0a862f1fdf70570be")}, + {entry.entry_id}, + ) + live = FakeDevice("live-device", {("plantrun", live_run_id)}, {entry.entry_id}) + foreign = FakeDevice("foreign-device", {("hue", "abc123")}, {entry.entry_id}) + + allowed = asyncio.run( + self.integration.async_remove_config_entry_device(hass, entry, orphan) + ) + refused_live = asyncio.run( + self.integration.async_remove_config_entry_device(hass, entry, live) + ) + refused_foreign = asyncio.run( + self.integration.async_remove_config_entry_device(hass, entry, foreign) + ) + + self.assertTrue(allowed) + self.assertFalse(refused_live) + self.assertFalse(refused_foreign) + + def test_setup_entry_prunes_orphan_device_shells(self): + hass = self._build_hass() + entry = sys.modules["homeassistant.config_entries"].ConfigEntry("entry-devices") + live_run_id = "run_2df887ffb2a4456389d1c05a3c66125c" + FakeStorage.seed_runs = [ + self.models.RunData( + friendly_name="Diesel Auto RQS", + start_time="2026-08-25T00:00:00+00:00", + id=live_run_id, + ) + ] + hass.device_registry = FakeDeviceRegistry( + [ + FakeDevice( + "0ba35f45b515117a8d3145580e6e6aeb", + {("plantrun", "0b46f385bb4541c0a862f1fdf70570be")}, + {entry.entry_id}, + ), + FakeDevice( + "2add613e0385d5f9695f1501312d94fc", + {("plantrun", "68b5f27b118a48e2a2cf302d0f3c5e72")}, + {entry.entry_id}, + ), + FakeDevice("live-device", {("plantrun", live_run_id)}, {entry.entry_id}), + FakeDevice("foreign-device", {("hue", "abc123")}, {entry.entry_id}), + ] + ) + + asyncio.run(self.integration.async_setup_entry(hass, entry)) + + self.assertEqual( + hass.device_registry.removed, + [ + "0ba35f45b515117a8d3145580e6e6aeb", + "2add613e0385d5f9695f1501312d94fc", + ], + ) + self.assertIn("live-device", hass.device_registry.devices) + self.assertIn("foreign-device", hass.device_registry.devices) + def test_setup_entry_cleanup_is_idempotent(self): hass = self._build_hass() entry = sys.modules["homeassistant.config_entries"].ConfigEntry("entry-cleanup")