diff --git a/custom_components/sws12500/__init__.py b/custom_components/sws12500/__init__.py index 7cdc35e..aa7f3dc 100644 --- a/custom_components/sws12500/__init__.py +++ b/custom_components/sws12500/__init__.py @@ -56,6 +56,7 @@ from .coordinator import WeatherDataUpdateCoordinator from .data import SWSConfigEntry, SWSRuntimeData from .health_coordinator import HealthCoordinator from .legacy import update_legacy_battery_issue +from .predecessor import async_adopt_predecessor, inherit_predecessor_options from .routes import Routes from .staleness import update_stale_sensors_issue @@ -143,9 +144,17 @@ def register_path( # Save initialised routes hass_data["routes"] = routes - except RuntimeError as Ex: + # ValueError as well as RuntimeError: aiohttp rejects a duplicate route *name* + # with ValueError, and the names here are fixed rather than domain-scoped. That + # is what a half-finished migration looks like - the previous version is still + # installed and holding the same routes - so it deserves a message the user can + # act on rather than an unhandled traceback. + except (RuntimeError, ValueError) as Ex: _LOGGER.critical("Routes cannot be added. Integration will not work as expected. %s", Ex) - raise ConfigEntryNotReady from Ex + raise ConfigEntryNotReady( + "Webhook routes are already registered by another instance of this " + "integration. Remove the previous version in HACS and restart Home Assistant." + ) from Ex # Finally create internal route dispatcher with provided urls, while we have webhooks registered. routes.add_route(DEFAULT_URL, _default_route, coordinator.received_data, enabled=_legacy and not _wslink) @@ -185,6 +194,11 @@ async def async_setup_entry(hass: HomeAssistant, entry: SWSConfigEntry) -> bool: hass.data.setdefault(DOMAIN, {}) + # First, because everything below reads the options: the protocol flags decide which + # coordinator and which webhook routes get wired up, and inheriting `WSLINK` after + # that point would leave the entry listening on the wrong endpoint until a reload. + inherit_predecessor_options(hass, entry) + coordinator = WeatherDataUpdateCoordinator(hass, entry) coordinator_health = HealthCoordinator(hass, entry) @@ -225,6 +239,20 @@ async def async_setup_entry(hass: HomeAssistant, entry: SWSConfigEntry) -> bool: await coordinator_health.async_config_entry_first_refresh() coordinator_health.update_forwarding(coordinator.windy, coordinator.pocasi) + # Late enough that the routes are proven, early enough that no entity exists yet. + # + # Late: adoption is the one destructive step - it removes the predecessor's config + # entry. Running it before the routes are up means a station that still has the old + # version installed gets its entry deleted and then a failed setup, with nothing to + # fall back to. Everything above this line can raise ConfigEntryNotReady and leave + # the user exactly where they started. + # + # Early: adoption rewrites the predecessor's registry entries in place and our + # platforms then bind to those same entries by unique id, keeping their entity_id + # and with it their recorder history. After `async_forward_entry_setups` the + # entity_ids would already have been handed out to freshly created entities. + await async_adopt_predecessor(hass, entry) + await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) @callback diff --git a/custom_components/sws12500/const.py b/custom_components/sws12500/const.py index b236a90..771350c 100644 --- a/custom_components/sws12500/const.py +++ b/custom_components/sws12500/const.py @@ -7,6 +7,13 @@ from typing import Final # Integration specific constants. DOMAIN = "sws12500" + +# Domain this integration was published under before the rename. Entities registered by +# it are adopted on setup so their recorder history survives the move - see +# `predecessor.py`. While the two are equal there is nothing to adopt and the whole pass +# short-circuits, so this stays correct until the day the domain above changes. +PREDECESSOR_DOMAIN: Final = "sws12500" + DEV_DBG: Final = "dev_debug_checkbox" diff --git a/custom_components/sws12500/predecessor.py b/custom_components/sws12500/predecessor.py new file mode 100644 index 0000000..18072f8 --- /dev/null +++ b/custom_components/sws12500/predecessor.py @@ -0,0 +1,275 @@ +"""Adopt the predecessor integration's entities without losing their history. + +Home Assistant keys recorder history and long-term statistics on `entity_id`, not on the +integration domain: `recorder/entity_registry.py` renames both `states_meta.entity_id` +and `statistics_meta.statistic_id` when - and only when - an `entity_id` changes. A +domain change therefore costs nothing *provided the registry entries are carried over +rather than recreated*. A recreated entity gets a fresh `entity_id` (or the same one +with an `_2` suffix, if the old entry still holds it) and its history is orphaned. + +`entity_registry.async_update_entity_platform` is the supported way to carry them over. +It rewrites `platform` and `config_entry_id` and leaves `entity_id` alone. Everything +the user set by hand rides along for free, because the registry entry itself survives: +renames, icons, areas, labels, categories, hidden and disabled state. + +This constrains the upgrade procedure, and the README has to say so plainly: + +1. Do **not** delete the old integration under Settings first. That path calls + `entity_registry.async_clear_config_entry`, which removes exactly the entries this + module needs. Once they are gone the history cannot be relinked. +2. Swap the repository in HACS and restart. The old entry stays behind in + `SETUP_ERROR` ("Integration not found"), which is expected and is the state adoption + needs. +3. Add this integration. Adoption runs during setup, before any of our own entities are + created, so the migrated entries are the ones our platforms bind to. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field +import logging + +from homeassistant.config_entries import ConfigEntry, ConfigEntryState +from homeassistant.const import ATTR_RESTORED, STATE_UNKNOWN +from homeassistant.core import HomeAssistant +from homeassistant.helpers import device_registry as dr, entity_registry as er + +from .const import DOMAIN, POCASI_CZ_ENABLED, POCASI_CZ_ENABLED_LEGACY, PREDECESSOR_DOMAIN +from .data import build_device_info + +_LOGGER = logging.getLogger(__name__) + + +@dataclass(slots=True) +class AdoptionResult: + """Outcome of one adoption pass, so the caller can report it and retry.""" + + adopted: list[str] = field(default_factory=list) + live: list[str] = field(default_factory=list) + conflicting: list[str] = field(default_factory=list) + predecessor_removed: bool = False + + @property + def complete(self) -> bool: + """Whether every entity of the predecessor was carried over.""" + return not self.live and not self.conflicting + + @property + def attempted(self) -> bool: + """Whether there was anything to do at all.""" + return bool(self.adopted or self.live or self.conflicting) + + +def _release_restored_state(hass: HomeAssistant, entity_id: str) -> bool: + """Drop the placeholder state Home Assistant writes for an unloaded entity. + + At startup `entity_registry._write_unavailable_states` gives every registered but + unloaded entity a state of `unavailable` carrying `ATTR_RESTORED`. That is exactly + the state the predecessor's entities are in once its files are gone from disk - and + `async_update_entity_platform` accepts only a missing state or `unknown`, so without + clearing the placeholder first every single adoption would raise `ValueError`. + + Returns False when a state is present that is *not* a placeholder. That means some + integration is still driving this entity, and moving it out from under a live + platform would leave two owners for one registry entry. + """ + + state = hass.states.get(entity_id) + if state is None or state.state == STATE_UNKNOWN: + return True + + if not state.attributes.get(ATTR_RESTORED): + return False + + hass.states.async_remove(entity_id) + return True + + +def inherit_predecessor_options( + hass: HomeAssistant, + entry: ConfigEntry, + *, + predecessor_domain: str | None = None, +) -> bool: + """Carry the predecessor's settings over, without overwriting fresh input. + + Deliberately separate from `async_adopt_predecessor`, and deliberately run earlier: + setup reads the options while wiring up the coordinator and the webhook routes, so + inheriting an option after that point would leave the entry running on the wrong + protocol until the next reload. It is also additive and reversible in a way the rest + of adoption is not, which is why it can safely run before the routes are proven. + + Adopting the registry alone would not be enough anyway. `SENSORS_TO_LOAD` decides + which entities the platform creates, so an empty one leaves every adopted entry + showing "no longer provided" until auto-discovery has seen a payload - and derived + sensors like the wind azimut are never in a payload at all, so those come back only + on the next restart. The forwarding credentials and the learned probe types would + likewise have to be entered again. + + Merged the other way round from what reads naturally: the predecessor fills gaps + only. Anything the user just entered in this integration's config flow is newer by + definition and wins. + + The version 1 spelling of the Pocasi Meteo flag is translated on the way in, because + `async_migrate_entry` runs against this entry's own version - already current - and + would never look at a key copied in afterwards. + """ + + predecessor_domain = predecessor_domain or PREDECESSOR_DOMAIN + if predecessor_domain == DOMAIN: + return False + + if not (old_entries := hass.config_entries.async_entries(predecessor_domain)): + return False + + inherited: dict[str, object] = {} + for old_entry in old_entries: + inherited.update(old_entry.options) + + if POCASI_CZ_ENABLED_LEGACY in inherited: + legacy_value = inherited.pop(POCASI_CZ_ENABLED_LEGACY) + inherited.setdefault(POCASI_CZ_ENABLED, legacy_value) + + merged = {**inherited, **entry.options} + if merged == dict(entry.options): + return False + + gained = len(merged) - len(entry.options) + hass.config_entries.async_update_entry(entry, options=merged) + _LOGGER.debug("Inherited %s settings from the previous integration", gained) + return True + + +def _adopt_devices(hass: HomeAssistant, entry: ConfigEntry, old_entry: ConfigEntry) -> None: + """Repoint the predecessor's devices at this entry, keeping the same device rows. + + The identifiers come from `build_device_info` rather than being rebuilt here, so the + device this leaves behind is by construction the one our platforms will look up. If + the two ever drifted, `async_get_or_create` would quietly mint a second device and + strand the adopted one. + + Add the new config entry before removing the old one. A device left with no config + entries is deleted, and deleting a device takes its entities with it + (`entity_registry.async_device_modified`). + """ + + device_registry = dr.async_get(hass) + identifiers = set(build_device_info(entry)["identifiers"]) + + for device in dr.async_entries_for_config_entry(device_registry, old_entry.entry_id): + device_registry.async_update_device( + device.id, + new_identifiers=identifiers, # type: ignore[arg-type] same 1-tuple shape as build_device_info + add_config_entry_id=entry.entry_id, + ) + device_registry.async_update_device(device.id, remove_config_entry_id=old_entry.entry_id) + + +async def _adopt_entities( + hass: HomeAssistant, + entry: ConfigEntry, + old_entry: ConfigEntry, + result: AdoptionResult, +) -> None: + """Move every registry entry of `old_entry` onto this integration.""" + + registry = er.async_get(hass) + + for old in er.async_entries_for_config_entry(registry, old_entry.entry_id): + # Home Assistant does not check this itself: the duplicate-unique_id guard in + # `_async_update_entity` only runs when `new_unique_id` is passed, so changing + # just the platform onto an already-taken key would leave two registry entries + # sharing one index slot. Happens when a user sets this integration up fresh + # and only then tries to adopt. + if (clash := registry.async_get_entity_id(old.domain, DOMAIN, old.unique_id)) is not None: + _LOGGER.warning( + "Cannot adopt %s: unique id %r is already used by %s", + old.entity_id, + old.unique_id, + clash, + ) + result.conflicting.append(old.entity_id) + continue + + if not _release_restored_state(hass, old.entity_id): + _LOGGER.warning( + "Cannot adopt %s: it still has a live state, so an integration is driving it", + old.entity_id, + ) + result.live.append(old.entity_id) + continue + + registry.async_update_entity_platform(old.entity_id, DOMAIN, new_config_entry_id=entry.entry_id) + result.adopted.append(old.entity_id) + + +async def async_adopt_predecessor( + hass: HomeAssistant, + entry: ConfigEntry, + *, + predecessor_domain: str | None = None, +) -> AdoptionResult: + """Carry the predecessor integration's entities over to this one. + + Safe to call on every setup: with no predecessor entry left it is a dictionary + lookup. Safe to call again after a partial run, because each entity is decided + independently and an already-adopted one is simply no longer listed under the old + config entry. + """ + + result = AdoptionResult() + + predecessor_domain = predecessor_domain or PREDECESSOR_DOMAIN + if predecessor_domain == DOMAIN: + # Nothing was renamed yet, so there is no predecessor to adopt from. + return result + + if not (old_entries := hass.config_entries.async_entries(predecessor_domain)): + return result + + # Normally already done by setup, before the options were read. Repeated here so a + # caller that forgets cannot silently drop the settings - it is a no-op second time. + inherit_predecessor_options(hass, entry, predecessor_domain=predecessor_domain) + + for old_entry in old_entries: + if old_entry.state is ConfigEntryState.LOADED: + # Its entities cannot be migrated while loaded, and both integrations would + # otherwise fight over the same webhook routes. Unload rather than refuse: + # the user is mid-migration and this entry is on its way out anyway. + if not await hass.config_entries.async_unload(old_entry.entry_id): + _LOGGER.error( + "Could not unload the previous integration (%s); adoption skipped. " + "Restart Home Assistant and try again", + old_entry.entry_id, + ) + continue + + await _adopt_entities(hass, entry, old_entry, result) + + # Before removing the old entry, not after: `async_remove` clears the entry from + # its devices, and a device that loses its last config entry is deleted along + # with the entities pointing at it. + _adopt_devices(hass, entry, old_entry) + + if not result.complete: + # Removing the entry now would delete whatever we could not carry over. + # Leaving it in place costs a repair notice and keeps a retry possible. + _LOGGER.error( + "Adopted %s entities but %s could not be moved; the previous integration " + "is left in place so nothing is lost", + len(result.adopted), + len(result.live) + len(result.conflicting), + ) + continue + + await hass.config_entries.async_remove(old_entry.entry_id) + result.predecessor_removed = True + + if result.adopted: + _LOGGER.info( + "Adopted %s entities from %s; their history and statistics are unchanged", + len(result.adopted), + predecessor_domain, + ) + + return result diff --git a/custom_components/sws12500/sensor.py b/custom_components/sws12500/sensor.py index 7207999..c6ade14 100644 --- a/custom_components/sws12500/sensor.py +++ b/custom_components/sws12500/sensor.py @@ -45,7 +45,7 @@ from .data import SWSConfigEntry, build_device_info from .sensors_common import WeatherSensorEntityDescription from .sensors_weather import SENSOR_TYPES_WEATHER_API from .sensors_wslink import SENSOR_TYPES_WSLINK -from .utils import channel_humidity_device_class, channel_types +from .utils import channel_humidity_device_class, channel_types, loaded_sensors if TYPE_CHECKING: from .coordinator import WeatherDataUpdateCoordinator @@ -127,6 +127,10 @@ def add_new_sensors(hass: HomeAssistant, config_entry: SWSConfigEntry, keys: lis - This function is intentionally a safe no-op if the sensor platform hasn't finished setting up yet (e.g. callback/description map missing). - Unknown payload keys are ignored (only keys with an entity description are added). + - Derived sensors are expanded here exactly as in `async_setup_entry`. They are + computed, never sent, so discovery can only ever offer their *inputs* - and + without this the azimut a newly discovered wind direction unlocks would sit + missing until the next restart, when platform setup finally applies the expansion. """ del hass # kept for backwards-compatible call signature; not used after runtime_data migration @@ -141,8 +145,18 @@ def add_new_sensors(hass: HomeAssistant, config_entry: SWSConfigEntry, keys: lis descriptions = runtime.sensor_descriptions coordinator = runtime.coordinator + # Only the derived sensors *this batch* unlocks, which is what keeps the expansion + # from re-adding one that platform setup or an earlier payload already created: + # anything derivable from the previous set was created back then. The caller has + # already written `keys` into `SENSORS_TO_LOAD`, so subtracting them recovers it. + loaded = set(loaded_sensors(config_entry)) + previously = loaded - set(keys) + unlocked = _auto_enable_derived_sensors(loaded) - _auto_enable_derived_sensors(previously) + new_entities: list[SensorEntity] = [ - WeatherSensor(desc, coordinator) for key in keys if (desc := descriptions.get(key)) is not None + WeatherSensor(desc, coordinator) + for key in sorted(set(keys) | unlocked) + if (desc := descriptions.get(key)) is not None ] if new_entities: diff --git a/tests/test_integration_lifecycle.py b/tests/test_integration_lifecycle.py index 1291684..3976207 100644 --- a/tests/test_integration_lifecycle.py +++ b/tests/test_integration_lifecycle.py @@ -115,8 +115,20 @@ async def test_register_path_registers_routes_and_stores_dispatcher(hass_with_ht @pytest.mark.asyncio -async def test_register_path_raises_config_entry_not_ready_on_router_runtime_error( +@pytest.mark.parametrize( + "router_error", + [ + RuntimeError("router broken"), + # aiohttp's own wording when a route *name* is taken. The names here are fixed + # rather than domain-scoped, so a previous version of this integration that is + # still installed holds them - the state a half-finished migration leaves behind. + ValueError("Duplicate '_default_route', already handled by "), + ], + ids=["runtime_error", "duplicate_route_name"], +) +async def test_register_path_raises_config_entry_not_ready_on_router_error( hass_with_http, + router_error: Exception, ): from homeassistant.exceptions import ConfigEntryNotReady @@ -131,11 +143,15 @@ async def test_register_path_raises_config_entry_not_ready_on_router_runtime_err coordinator_health = HealthCoordinator(hass_with_http, entry) router: _RouterStub = hass_with_http.http.app.router - router.raise_on_add = RuntimeError("router broken") + router.raise_on_add = router_error - with pytest.raises(ConfigEntryNotReady): + with pytest.raises(ConfigEntryNotReady) as excinfo: register_path(hass_with_http, coordinator, coordinator_health, entry) + # The message is the only thing the user sees on the integrations page, so it has to + # name the fix rather than repeat the aiohttp internals. + assert "HACS" in str(excinfo.value) + @pytest.mark.asyncio async def test_register_path_checked_hass_data_wrong_type_raises_config_entry_not_ready( diff --git a/tests/test_predecessor_adoption.py b/tests/test_predecessor_adoption.py new file mode 100644 index 0000000..fc1aafa --- /dev/null +++ b/tests/test_predecessor_adoption.py @@ -0,0 +1,519 @@ +"""Adopting the predecessor integration must not cost a single measurement. + +Recorder history and long-term statistics hang off `entity_id`. Every test here exists +to defend one sentence: after adoption the `entity_id` is byte-for-byte what it was. +Everything else - the platform, the config entry, the device - may change freely. + +The tests drive the real entity and device registries rather than stubs, because the +failure modes being guarded against (a placeholder `unavailable` state blocking the +migration, a device deleted with its entities in tow, a duplicate unique id landing in +the registry index) all live inside Home Assistant's own bookkeeping. +""" + +from __future__ import annotations + +from typing import Any + +import pytest +from pytest_homeassistant_custom_component.common import MockConfigEntry + +from custom_components.sws12500.const import ( + CHANNEL_TYPES, + DOMAIN, + POCASI_CZ_ENABLED, + POCASI_CZ_ENABLED_LEGACY, + SENSORS_TO_LOAD, +) +from custom_components.sws12500.data import build_device_info +from custom_components.sws12500.predecessor import async_adopt_predecessor +from homeassistant.const import ATTR_RESTORED, STATE_UNAVAILABLE, STATE_UNKNOWN +from homeassistant.core import HomeAssistant +from homeassistant.helpers import device_registry as dr, entity_registry as er + +OLD_DOMAIN = "sws12500_legacy" + +# A representative slice: a plain sensor, a binary sensor, and the suffixed unique id +# the battery entities use. +SEEDED = ( + ("sensor", "outside_temp"), + ("sensor", "lightning_distance"), + ("binary_sensor", "leak_ch1_binary"), +) + + +@pytest.fixture +def entries(hass: HomeAssistant) -> tuple[MockConfigEntry, MockConfigEntry]: + """A predecessor entry with entities, plus the new entry adopting them.""" + old = MockConfigEntry(domain=OLD_DOMAIN, title="Old", options={"api_id": "x"}) + old.add_to_hass(hass) + new = MockConfigEntry(domain=DOMAIN, title="New") + new.add_to_hass(hass) + return old, new + + +def _seed(hass: HomeAssistant, old: MockConfigEntry, **kwargs: Any) -> dict[str, str]: + """Register the predecessor's entities; returns unique_id -> entity_id.""" + registry = er.async_get(hass) + created: dict[str, str] = {} + for domain, unique_id in SEEDED: + entry = registry.async_get_or_create( + domain, OLD_DOMAIN, unique_id, config_entry=old, **kwargs + ) + created[unique_id] = entry.entity_id + return created + + +# --------------------------------------------------------------------------- +# The one that matters +# --------------------------------------------------------------------------- + + +async def test_entity_ids_are_untouched(hass: HomeAssistant, entries) -> None: + """History follows entity_id. If this test fails, users lose their measurements.""" + old, new = entries + before = _seed(hass, old) + + result = await async_adopt_predecessor(hass, new, predecessor_domain=OLD_DOMAIN) + + registry = er.async_get(hass) + for unique_id, entity_id in before.items(): + adopted = registry.async_get_entity_id(entity_id.split(".")[0], DOMAIN, unique_id) + assert adopted == entity_id, f"{unique_id} changed entity_id: {entity_id} -> {adopted}" + + assert sorted(result.adopted) == sorted(before.values()) + assert result.complete + + +async def test_unique_ids_are_untouched(hass: HomeAssistant, entries) -> None: + """A rewritten unique id would break the bind between platform and registry entry.""" + old, new = entries + _seed(hass, old) + + await async_adopt_predecessor(hass, new, predecessor_domain=OLD_DOMAIN) + + registry = er.async_get(hass) + adopted = {e.unique_id for e in er.async_entries_for_config_entry(registry, new.entry_id)} + assert adopted == {unique_id for _, unique_id in SEEDED} + + +async def test_platform_and_config_entry_are_repointed(hass: HomeAssistant, entries) -> None: + """The registry entry has to end up owned by this integration.""" + old, new = entries + _seed(hass, old) + + await async_adopt_predecessor(hass, new, predecessor_domain=OLD_DOMAIN) + + registry = er.async_get(hass) + assert not er.async_entries_for_config_entry(registry, old.entry_id) + for entry in er.async_entries_for_config_entry(registry, new.entry_id): + assert entry.platform == DOMAIN + + +async def test_user_customisations_survive(hass: HomeAssistant, entries) -> None: + """Renames, icons and placement are part of what the user would lose on a recreate.""" + old, new = entries + created = _seed(hass, old) + registry = er.async_get(hass) + entity_id = created["outside_temp"] + registry.async_update_entity( + entity_id, + name="Teplota na zahradÄ›", + icon="mdi:thermometer", + area_id="garden", + ) + + await async_adopt_predecessor(hass, new, predecessor_domain=OLD_DOMAIN) + + entry = registry.async_get(entity_id) + assert entry is not None + assert entry.name == "Teplota na zahradÄ›" + assert entry.icon == "mdi:thermometer" + assert entry.area_id == "garden" + + +# --------------------------------------------------------------------------- +# The trap: the placeholder state Home Assistant writes for unloaded entities +# --------------------------------------------------------------------------- + + +async def test_restored_placeholder_does_not_block_adoption(hass: HomeAssistant, entries) -> None: + """The exact production state: old files gone, entities restored as `unavailable`. + + `async_update_entity_platform` accepts only a missing state or `unknown`, and on + startup HA writes `unavailable` + ATTR_RESTORED for every registered but unloaded + entity. Without clearing that placeholder first, every adoption raises ValueError - + which is to say, nobody's history would migrate at all. + """ + old, new = entries + created = _seed(hass, old) + for entity_id in created.values(): + hass.states.async_set(entity_id, STATE_UNAVAILABLE, {ATTR_RESTORED: True}) + + result = await async_adopt_predecessor(hass, new, predecessor_domain=OLD_DOMAIN) + + assert result.complete + assert sorted(result.adopted) == sorted(created.values()) + + +async def test_unknown_state_is_accepted(hass: HomeAssistant, entries) -> None: + """`unknown` is what the registry API itself tolerates.""" + old, new = entries + created = _seed(hass, old) + hass.states.async_set(created["outside_temp"], STATE_UNKNOWN) + + result = await async_adopt_predecessor(hass, new, predecessor_domain=OLD_DOMAIN) + + assert result.complete + + +async def test_live_entity_is_refused(hass: HomeAssistant, entries) -> None: + """A real state means someone owns this entity; stealing it would give it two owners.""" + old, new = entries + created = _seed(hass, old) + live = created["outside_temp"] + hass.states.async_set(live, "21.5") # no ATTR_RESTORED: a genuine reading + + result = await async_adopt_predecessor(hass, new, predecessor_domain=OLD_DOMAIN) + + assert result.live == [live] + assert not result.complete + registry = er.async_get(hass) + still_old = registry.async_get(live) + assert still_old is not None + assert still_old.platform == OLD_DOMAIN, "a live entity must be left exactly as it was" + + +# --------------------------------------------------------------------------- +# Registry integrity +# --------------------------------------------------------------------------- + + +async def test_unique_id_conflict_is_refused(hass: HomeAssistant, entries) -> None: + """HA does not guard this itself when only the platform changes. + + `_async_update_entity` checks for a duplicate unique id only when `new_unique_id` is + passed. Changing just the platform onto a key this integration already uses would + put two registry entries on one index slot, and the loser becomes unreachable. + """ + old, new = entries + created = _seed(hass, old) + registry = er.async_get(hass) + ours = registry.async_get_or_create("sensor", DOMAIN, "outside_temp", config_entry=new) + + result = await async_adopt_predecessor(hass, new, predecessor_domain=OLD_DOMAIN) + + assert result.conflicting == [created["outside_temp"]] + # Both entries still exist and still resolve to different entity_ids. + assert registry.async_get_entity_id("sensor", DOMAIN, "outside_temp") == ours.entity_id + assert registry.async_get_entity_id("sensor", OLD_DOMAIN, "outside_temp") == created["outside_temp"] + + +async def test_predecessor_is_kept_when_anything_was_skipped(hass: HomeAssistant, entries) -> None: + """Removing the old entry deletes whatever is still attached to it.""" + old, new = entries + created = _seed(hass, old) + hass.states.async_set(created["outside_temp"], "21.5") + + result = await async_adopt_predecessor(hass, new, predecessor_domain=OLD_DOMAIN) + + assert not result.predecessor_removed + assert hass.config_entries.async_get_entry(old.entry_id) is not None + registry = er.async_get(hass) + assert registry.async_get(created["outside_temp"]) is not None + + +async def test_predecessor_is_removed_when_complete(hass: HomeAssistant, entries) -> None: + """A finished migration must not leave a broken entry behind in the UI.""" + old, new = entries + _seed(hass, old) + + result = await async_adopt_predecessor(hass, new, predecessor_domain=OLD_DOMAIN) + + assert result.predecessor_removed + assert hass.config_entries.async_get_entry(old.entry_id) is None + + +# --------------------------------------------------------------------------- +# Devices - the second way to lose entities +# --------------------------------------------------------------------------- + + +async def test_device_is_repointed_and_keeps_its_entities(hass: HomeAssistant, entries) -> None: + """Deleting a device deletes its entities, so the device has to move, not be replaced.""" + old, new = entries + device_registry = dr.async_get(hass) + device = device_registry.async_get_or_create( + config_entry_id=old.entry_id, + identifiers={(OLD_DOMAIN,)}, # type: ignore[arg-type] + name="Weather Station", + ) + created = _seed(hass, old, device_id=device.id) + + await async_adopt_predecessor(hass, new, predecessor_domain=OLD_DOMAIN) + + survived = device_registry.async_get(device.id) + assert survived is not None, "the device was deleted, which takes its entities with it" + assert survived.identifiers == set(build_device_info(new)["identifiers"]) + assert new.entry_id in survived.config_entries + assert old.entry_id not in survived.config_entries + + registry = er.async_get(hass) + for entity_id in created.values(): + entry = registry.async_get(entity_id) + assert entry is not None + assert entry.device_id == device.id + + +async def test_device_identifiers_match_what_the_platform_will_look_up( + hass: HomeAssistant, entries +) -> None: + """If adoption and `build_device_info` drift, setup mints a second device. + + The adopted device would then be stranded and every entity would be moved onto the + new one. History survives that, but the device page does not. + """ + old, new = entries + device_registry = dr.async_get(hass) + device = device_registry.async_get_or_create( + config_entry_id=old.entry_id, + identifiers={(OLD_DOMAIN,)}, # type: ignore[arg-type] + name="Weather Station", + ) + _seed(hass, old, device_id=device.id) + + await async_adopt_predecessor(hass, new, predecessor_domain=OLD_DOMAIN) + + looked_up = device_registry.async_get_device(identifiers=set(build_device_info(new)["identifiers"])) + assert looked_up is not None + assert looked_up.id == device.id + + +# --------------------------------------------------------------------------- +# Repeatability +# --------------------------------------------------------------------------- + + +async def test_running_twice_changes_nothing(hass: HomeAssistant, entries) -> None: + """Adoption runs on every setup, so the second pass must be inert.""" + old, new = entries + before = _seed(hass, old) + + await async_adopt_predecessor(hass, new, predecessor_domain=OLD_DOMAIN) + second = await async_adopt_predecessor(hass, new, predecessor_domain=OLD_DOMAIN) + + assert not second.attempted + registry = er.async_get(hass) + for unique_id, entity_id in before.items(): + assert registry.async_get_entity_id(entity_id.split(".")[0], DOMAIN, unique_id) == entity_id + + +async def test_a_partial_run_can_be_finished(hass: HomeAssistant, entries) -> None: + """Interrupted migration: the retry has to pick up what was left behind.""" + old, new = entries + created = _seed(hass, old) + blocked = created["outside_temp"] + hass.states.async_set(blocked, "21.5") + + first = await async_adopt_predecessor(hass, new, predecessor_domain=OLD_DOMAIN) + assert first.live == [blocked] + + # The blocker goes away (old integration finally unloaded / HA restarted). + hass.states.async_remove(blocked) + second = await async_adopt_predecessor(hass, new, predecessor_domain=OLD_DOMAIN) + + assert second.adopted == [blocked] + assert second.complete + assert second.predecessor_removed + registry = er.async_get(hass) + assert registry.async_get_entity_id("sensor", DOMAIN, "outside_temp") == blocked + + +async def test_no_predecessor_is_a_noop(hass: HomeAssistant) -> None: + """The overwhelmingly common case: a fresh install, on every single restart.""" + new = MockConfigEntry(domain=DOMAIN) + new.add_to_hass(hass) + + result = await async_adopt_predecessor(hass, new, predecessor_domain=OLD_DOMAIN) + + assert not result.attempted + assert not result.predecessor_removed + + +# --------------------------------------------------------------------------- +# Settings - without them the adopted entities have nothing asking for them +# --------------------------------------------------------------------------- + + +async def test_settings_are_inherited(hass: HomeAssistant, entries) -> None: + """`SENSORS_TO_LOAD` is what makes the platform create an entity at all. + + Adopting the registry but not this leaves every entity showing "no longer provided" + until auto-discovery has seen a payload - and derived sensors, which are never in a + payload, only reappear on the next restart. The forwarding credentials would have + to be typed in again on top of that. + """ + old, new = entries + hass.config_entries.async_update_entry( + old, + options={ + SENSORS_TO_LOAD: ["outside_temp", "wind_dir"], + "WINDY_API_KEY": "windy-secret", + CHANNEL_TYPES: {"t234c1tp": "4"}, + }, + ) + + await async_adopt_predecessor(hass, new, predecessor_domain=OLD_DOMAIN) + + assert new.options[SENSORS_TO_LOAD] == ["outside_temp", "wind_dir"] + assert new.options["WINDY_API_KEY"] == "windy-secret" + assert new.options[CHANNEL_TYPES] == {"t234c1tp": "4"} + + +async def test_fresh_input_wins_over_inherited(hass: HomeAssistant, entries) -> None: + """What the user just typed into this integration's flow is newer by definition.""" + old, new = entries + hass.config_entries.async_update_entry(old, options={"API_ID": "old-id", "API_KEY": "old-key"}) + hass.config_entries.async_update_entry(new, options={"API_ID": "new-id"}) + + await async_adopt_predecessor(hass, new, predecessor_domain=OLD_DOMAIN) + + assert new.options["API_ID"] == "new-id", "the freshly entered value must not be overwritten" + assert new.options["API_KEY"] == "old-key", "gaps are still filled from the predecessor" + + +async def test_the_misspelled_pocasi_flag_is_translated(hass: HomeAssistant, entries) -> None: + """A version 1 entry carries the old spelling, and `async_migrate_entry` cannot help. + + Migration runs against this entry's own version, which is already current, so a key + copied in afterwards would never be looked at again - and Pocasi forwarding would + silently come up disabled. + """ + old, new = entries + hass.config_entries.async_update_entry(old, options={POCASI_CZ_ENABLED_LEGACY: True}) + + await async_adopt_predecessor(hass, new, predecessor_domain=OLD_DOMAIN) + + assert new.options[POCASI_CZ_ENABLED] is True + assert POCASI_CZ_ENABLED_LEGACY not in new.options + + +async def test_a_correct_flag_is_not_clobbered_by_the_misspelled_one(hass: HomeAssistant, entries) -> None: + """A partially upgraded entry can carry both; the correct spelling has to win.""" + old, new = entries + hass.config_entries.async_update_entry( + old, options={POCASI_CZ_ENABLED_LEGACY: True, POCASI_CZ_ENABLED: False} + ) + + await async_adopt_predecessor(hass, new, predecessor_domain=OLD_DOMAIN) + + assert new.options[POCASI_CZ_ENABLED] is False + + +async def test_a_failed_setup_leaves_the_predecessor_untouched(hass, entries, monkeypatch) -> None: + """The destructive step must come after setup is known to work. + + Removing the predecessor's config entry is the one thing adoption cannot undo. If + it ran before the webhook routes were up, a user who still has the previous version + installed - which is exactly what causes the route clash - would get their old entry + deleted and then a failed setup, with nothing to fall back to. + """ + from types import SimpleNamespace + from unittest.mock import AsyncMock + + from custom_components.sws12500 import async_setup_entry + from custom_components.sws12500.const import API_ID, API_KEY + from homeassistant.exceptions import ConfigEntryNotReady + + old, new = entries + before = _seed(hass, old) + hass.config_entries.async_update_entry(old, options={API_ID: "id", API_KEY: "key"}) + + class _BrokenRouter: + def add_get(self, *_a: Any, **_kw: Any) -> Any: + raise ValueError("Duplicate '_default_route', already handled by ...") + + def add_post(self, *_a: Any, **_kw: Any) -> Any: + raise ValueError("Duplicate '_wslink_post_route', already handled by ...") + + hass.http = SimpleNamespace(app=SimpleNamespace(router=_BrokenRouter())) + monkeypatch.setattr( + "custom_components.sws12500.HealthCoordinator.async_config_entry_first_refresh", + AsyncMock(return_value=None), + ) + monkeypatch.setattr("custom_components.sws12500.PREDECESSOR_DOMAIN", OLD_DOMAIN, raising=False) + monkeypatch.setattr("custom_components.sws12500.predecessor.PREDECESSOR_DOMAIN", OLD_DOMAIN) + + with pytest.raises(ConfigEntryNotReady): + await async_setup_entry(hass, new) + + assert hass.config_entries.async_get_entry(old.entry_id) is not None, ( + "the predecessor was removed even though setup failed" + ) + registry = er.async_get(hass) + for entity_id in before.values(): + entry = registry.async_get(entity_id) + assert entry is not None + assert entry.platform == OLD_DOMAIN + + # Settings are additive and safe to inherit early, so those may already be across. + assert new.options[API_ID] == "id" + + +async def test_adoption_runs_before_any_entity_is_created(hass, monkeypatch) -> None: + """Ordering inside `async_setup_entry`, and it fails silently if broken. + + Adoption has to finish before `async_forward_entry_setups`. Run the other way round, + our platforms register first, take the free entity_ids for themselves, and the + predecessor's entries are then left holding ids nobody reads - no error, no warning, + just every user's history detached. + """ + from types import SimpleNamespace + from unittest.mock import AsyncMock + + from custom_components.sws12500 import async_setup_entry + from custom_components.sws12500.const import API_ID, API_KEY, WSLINK + + class _Router: + def add_get(self, *_a: Any, **_kw: Any) -> Any: + return SimpleNamespace(method="GET") + + def add_post(self, *_a: Any, **_kw: Any) -> Any: + return SimpleNamespace(method="POST") + + hass.http = SimpleNamespace(app=SimpleNamespace(router=_Router())) + entry = MockConfigEntry(domain=DOMAIN, options={API_ID: "id", API_KEY: "key", WSLINK: False}) + entry.add_to_hass(hass) + + calls: list[str] = [] + monkeypatch.setattr( + "custom_components.sws12500.HealthCoordinator.async_config_entry_first_refresh", + AsyncMock(return_value=None), + ) + + async def _adopt(*_a: Any, **_kw: Any) -> Any: + calls.append("adopt") + return SimpleNamespace(adopted=[], live=[], conflicting=[], complete=True, attempted=False) + + async def _forward(*_a: Any, **_kw: Any) -> bool: + calls.append("forward") + return True + + monkeypatch.setattr("custom_components.sws12500.async_adopt_predecessor", _adopt) + monkeypatch.setattr(hass.config_entries, "async_forward_entry_setups", _forward) + + assert await async_setup_entry(hass, entry) is True + assert calls == ["adopt", "forward"], f"adoption must precede entity creation, got {calls}" + + +async def test_same_domain_short_circuits(hass: HomeAssistant) -> None: + """Until the rename actually happens, this must never touch anything.""" + entry = MockConfigEntry(domain=DOMAIN) + entry.add_to_hass(hass) + registry = er.async_get(hass) + ours = registry.async_get_or_create("sensor", DOMAIN, "outside_temp", config_entry=entry) + + result = await async_adopt_predecessor(hass, entry, predecessor_domain=DOMAIN) + + assert not result.attempted + assert registry.async_get(ours.entity_id) is not None + assert hass.config_entries.async_get_entry(entry.entry_id) is not None diff --git a/tests/test_sensor_platform.py b/tests/test_sensor_platform.py index b0d7cbd..9d33f05 100644 --- a/tests/test_sensor_platform.py +++ b/tests/test_sensor_platform.py @@ -214,6 +214,68 @@ def test_add_new_sensors_adds_known_keys(hass): assert entities_arg[0].entity_description.key == known_desc.key +def _descriptions_for(*keys: str) -> dict[str, Any]: + by_key = {desc.key: desc for desc in SENSOR_TYPES_WEATHER_API} + return {key: by_key[key] for key in keys} + + +def test_discovering_wind_dir_also_adds_the_azimut(hass): + """Derived sensors are computed, never sent, so discovery can only offer their inputs. + + Without expanding here they would appear only after the next restart, when platform + setup applies `_auto_enable_derived_sensors` for the first time. That is exactly what + left Azimut and Heat index sitting as "no longer provided" after a domain migration. + """ + # The caller writes the discovered keys into SENSORS_TO_LOAD before calling us. + entry, _coordinator, runtime = _make_entry(options={SENSORS_TO_LOAD: [WIND_DIR]}) + add_entities = MagicMock() + runtime.add_sensor_entities = add_entities + runtime.sensor_descriptions = _descriptions_for(WIND_DIR, WIND_AZIMUT) + + add_new_sensors(hass, entry, keys=[WIND_DIR]) + + (entities_arg,) = add_entities.call_args.args + assert {e.entity_description.key for e in entities_arg} == {WIND_DIR, WIND_AZIMUT} + + +def test_heat_index_appears_only_once_both_inputs_are_known(hass): + """It needs temperature *and* humidity, so the first of the two must not trigger it.""" + entry, _coordinator, runtime = _make_entry(options={SENSORS_TO_LOAD: [OUTSIDE_TEMP]}) + add_entities = MagicMock() + runtime.add_sensor_entities = add_entities + runtime.sensor_descriptions = _descriptions_for(OUTSIDE_TEMP, OUTSIDE_HUMIDITY, HEAT_INDEX) + + add_new_sensors(hass, entry, keys=[OUTSIDE_TEMP]) + (first,) = add_entities.call_args.args + assert {e.entity_description.key for e in first} == {OUTSIDE_TEMP} + + entry.options = {SENSORS_TO_LOAD: [OUTSIDE_TEMP, OUTSIDE_HUMIDITY]} + add_new_sensors(hass, entry, keys=[OUTSIDE_HUMIDITY]) + (second,) = add_entities.call_args.args + assert {e.entity_description.key for e in second} == {OUTSIDE_HUMIDITY, HEAT_INDEX} + + +def test_a_derived_sensor_is_never_added_twice(hass): + """A second entity with the same unique id would be rejected and logged as an error. + + Once the azimut exists, later discoveries must not offer it again - the wind + direction that unlocked it is still in SENSORS_TO_LOAD forever after. + """ + entry, _coordinator, runtime = _make_entry(options={SENSORS_TO_LOAD: [WIND_DIR]}) + add_entities = MagicMock() + runtime.add_sensor_entities = add_entities + runtime.sensor_descriptions = _descriptions_for(WIND_DIR, WIND_AZIMUT, OUTSIDE_TEMP) + + add_new_sensors(hass, entry, keys=[WIND_DIR]) + + # A later payload brings something unrelated; the azimut already exists. + entry.options = {SENSORS_TO_LOAD: [WIND_DIR, OUTSIDE_TEMP]} + add_new_sensors(hass, entry, keys=[OUTSIDE_TEMP]) + + (second,) = add_entities.call_args.args + assert {e.entity_description.key for e in second} == {OUTSIDE_TEMP} + + def test_add_new_sensors_noop_when_runtime_data_missing(): """add_new_sensors is a safe no-op when the entry is unloaded (no runtime_data).""" entry = SimpleNamespace(entry_id="x", options={}, runtime_data=None)