From f38ca7b04af2554f044eefa6a156cd62e8bfc481 Mon Sep 17 00:00:00 2001 From: David Bishop Date: Mon, 9 Mar 2026 11:35:34 -0700 Subject: [PATCH] Add unique_id to Whisker (Litter-Robot) config entries (#164766) Co-authored-by: Claude Opus 4.6 Co-authored-by: Joostlek --- .../components/litterrobot/__init__.py | 53 +++++++- .../components/litterrobot/config_flow.py | 10 +- .../components/litterrobot/strings.json | 3 +- tests/components/litterrobot/common.py | 1 + tests/components/litterrobot/conftest.py | 5 + .../litterrobot/test_config_flow.py | 60 ++++++++- tests/components/litterrobot/test_init.py | 117 +++++++++++++++++- 7 files changed, 241 insertions(+), 8 deletions(-) diff --git a/homeassistant/components/litterrobot/__init__.py b/homeassistant/components/litterrobot/__init__.py index 14902a57aa5..1a9fda45c28 100644 --- a/homeassistant/components/litterrobot/__init__.py +++ b/homeassistant/components/litterrobot/__init__.py @@ -3,10 +3,15 @@ from __future__ import annotations import itertools +import logging -from homeassistant.const import Platform +from pylitterbot import Account +from pylitterbot.exceptions import LitterRobotException + +from homeassistant.const import CONF_PASSWORD, CONF_USERNAME, Platform from homeassistant.core import HomeAssistant from homeassistant.helpers import config_validation as cv +from homeassistant.helpers.aiohttp_client import async_get_clientsession from homeassistant.helpers.device_registry import DeviceEntry from homeassistant.helpers.typing import ConfigType @@ -14,6 +19,8 @@ from .const import DOMAIN from .coordinator import LitterRobotConfigEntry, LitterRobotDataUpdateCoordinator from .services import async_setup_services +_LOGGER = logging.getLogger(__name__) + CONFIG_SCHEMA = cv.config_entry_only_config_schema(DOMAIN) PLATFORMS = [ Platform.BINARY_SENSOR, @@ -33,6 +40,50 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: return True +async def async_migrate_entry( + hass: HomeAssistant, entry: LitterRobotConfigEntry +) -> bool: + """Migrate old entry.""" + _LOGGER.debug( + "Migrating configuration from version %s.%s", + entry.version, + entry.minor_version, + ) + + if entry.version > 1: + return False + + if entry.minor_version < 2: + account = Account(websession=async_get_clientsession(hass)) + try: + await account.connect( + username=entry.data[CONF_USERNAME], + password=entry.data[CONF_PASSWORD], + ) + user_id = account.user_id + except LitterRobotException: + _LOGGER.debug("Could not connect to set unique_id during migration") + return False + finally: + await account.disconnect() + + if user_id and not hass.config_entries.async_entry_for_domain_unique_id( + DOMAIN, user_id + ): + hass.config_entries.async_update_entry( + entry, unique_id=user_id, minor_version=2 + ) + else: + hass.config_entries.async_update_entry(entry, minor_version=2) + + _LOGGER.debug( + "Migration to configuration version %s.%s successful", + entry.version, + entry.minor_version, + ) + return True + + async def async_setup_entry(hass: HomeAssistant, entry: LitterRobotConfigEntry) -> bool: """Set up Litter-Robot from a config entry.""" coordinator = LitterRobotDataUpdateCoordinator(hass, entry) diff --git a/homeassistant/components/litterrobot/config_flow.py b/homeassistant/components/litterrobot/config_flow.py index 90f1fcba56d..149142ab7fe 100644 --- a/homeassistant/components/litterrobot/config_flow.py +++ b/homeassistant/components/litterrobot/config_flow.py @@ -27,8 +27,10 @@ class LitterRobotConfigFlow(ConfigFlow, domain=DOMAIN): """Handle a config flow for Litter-Robot.""" VERSION = 1 + MINOR_VERSION = 2 username: str + _account_user_id: str | None = None async def async_step_reauth( self, entry_data: Mapping[str, Any] @@ -45,6 +47,8 @@ class LitterRobotConfigFlow(ConfigFlow, domain=DOMAIN): if user_input: user_input = user_input | {CONF_USERNAME: self.username} if not (error := await self._async_validate_input(user_input)): + await self.async_set_unique_id(self._account_user_id) + self._abort_if_unique_id_mismatch() return self.async_update_reload_and_abort( self._get_reauth_entry(), data_updates=user_input ) @@ -65,8 +69,9 @@ class LitterRobotConfigFlow(ConfigFlow, domain=DOMAIN): if user_input is not None: self._async_abort_entries_match({CONF_USERNAME: user_input[CONF_USERNAME]}) - if not (error := await self._async_validate_input(user_input)): + await self.async_set_unique_id(self._account_user_id) + self._abort_if_unique_id_configured() return self.async_create_entry( title=user_input[CONF_USERNAME], data=user_input ) @@ -92,4 +97,7 @@ class LitterRobotConfigFlow(ConfigFlow, domain=DOMAIN): except Exception: _LOGGER.exception("Unexpected exception") return "unknown" + self._account_user_id = account.user_id + if not self._account_user_id: + return "unknown" return "" diff --git a/homeassistant/components/litterrobot/strings.json b/homeassistant/components/litterrobot/strings.json index f9e99b52b42..1ebe8490976 100644 --- a/homeassistant/components/litterrobot/strings.json +++ b/homeassistant/components/litterrobot/strings.json @@ -2,7 +2,8 @@ "config": { "abort": { "already_configured": "[%key:common::config_flow::abort::already_configured_account%]", - "reauth_successful": "[%key:common::config_flow::abort::reauth_successful%]" + "reauth_successful": "[%key:common::config_flow::abort::reauth_successful%]", + "unique_id_mismatch": "The Whisker account does not match the previously configured account. Please re-authenticate using the same account, or remove this integration and set it up again if you want to use a different account." }, "error": { "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]", diff --git a/tests/components/litterrobot/common.py b/tests/components/litterrobot/common.py index a86c782a2eb..b9780729d58 100644 --- a/tests/components/litterrobot/common.py +++ b/tests/components/litterrobot/common.py @@ -6,6 +6,7 @@ from homeassistant.const import CONF_PASSWORD, CONF_USERNAME BASE_PATH = "homeassistant.components.litterrobot" CONFIG = {DOMAIN: {CONF_USERNAME: "user@example.com", CONF_PASSWORD: "password"}} +ACCOUNT_USER_ID = "1234567" ROBOT_NAME = "Test" ROBOT_SERIAL = "LR3C012345" ROBOT_DATA = { diff --git a/tests/components/litterrobot/conftest.py b/tests/components/litterrobot/conftest.py index f13d0f82d2b..af13c96a71c 100644 --- a/tests/components/litterrobot/conftest.py +++ b/tests/components/litterrobot/conftest.py @@ -13,6 +13,7 @@ import pytest from homeassistant.core import HomeAssistant from .common import ( + ACCOUNT_USER_ID, CONFIG, DOMAIN, FEEDER_ROBOT_DATA, @@ -79,6 +80,7 @@ def create_mock_account( account = MagicMock(spec=Account) account.connect = AsyncMock() account.refresh_robots = AsyncMock() + account.user_id = ACCOUNT_USER_ID account.robots = ( [] if skip_robots @@ -163,6 +165,9 @@ async def setup_integration( entry = MockConfigEntry( domain=DOMAIN, data=CONFIG[DOMAIN], + unique_id=ACCOUNT_USER_ID, + version=1, + minor_version=2, ) entry.add_to_hass(hass) diff --git a/tests/components/litterrobot/test_config_flow.py b/tests/components/litterrobot/test_config_flow.py index caaf832b780..69527c87760 100644 --- a/tests/components/litterrobot/test_config_flow.py +++ b/tests/components/litterrobot/test_config_flow.py @@ -1,6 +1,6 @@ """Test the Litter-Robot config flow.""" -from unittest.mock import patch +from unittest.mock import PropertyMock, patch from pylitterbot import Account from pylitterbot.exceptions import LitterRobotException, LitterRobotLoginException @@ -11,7 +11,7 @@ from homeassistant.const import CONF_PASSWORD from homeassistant.core import HomeAssistant from homeassistant.data_entry_flow import FlowResultType -from .common import CONF_USERNAME, CONFIG, DOMAIN +from .common import ACCOUNT_USER_ID, CONF_USERNAME, CONFIG, DOMAIN from tests.common import MockConfigEntry @@ -29,6 +29,11 @@ async def test_full_flow(hass: HomeAssistant, mock_account) -> None: "homeassistant.components.litterrobot.config_flow.Account.connect", return_value=mock_account, ), + patch( + "homeassistant.components.litterrobot.config_flow.Account.user_id", + new_callable=PropertyMock, + return_value=ACCOUNT_USER_ID, + ), patch( "homeassistant.components.litterrobot.async_setup_entry", return_value=True, @@ -41,14 +46,16 @@ async def test_full_flow(hass: HomeAssistant, mock_account) -> None: assert result["type"] is FlowResultType.CREATE_ENTRY assert result["title"] == CONFIG[DOMAIN][CONF_USERNAME] assert result["data"] == CONFIG[DOMAIN] + assert result["result"].unique_id == ACCOUNT_USER_ID assert len(mock_setup_entry.mock_calls) == 1 async def test_already_configured(hass: HomeAssistant) -> None: - """Test already configured case.""" + """Test already configured account is rejected before authentication.""" MockConfigEntry( domain=DOMAIN, data=CONFIG[DOMAIN], + unique_id=ACCOUNT_USER_ID, ).add_to_hass(hass) result = await hass.config_entries.flow.async_init( @@ -72,7 +79,7 @@ async def test_already_configured(hass: HomeAssistant) -> None: async def test_create_entry( hass: HomeAssistant, mock_account, side_effect, connect_errors ) -> None: - """Test creating an entry.""" + """Test creating an entry after error recovery.""" result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_USER} ) @@ -93,6 +100,11 @@ async def test_create_entry( "homeassistant.components.litterrobot.config_flow.Account.connect", return_value=mock_account, ), + patch( + "homeassistant.components.litterrobot.config_flow.Account.user_id", + new_callable=PropertyMock, + return_value=ACCOUNT_USER_ID, + ), patch( "homeassistant.components.litterrobot.async_setup_entry", return_value=True, @@ -105,6 +117,7 @@ async def test_create_entry( assert result["type"] is FlowResultType.CREATE_ENTRY assert result["title"] == CONFIG[DOMAIN][CONF_USERNAME] assert result["data"] == CONFIG[DOMAIN] + assert result["result"].unique_id == ACCOUNT_USER_ID async def test_reauth(hass: HomeAssistant, mock_account: Account) -> None: @@ -112,6 +125,7 @@ async def test_reauth(hass: HomeAssistant, mock_account: Account) -> None: entry = MockConfigEntry( domain=DOMAIN, data=CONFIG[DOMAIN], + unique_id=ACCOUNT_USER_ID, ) entry.add_to_hass(hass) @@ -137,6 +151,11 @@ async def test_reauth(hass: HomeAssistant, mock_account: Account) -> None: "homeassistant.components.litterrobot.config_flow.Account.connect", return_value=mock_account, ), + patch( + "homeassistant.components.litterrobot.config_flow.Account.user_id", + new_callable=PropertyMock, + return_value=ACCOUNT_USER_ID, + ), patch( "homeassistant.components.litterrobot.async_setup_entry", return_value=True, @@ -148,4 +167,37 @@ async def test_reauth(hass: HomeAssistant, mock_account: Account) -> None: ) assert result["type"] is FlowResultType.ABORT assert result["reason"] == "reauth_successful" + assert entry.unique_id == ACCOUNT_USER_ID assert len(mock_setup_entry.mock_calls) == 1 + + +async def test_reauth_wrong_account(hass: HomeAssistant) -> None: + """Test reauth flow aborts when credentials belong to a different account.""" + entry = MockConfigEntry( + domain=DOMAIN, + data=CONFIG[DOMAIN], + unique_id=ACCOUNT_USER_ID, + ) + entry.add_to_hass(hass) + + result = await entry.start_reauth_flow(hass) + + with ( + patch( + "homeassistant.components.litterrobot.config_flow.Account.connect", + ), + patch( + "homeassistant.components.litterrobot.config_flow.Account.user_id", + new_callable=PropertyMock, + return_value="different_user_id", + ), + ): + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + user_input={CONF_PASSWORD: CONFIG[DOMAIN][CONF_PASSWORD]}, + ) + + assert result["type"] is FlowResultType.ABORT + assert result["reason"] == "unique_id_mismatch" + assert entry.unique_id == ACCOUNT_USER_ID + assert entry.data == CONFIG[DOMAIN] diff --git a/tests/components/litterrobot/test_init.py b/tests/components/litterrobot/test_init.py index 9ba4acaa935..c632baf1a7b 100644 --- a/tests/components/litterrobot/test_init.py +++ b/tests/components/litterrobot/test_init.py @@ -16,7 +16,7 @@ from homeassistant.core import HomeAssistant from homeassistant.helpers import device_registry as dr, entity_registry as er from homeassistant.setup import async_setup_component -from .common import CONFIG, DOMAIN, VACUUM_ENTITY_ID +from .common import ACCOUNT_USER_ID, CONFIG, DOMAIN, VACUUM_ENTITY_ID from .conftest import setup_integration from tests.common import MockConfigEntry @@ -58,6 +58,9 @@ async def test_entry_not_setup( entry = MockConfigEntry( domain=DOMAIN, data=CONFIG[DOMAIN], + unique_id=ACCOUNT_USER_ID, + version=1, + minor_version=2, ) entry.add_to_hass(hass) @@ -69,6 +72,118 @@ async def test_entry_not_setup( assert entry.state is expected_state +async def test_unique_id_migration( + hass: HomeAssistant, mock_account: MagicMock +) -> None: + """Test that legacy entries get unique_id set during migration.""" + entry = MockConfigEntry( + domain=DOMAIN, + data=CONFIG[DOMAIN], + version=1, + minor_version=1, + ) + assert entry.unique_id is None + entry.add_to_hass(hass) + + with ( + patch( + "homeassistant.components.litterrobot.Account", + return_value=mock_account, + ), + patch( + "homeassistant.components.litterrobot.coordinator.Account", + return_value=mock_account, + ), + ): + await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + assert entry.unique_id == ACCOUNT_USER_ID + assert entry.minor_version == 2 + mock_account.disconnect.assert_called_once() + + +async def test_unique_id_migration_unsupported_version( + hass: HomeAssistant, +) -> None: + """Test that migration fails for entries with version > 1.""" + entry = MockConfigEntry( + domain=DOMAIN, + data=CONFIG[DOMAIN], + unique_id=ACCOUNT_USER_ID, + version=2, + minor_version=1, + ) + entry.add_to_hass(hass) + + await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + assert entry.state is ConfigEntryState.MIGRATION_ERROR + + +async def test_unique_id_migration_conflict( + hass: HomeAssistant, mock_account: MagicMock +) -> None: + """Test that migration skips unique_id when another entry owns it.""" + # First entry already has the unique_id + MockConfigEntry( + domain=DOMAIN, + data=CONFIG[DOMAIN], + unique_id=ACCOUNT_USER_ID, + ).add_to_hass(hass) + + # Second entry is legacy (no unique_id) + entry = MockConfigEntry( + domain=DOMAIN, + data=CONFIG[DOMAIN], + version=1, + minor_version=1, + ) + assert entry.unique_id is None + entry.add_to_hass(hass) + + with ( + patch( + "homeassistant.components.litterrobot.Account", + return_value=mock_account, + ), + patch( + "homeassistant.components.litterrobot.coordinator.Account", + return_value=mock_account, + ), + ): + await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + assert entry.unique_id is None + assert entry.minor_version == 2 + + +async def test_unique_id_migration_connection_failure( + hass: HomeAssistant, +) -> None: + """Test that migration fails when the API is unreachable during unique_id backfill.""" + entry = MockConfigEntry( + domain=DOMAIN, + data=CONFIG[DOMAIN], + version=1, + minor_version=1, + ) + entry.add_to_hass(hass) + + with patch( + "homeassistant.components.litterrobot.Account.connect", + side_effect=LitterRobotException, + ): + await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + assert entry.unique_id is None + assert entry.minor_version == 1 + assert entry.state is ConfigEntryState.MIGRATION_ERROR + + async def test_device_remove_devices( hass: HomeAssistant, hass_ws_client: WebSocketGenerator,