diff --git a/custom_components/inpost_air/__init__.py b/custom_components/inpost_air/__init__.py index b177f6a..8abacaa 100644 --- a/custom_components/inpost_air/__init__.py +++ b/custom_components/inpost_air/__init__.py @@ -9,10 +9,12 @@ from homeassistant.const import Platform from homeassistant.core import HomeAssistant from homeassistant.helpers import device_registry as dr +from homeassistant.helpers import entity_registry as er from custom_components.inpost_air.coordinator import InPostAirDataCoordinator from custom_components.inpost_air.models import ParcelLocker from custom_components.inpost_air.utils import get_device_info, get_parcel_locker_url +from custom_components.inpost_air.const import DOMAIN from .api import InPostAirPoint, InPostApi @@ -29,12 +31,12 @@ class InPostAirData: coordinator: InPostAirDataCoordinator -type InPostAirConfiEntry = ConfigEntry[InPostAirData] +type InPostAirConfigEntry = ConfigEntry[InPostAirData] PLATFORMS: list[Platform] = [Platform.SENSOR] -async def async_setup_entry(hass: HomeAssistant, entry: InPostAirConfiEntry) -> bool: +async def async_setup_entry(hass: HomeAssistant, entry: InPostAirConfigEntry) -> bool: """Set up InPost Air from a config entry.""" api_client = InPostApi(hass) entry_data = entry.data.get("parcel_locker") @@ -71,30 +73,75 @@ async def async_setup_entry(hass: HomeAssistant, entry: InPostAirConfiEntry) -> return True -async def async_unload_entry(hass: HomeAssistant, entry: InPostAirConfiEntry) -> bool: +async def async_unload_entry(hass: HomeAssistant, entry: InPostAirConfigEntry) -> bool: """Unload a config entry.""" return await hass.config_entries.async_unload_platforms(entry, PLATFORMS) -async def async_migrate_entry(hass: HomeAssistant, config_entry: InPostAirConfiEntry): - """Migrate old entry.""" - _LOGGER.debug( - "Migrating %s from version %s", config_entry.title, config_entry.version - ) +async def async_migrate_entry(hass: HomeAssistant, entry: InPostAirConfigEntry) -> bool: + """Remove legacy 3-tuple devices left by ≤ 1.4.x.""" - if config_entry.version > 2: + if entry.version > 2: # This means the user has downgraded from a future version return False - if config_entry.version == 1: - hass.config_entries.async_update_entry( - config_entry, - data={"parcel_locker": from_dict(InPostAirPoint, config_entry.data)}, - version=2, - ) - - _LOGGER.debug( - "Migrating %s to version %s completed", config_entry.title, config_entry.version - ) - + if entry.version == 1: # 1.5.0 → 1.5.1 + dev_reg = dr.async_get(hass) + ent_reg = er.async_get(hass) + + for device in list(dev_reg.devices.values()): + if entry.entry_id not in device.config_entries: + continue + + # Detect an old device with a single 3-element identifier + old_id = next( + (i for i in device.identifiers if len(i) == 3 and i[0] == DOMAIN), + None, + ) + if not old_id: + continue + + # Skip if it still has entities (unexpected edge-case) + device_entities = er.async_entries_for_device( + registry=ent_reg, device_id=device.id + ) + if device_entities: + _LOGGER.warning( + "Legacy device %s still has entities; skipping cleanup", device.id + ) + continue + + # Optionally carry over area/name/disabled flags to the new device + _, code, _ = old_id + new_dev = next( + ( + d + for d in dev_reg.devices.values() + if (DOMAIN, code) in d.identifiers + and entry.entry_id in d.config_entries + and d.id != device.id + ), + None, + ) + if new_dev: + # Convert string disabled_by to enum if needed + old_disabled_by = device.disabled_by + if isinstance(old_disabled_by, str): + old_disabled_by = dr.DeviceEntryDisabler(old_disabled_by) + + new_disabled_by = new_dev.disabled_by + if isinstance(new_disabled_by, str): + new_disabled_by = dr.DeviceEntryDisabler(new_disabled_by) + + dev_reg.async_update_device( + new_dev.id, + area_id=device.area_id or new_dev.area_id, + name_by_user=device.name_by_user or new_dev.name_by_user, + disabled_by=old_disabled_by or new_disabled_by, + ) + + # Remove the orphaned 3-tuple device + dev_reg.async_remove_device(device.id) + + entry.version = 2 # mark migration complete return True diff --git a/tests/test_migration.py b/tests/test_migration.py new file mode 100644 index 0000000..f666743 --- /dev/null +++ b/tests/test_migration.py @@ -0,0 +1,458 @@ +"""Tests for config entry migration.""" + +import pytest +from unittest.mock import Mock, patch +from homeassistant.config_entries import ConfigEntry +from homeassistant.core import HomeAssistant +from homeassistant.helpers import device_registry as dr + +from custom_components.inpost_air import async_migrate_entry +from custom_components.inpost_air.const import DOMAIN + + +@pytest.fixture +def mock_device_registry(): + """Create a mock device registry.""" + return Mock(spec=dr.DeviceRegistry) + + +@pytest.fixture +def mock_device(): + """Create a mock device.""" + device = Mock() + device.id = "test_device_id" + device.identifiers = set() + device.config_entries = set() + device.entities = [] + device.area_id = None + device.name_by_user = None + device.disabled_by = None + return device + + +@pytest.fixture +def mock_config_entry(): + """Create a mock config entry.""" + entry = Mock(spec=ConfigEntry) + entry.entry_id = "test_entry_id" + entry.version = 1 + return entry + + +async def test_migrate_entry_no_migration_needed_version_2( + hass: HomeAssistant, mock_config_entry, mock_device_registry +): + """Test migration when entry is already version 2.""" + mock_config_entry.version = 2 + + with patch( + "homeassistant.helpers.device_registry.async_get", + return_value=mock_device_registry, + ): + result = await async_migrate_entry(hass, mock_config_entry) + + assert result is True + mock_device_registry.async_update_device.assert_not_called() + mock_device_registry.async_remove_device.assert_not_called() + + +async def test_migrate_entry_no_devices( + hass: HomeAssistant, mock_config_entry, mock_device_registry +): + """Test migration when no devices exist.""" + mock_device_registry.devices = {} + + with patch( + "homeassistant.helpers.device_registry.async_get", + return_value=mock_device_registry, + ): + result = await async_migrate_entry(hass, mock_config_entry) + + assert result is True + assert mock_config_entry.version == 2 + mock_device_registry.async_update_device.assert_not_called() + mock_device_registry.async_remove_device.assert_not_called() + + +async def test_migrate_entry_device_not_in_config_entry( + hass: HomeAssistant, mock_config_entry, mock_device_registry, mock_device +): + """Test migration skips devices not belonging to this config entry.""" + mock_device.config_entries = {"other_entry_id"} + mock_device_registry.devices = {"device1": mock_device} + + with patch( + "homeassistant.helpers.device_registry.async_get", + return_value=mock_device_registry, + ): + result = await async_migrate_entry(hass, mock_config_entry) + + assert result is True + assert mock_config_entry.version == 2 + mock_device_registry.async_update_device.assert_not_called() + mock_device_registry.async_remove_device.assert_not_called() + + +async def test_migrate_entry_device_no_legacy_identifier( + hass: HomeAssistant, mock_config_entry, mock_device_registry, mock_device +): + """Test migration skips devices without legacy 3-tuple identifiers.""" + mock_device.config_entries = {mock_config_entry.entry_id} + mock_device.identifiers = { + (DOMAIN, "ABC123"), + (DOMAIN, "123456"), + } # Already new format + mock_device_registry.devices = {"device1": mock_device} + + with patch( + "homeassistant.helpers.device_registry.async_get", + return_value=mock_device_registry, + ): + result = await async_migrate_entry(hass, mock_config_entry) + + assert result is True + assert mock_config_entry.version == 2 + mock_device_registry.async_update_device.assert_not_called() + mock_device_registry.async_remove_device.assert_not_called() + + +async def test_migrate_entry_device_has_entities_warning( + hass: HomeAssistant, mock_config_entry, mock_device_registry, mock_device, caplog +): + """Test migration logs warning and skips devices that still have entities.""" + mock_device.config_entries = {mock_config_entry.entry_id} + mock_device.identifiers = {(DOMAIN, "ABC123", "123456")} # Legacy 3-tuple + mock_device.entities = ["entity1", "entity2"] # Has entities + mock_device_registry.devices = {"device1": mock_device} + + with ( + patch( + "homeassistant.helpers.device_registry.async_get", + return_value=mock_device_registry, + ), + patch( + "homeassistant.helpers.entity_registry.async_get", + return_value=Mock(), + ), + patch( + "homeassistant.helpers.entity_registry.async_entries_for_device", + return_value=["entity1", "entity2"], # Mock entities found + ), + ): + result = await async_migrate_entry(hass, mock_config_entry) + + assert result is True + assert mock_config_entry.version == 2 + assert ( + "Legacy device test_device_id still has entities; skipping cleanup" + in caplog.text + ) + mock_device_registry.async_update_device.assert_not_called() + mock_device_registry.async_remove_device.assert_not_called() + + +async def test_migrate_entry_remove_orphaned_device( + hass: HomeAssistant, mock_config_entry, mock_device_registry, mock_device +): + """Test migration removes orphaned legacy device when no new device exists.""" + mock_device.config_entries = {mock_config_entry.entry_id} + mock_device.identifiers = {(DOMAIN, "ABC123", "123456")} # Legacy 3-tuple + mock_device.entities = [] # No entities + mock_device_registry.devices = {"device1": mock_device} + + with patch( + "homeassistant.helpers.device_registry.async_get", + return_value=mock_device_registry, + ): + result = await async_migrate_entry(hass, mock_config_entry) + + assert result is True + assert mock_config_entry.version == 2 + mock_device_registry.async_remove_device.assert_called_once_with("test_device_id") + mock_device_registry.async_update_device.assert_not_called() + + +async def test_migrate_entry_transfer_settings_to_new_device( + hass: HomeAssistant, mock_config_entry, mock_device_registry +): + """Test migration transfers settings from old device to new device.""" + # Create old device with legacy identifier and custom settings + old_device = Mock() + old_device.id = "old_device_id" + old_device.config_entries = {mock_config_entry.entry_id} + old_device.identifiers = {(DOMAIN, "ABC123", "123456")} # Legacy 3-tuple + old_device.entities = [] + old_device.area_id = "living_room" + old_device.name_by_user = "My Custom Name" + old_device.disabled_by = None + + # Create new device with new identifier format but no custom settings + new_device = Mock() + new_device.id = "new_device_id" + new_device.config_entries = {mock_config_entry.entry_id} + new_device.identifiers = { + (DOMAIN, "ABC123"), + (DOMAIN, "123456"), + } # New 2-tuple format + new_device.area_id = None + new_device.name_by_user = None + new_device.disabled_by = None + + mock_device_registry.devices = {"old_device": old_device, "new_device": new_device} + + with patch( + "homeassistant.helpers.device_registry.async_get", + return_value=mock_device_registry, + ): + result = await async_migrate_entry(hass, mock_config_entry) + + assert result is True + assert mock_config_entry.version == 2 + + # Verify old device is removed + mock_device_registry.async_remove_device.assert_called_once_with("old_device_id") + + # Verify new device gets updated with old device's settings + mock_device_registry.async_update_device.assert_called_once_with( + "new_device_id", + area_id="living_room", + name_by_user="My Custom Name", + disabled_by=None, + ) + + +async def test_migrate_entry_prefer_old_device_settings( + hass: HomeAssistant, mock_config_entry, mock_device_registry +): + """Test migration prefers old device settings over new device when old has values.""" + # Create old device with legacy identifier and custom settings + old_device = Mock() + old_device.id = "old_device_id" + old_device.config_entries = {mock_config_entry.entry_id} + old_device.identifiers = {(DOMAIN, "ABC123", "123456")} # Legacy 3-tuple + old_device.entities = [] + old_device.area_id = "living_room" + old_device.name_by_user = "Old Custom Name" + old_device.disabled_by = "user" + + # Create new device with new identifier format and existing settings + new_device = Mock() + new_device.id = "new_device_id" + new_device.config_entries = {mock_config_entry.entry_id} + new_device.identifiers = { + (DOMAIN, "ABC123"), + (DOMAIN, "123456"), + } # New 2-tuple format + new_device.area_id = "kitchen" # Already has area + new_device.name_by_user = "New Custom Name" # Already has name + new_device.disabled_by = None # Already has disabled status + + mock_device_registry.devices = {"old_device": old_device, "new_device": new_device} + + with patch( + "homeassistant.helpers.device_registry.async_get", + return_value=mock_device_registry, + ): + result = await async_migrate_entry(hass, mock_config_entry) + + assert result is True + assert mock_config_entry.version == 2 + + # Verify old device is removed + mock_device_registry.async_remove_device.assert_called_once_with("old_device_id") + + # Verify new device gets old device's settings (since old device has values) + mock_device_registry.async_update_device.assert_called_once_with( + "new_device_id", + area_id="living_room", # Takes from old device + name_by_user="Old Custom Name", # Takes from old device + disabled_by="user", # Takes from old device + ) + + +async def test_migrate_entry_fallback_to_new_device_settings( + hass: HomeAssistant, mock_config_entry, mock_device_registry +): + """Test migration falls back to new device settings when old device has None values.""" + # Create old device with legacy identifier but no custom settings + old_device = Mock() + old_device.id = "old_device_id" + old_device.config_entries = {mock_config_entry.entry_id} + old_device.identifiers = {(DOMAIN, "ABC123", "123456")} # Legacy 3-tuple + old_device.entities = [] + old_device.area_id = None # No area + old_device.name_by_user = None # No custom name + old_device.disabled_by = None # Not disabled + + # Create new device with new identifier format and existing settings + new_device = Mock() + new_device.id = "new_device_id" + new_device.config_entries = {mock_config_entry.entry_id} + new_device.identifiers = { + (DOMAIN, "ABC123"), + (DOMAIN, "123456"), + } # New 2-tuple format + new_device.area_id = "kitchen" # Already has area + new_device.name_by_user = "New Custom Name" # Already has name + new_device.disabled_by = "user" # Already disabled + + mock_device_registry.devices = {"old_device": old_device, "new_device": new_device} + + with patch( + "homeassistant.helpers.device_registry.async_get", + return_value=mock_device_registry, + ): + result = await async_migrate_entry(hass, mock_config_entry) + + assert result is True + assert mock_config_entry.version == 2 + + # Verify old device is removed + mock_device_registry.async_remove_device.assert_called_once_with("old_device_id") + + # Verify new device keeps its existing settings (since old device has None values) + mock_device_registry.async_update_device.assert_called_once_with( + "new_device_id", + area_id="kitchen", # Falls back to new device + name_by_user="New Custom Name", # Falls back to new device + disabled_by="user", # Falls back to new device + ) + + +async def test_migrate_entry_multiple_legacy_devices( + hass: HomeAssistant, mock_config_entry, mock_device_registry +): + """Test migration handles multiple legacy devices.""" + # Create first old device + old_device1 = Mock() + old_device1.id = "old_device_1" + old_device1.config_entries = {mock_config_entry.entry_id} + old_device1.identifiers = {(DOMAIN, "ABC123", "123456")} + old_device1.entities = [] + old_device1.area_id = "living_room" + old_device1.name_by_user = None + old_device1.disabled_by = None + + # Create second old device + old_device2 = Mock() + old_device2.id = "old_device_2" + old_device2.config_entries = {mock_config_entry.entry_id} + old_device2.identifiers = {(DOMAIN, "XYZ789", "987654")} + old_device2.entities = [] + old_device2.area_id = None + old_device2.name_by_user = "Custom Device 2" + old_device2.disabled_by = "user" + + # Create corresponding new devices + new_device1 = Mock() + new_device1.id = "new_device_1" + new_device1.config_entries = {mock_config_entry.entry_id} + new_device1.identifiers = {(DOMAIN, "ABC123"), (DOMAIN, "123456")} + new_device1.area_id = None + new_device1.name_by_user = None + new_device1.disabled_by = None + + new_device2 = Mock() + new_device2.id = "new_device_2" + new_device2.config_entries = {mock_config_entry.entry_id} + new_device2.identifiers = {(DOMAIN, "XYZ789"), (DOMAIN, "987654")} + new_device2.area_id = None + new_device2.name_by_user = None + new_device2.disabled_by = None + + mock_device_registry.devices = { + "old_device_1": old_device1, + "old_device_2": old_device2, + "new_device_1": new_device1, + "new_device_2": new_device2, + } + + with patch( + "homeassistant.helpers.device_registry.async_get", + return_value=mock_device_registry, + ): + result = await async_migrate_entry(hass, mock_config_entry) + + assert result is True + assert mock_config_entry.version == 2 + + # Verify both old devices are removed + assert mock_device_registry.async_remove_device.call_count == 2 + mock_device_registry.async_remove_device.assert_any_call("old_device_1") + mock_device_registry.async_remove_device.assert_any_call("old_device_2") + + # Verify both new devices are updated + assert mock_device_registry.async_update_device.call_count == 2 + mock_device_registry.async_update_device.assert_any_call( + "new_device_1", + area_id="living_room", # Takes from old device + name_by_user=None, # Old device has None, so None is used + disabled_by=None, # Old device has None, so None is used + ) + mock_device_registry.async_update_device.assert_any_call( + "new_device_2", + area_id=None, # Old device has None, so None is used + name_by_user="Custom Device 2", # Takes from old device + disabled_by="user", # Takes from old device + ) + + +async def test_migrate_entry_mixed_identifier_formats( + hass: HomeAssistant, mock_config_entry, mock_device_registry +): + """Test migration ignores non-domain and non-3-tuple identifiers.""" + # Create device with mixed identifier formats + device = Mock() + device.id = "mixed_device_id" + device.config_entries = {mock_config_entry.entry_id} + device.identifiers = { + (DOMAIN, "ABC123", "123456"), # Legacy 3-tuple (should be processed) + (DOMAIN, "XYZ789"), # New 2-tuple (should be ignored) + ("other_domain", "test", "value"), # Different domain (should be ignored) + ("single_value",), # Single value (should be ignored) + } + device.entities = [] + device.area_id = None + device.name_by_user = None + device.disabled_by = None + + mock_device_registry.devices = {"device1": device} + + with patch( + "homeassistant.helpers.device_registry.async_get", + return_value=mock_device_registry, + ): + result = await async_migrate_entry(hass, mock_config_entry) + + assert result is True + assert mock_config_entry.version == 2 + + # Should remove the device since it has a legacy 3-tuple identifier + mock_device_registry.async_remove_device.assert_called_once_with("mixed_device_id") + + +async def test_migrate_entry_invalid_old_identifier_format( + hass: HomeAssistant, mock_config_entry, mock_device_registry +): + """Test migration handles edge case where 3-tuple doesn't unpack correctly.""" + # This shouldn't happen in practice, but we test the robustness + device = Mock() + device.id = "invalid_device_id" + device.config_entries = {mock_config_entry.entry_id} + device.identifiers = {(DOMAIN, "only_two_elements")} # Not a 3-tuple + device.entities = [] + + mock_device_registry.devices = {"device1": device} + + with patch( + "homeassistant.helpers.device_registry.async_get", + return_value=mock_device_registry, + ): + result = await async_migrate_entry(hass, mock_config_entry) + + assert result is True + assert mock_config_entry.version == 2 + + # Should not process this device since it doesn't have a 3-tuple + mock_device_registry.async_remove_device.assert_not_called() + mock_device_registry.async_update_device.assert_not_called() diff --git a/tests/test_migration_integration.py b/tests/test_migration_integration.py new file mode 100644 index 0000000..d0a683c --- /dev/null +++ b/tests/test_migration_integration.py @@ -0,0 +1,179 @@ +"""Integration tests for migration functionality.""" + +import pytest +from homeassistant.config_entries import ConfigEntry +from homeassistant.core import HomeAssistant +from homeassistant.helpers import device_registry as dr + +from custom_components.inpost_air import async_migrate_entry +from custom_components.inpost_air.const import DOMAIN + + +@pytest.mark.asyncio +async def test_migration_integration_end_to_end(hass: HomeAssistant): + """Test the complete migration process with real device registry.""" + + # Create a real device registry + dev_reg = dr.async_get(hass) + + # Create a mock config entry and add it to the config entry registry + entry = ConfigEntry( + version=1, + minor_version=1, + domain=DOMAIN, + title="Test Entry", + data={}, + options={}, + source="user", + entry_id="test_entry_id", + unique_id=None, + ) + + # Add the config entry to Home Assistant's registry + hass.config_entries._entries[entry.entry_id] = entry + + # Create a legacy device with 3-tuple identifier + old_device = dev_reg.async_get_or_create( + config_entry_id=entry.entry_id, + identifiers={(DOMAIN, "ABC123", "legacy_id")}, # Legacy 3-tuple + manufacturer="InPost", + name="Legacy Device", + ) + + # Set some custom properties on the old device + dev_reg.async_update_device( + old_device.id, + area_id="living_room", + name_by_user="My Custom Locker", + disabled_by=dr.DeviceEntryDisabler.USER, + ) + + # Create a new device with 2-tuple identifiers + new_device = dev_reg.async_get_or_create( + config_entry_id=entry.entry_id, + identifiers={(DOMAIN, "ABC123"), (DOMAIN, "new_id")}, # New 2-tuple format + manufacturer="InPost", + name="New Device", + ) + + # Verify initial state + assert len(dev_reg.devices) == 2 + old_device_fresh = dev_reg.async_get(old_device.id) + assert old_device_fresh.area_id == "living_room" + assert old_device_fresh.name_by_user == "My Custom Locker" + assert old_device_fresh.disabled_by == "user" + + # Run migration + result = await async_migrate_entry(hass, entry) + assert result is True + assert entry.version == 2 + + # Verify migration results + remaining_devices = dev_reg.devices + assert len(remaining_devices) == 1 # Old device should be removed + + # Verify old device is gone + assert dev_reg.async_get(old_device.id) is None + + # Verify new device still exists and has inherited settings + new_device_after = dev_reg.async_get(new_device.id) + assert new_device_after is not None + assert new_device_after.area_id == "living_room" # Inherited from old device + assert ( + new_device_after.name_by_user == "My Custom Locker" + ) # Inherited from old device + assert new_device_after.disabled_by == "user" # Inherited from old device + + +@pytest.mark.asyncio +async def test_migration_no_new_device_removes_old(hass: HomeAssistant): + """Test migration removes orphaned old device when no corresponding new device exists.""" + + # Create a real device registry + dev_reg = dr.async_get(hass) + + # Create a mock config entry and add it to the config entry registry + entry = ConfigEntry( + version=1, + minor_version=1, + domain=DOMAIN, + title="Test Entry", + data={}, + options={}, + source="user", + entry_id="test_entry_id", + unique_id=None, + ) + + # Add the config entry to Home Assistant's registry + hass.config_entries._entries[entry.entry_id] = entry + + # Create only a legacy device with 3-tuple identifier (no corresponding new device) + old_device = dev_reg.async_get_or_create( + config_entry_id=entry.entry_id, + identifiers={(DOMAIN, "ORPHAN123", "legacy_id")}, # Legacy 3-tuple + manufacturer="InPost", + name="Orphaned Device", + ) + + # Verify initial state + assert len(dev_reg.devices) == 1 + + # Run migration + result = await async_migrate_entry(hass, entry) + assert result is True + assert entry.version == 2 + + # Verify old device is removed + assert len(dev_reg.devices) == 0 + assert dev_reg.async_get(old_device.id) is None + + +@pytest.mark.asyncio +async def test_migration_idempotent(hass: HomeAssistant): + """Test that migration can be run multiple times safely.""" + + # Create a real device registry + dev_reg = dr.async_get(hass) + + # Create a mock config entry and add it to the config entry registry + entry = ConfigEntry( + version=1, + minor_version=1, + domain=DOMAIN, + title="Test Entry", + data={}, + options={}, + source="user", + entry_id="test_entry_id", + unique_id=None, + ) + + # Add the config entry to Home Assistant's registry + hass.config_entries._entries[entry.entry_id] = entry + + # Create only a new device (no legacy devices) + new_device = dev_reg.async_get_or_create( + config_entry_id=entry.entry_id, + identifiers={(DOMAIN, "ABC123"), (DOMAIN, "new_id")}, # New 2-tuple format + manufacturer="InPost", + name="New Device", + ) + + # Run migration first time + result1 = await async_migrate_entry(hass, entry) + assert result1 is True + assert entry.version == 2 + + # Verify device still exists + assert len(dev_reg.devices) == 1 + assert dev_reg.async_get(new_device.id) is not None + + # Run migration second time (should be safe) + result2 = await async_migrate_entry(hass, entry) + assert result2 is True + assert entry.version == 2 + + # Verify device still exists and nothing changed + assert len(dev_reg.devices) == 1 + assert dev_reg.async_get(new_device.id) is not None