diff --git a/homeassistant/components/cloud/client.py b/homeassistant/components/cloud/client.py index cef3c5f0d42..8cf79d20c5d 100644 --- a/homeassistant/components/cloud/client.py +++ b/homeassistant/components/cloud/client.py @@ -29,6 +29,8 @@ from . import alexa_config, google_config from .const import DISPATCHER_REMOTE_UPDATE, DOMAIN from .prefs import CloudPreferences +_LOGGER = logging.getLogger(__name__) + VALID_REPAIR_TRANSLATION_KEYS = { "warn_bad_custom_domain_configuration", "reset_bad_custom_domain_configuration", @@ -149,6 +151,7 @@ class CloudClient(Interface): async def cloud_connected(self) -> None: """When cloud is connected.""" + _LOGGER.debug("cloud_connected") is_new_user = await self.prefs.async_set_username(self.cloud.username) async def enable_alexa(_: Any) -> None: @@ -196,6 +199,9 @@ class CloudClient(Interface): async def cloud_disconnected(self) -> None: """When cloud disconnected.""" + _LOGGER.debug("cloud_disconnected") + if self._google_config: + self._google_config.async_disable_local_sdk() async def cloud_started(self) -> None: """When cloud is started.""" @@ -207,6 +213,8 @@ class CloudClient(Interface): """Cleanup some stuff after logout.""" await self.prefs.async_set_username(None) + if self._google_config: + self._google_config.async_deinitialize() self._google_config = None @callback diff --git a/homeassistant/components/cloud/google_config.py b/homeassistant/components/cloud/google_config.py index b64ec558389..42f25f43ae1 100644 --- a/homeassistant/components/cloud/google_config.py +++ b/homeassistant/components/cloud/google_config.py @@ -23,6 +23,7 @@ from homeassistant.components.homeassistant.exposed_entities import ( from homeassistant.components.sensor import SensorDeviceClass from homeassistant.const import CLOUD_NEVER_EXPOSED_ENTITIES from homeassistant.core import ( + CALLBACK_TYPE, CoreState, Event, HomeAssistant, @@ -144,6 +145,7 @@ class CloudGoogleConfig(AbstractConfig): self._prefs = prefs self._cloud = cloud self._sync_entities_lock = asyncio.Lock() + self._on_deinitialize: list[CALLBACK_TYPE] = [] @property def enabled(self) -> bool: @@ -209,9 +211,11 @@ class CloudGoogleConfig(AbstractConfig): async def async_initialize(self) -> None: """Perform async initialization of config.""" + _LOGGER.debug("async_initialize") await super().async_initialize() async def on_hass_started(hass: HomeAssistant) -> None: + _LOGGER.debug("async_initialize on_hass_started") if self._prefs.google_settings_version != GOOGLE_SETTINGS_VERSION: _LOGGER.info( "Start migration of Google Assistant settings from v%s to v%s", @@ -238,16 +242,19 @@ class CloudGoogleConfig(AbstractConfig): await self._prefs.async_update( google_settings_version=GOOGLE_SETTINGS_VERSION ) - async_listen_entity_updates( - self.hass, CLOUD_GOOGLE, self._async_exposed_entities_updated + self._on_deinitialize.append( + async_listen_entity_updates( + self.hass, CLOUD_GOOGLE, self._async_exposed_entities_updated + ) ) async def on_hass_start(hass: HomeAssistant) -> None: + _LOGGER.debug("async_initialize on_hass_start") if self.enabled and GOOGLE_DOMAIN not in self.hass.config.components: await async_setup_component(self.hass, GOOGLE_DOMAIN, {}) - start.async_at_start(self.hass, on_hass_start) - start.async_at_started(self.hass, on_hass_started) + self._on_deinitialize.append(start.async_at_start(self.hass, on_hass_start)) + self._on_deinitialize.append(start.async_at_started(self.hass, on_hass_started)) # Remove any stored user agent id that is not ours remove_agent_user_ids = [] @@ -255,18 +262,33 @@ class CloudGoogleConfig(AbstractConfig): if agent_user_id != self.agent_user_id: remove_agent_user_ids.append(agent_user_id) + if remove_agent_user_ids: + _LOGGER.debug("remove non cloud agent_user_ids: %s", remove_agent_user_ids) for agent_user_id in remove_agent_user_ids: await self.async_disconnect_agent_user(agent_user_id) - self._prefs.async_listen_updates(self._async_prefs_updated) - self.hass.bus.async_listen( - er.EVENT_ENTITY_REGISTRY_UPDATED, - self._handle_entity_registry_updated, + self._on_deinitialize.append( + self._prefs.async_listen_updates(self._async_prefs_updated) ) - self.hass.bus.async_listen( - dr.EVENT_DEVICE_REGISTRY_UPDATED, - self._handle_device_registry_updated, + self._on_deinitialize.append( + self.hass.bus.async_listen( + er.EVENT_ENTITY_REGISTRY_UPDATED, + self._handle_entity_registry_updated, + ) ) + self._on_deinitialize.append( + self.hass.bus.async_listen( + dr.EVENT_DEVICE_REGISTRY_UPDATED, + self._handle_device_registry_updated, + ) + ) + + @callback + def async_deinitialize(self) -> None: + """Remove listeners.""" + _LOGGER.debug("async_deinitialize") + while self._on_deinitialize: + self._on_deinitialize.pop()() def should_expose(self, state: State) -> bool: """If a state object should be exposed.""" @@ -365,6 +387,7 @@ class CloudGoogleConfig(AbstractConfig): async def _async_prefs_updated(self, prefs: CloudPreferences) -> None: """Handle updated preferences.""" + _LOGGER.debug("_async_prefs_updated") if not self._cloud.is_logged_in: if self.is_reporting_state: self.async_disable_report_state() diff --git a/homeassistant/components/google_assistant/helpers.py b/homeassistant/components/google_assistant/helpers.py index c89925664e0..f3d0d24f7c8 100644 --- a/homeassistant/components/google_assistant/helpers.py +++ b/homeassistant/components/google_assistant/helpers.py @@ -316,6 +316,7 @@ class AbstractConfig(ABC): @callback def async_enable_local_sdk(self) -> None: """Enable the local SDK.""" + _LOGGER.debug("async_enable_local_sdk") setup_successful = True setup_webhook_ids = [] @@ -324,11 +325,16 @@ class AbstractConfig(ABC): self._local_sdk_active = False return - for user_agent_id, _ in self._store.agent_user_ids.items(): + for user_agent_id in self._store.agent_user_ids: if (webhook_id := self.get_local_webhook_id(user_agent_id)) is None: setup_successful = False break + _LOGGER.debug( + "Register webhook handler %s for agent user id %s", + webhook_id, + user_agent_id, + ) try: webhook.async_register( self.hass, @@ -360,13 +366,18 @@ class AbstractConfig(ABC): @callback def async_disable_local_sdk(self) -> None: """Disable the local SDK.""" + _LOGGER.debug("async_disable_local_sdk") if not self._local_sdk_active: return for agent_user_id in self._store.agent_user_ids: - webhook.async_unregister( - self.hass, self.get_local_webhook_id(agent_user_id) + webhook_id = self.get_local_webhook_id(agent_user_id) + _LOGGER.debug( + "Unregister webhook handler %s for agent user id %s", + webhook_id, + agent_user_id, ) + webhook.async_unregister(self.hass, webhook_id) self._local_sdk_active = False diff --git a/homeassistant/components/homeassistant/exposed_entities.py b/homeassistant/components/homeassistant/exposed_entities.py index b53f28f4cee..38c7f8e8128 100644 --- a/homeassistant/components/homeassistant/exposed_entities.py +++ b/homeassistant/components/homeassistant/exposed_entities.py @@ -12,7 +12,7 @@ from homeassistant.components import websocket_api from homeassistant.components.binary_sensor import BinarySensorDeviceClass from homeassistant.components.sensor import SensorDeviceClass from homeassistant.const import CLOUD_NEVER_EXPOSED_ENTITIES -from homeassistant.core import HomeAssistant, callback, split_entity_id +from homeassistant.core import CALLBACK_TYPE, HomeAssistant, callback, split_entity_id from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers import entity_registry as er from homeassistant.helpers.entity import get_device_class @@ -129,10 +129,17 @@ class ExposedEntities: @callback def async_listen_entity_updates( self, assistant: str, listener: Callable[[], None] - ) -> None: + ) -> CALLBACK_TYPE: """Listen for updates to entity expose settings.""" + + def unsubscribe() -> None: + """Stop listening to entity updates.""" + self._listeners[assistant].remove(listener) + self._listeners.setdefault(assistant, []).append(listener) + return unsubscribe + @callback def async_set_assistant_option( self, assistant: str, entity_id: str, key: str, value: Any @@ -484,10 +491,10 @@ def ws_expose_new_entities_set( @callback def async_listen_entity_updates( hass: HomeAssistant, assistant: str, listener: Callable[[], None] -) -> None: +) -> CALLBACK_TYPE: """Listen for updates to entity expose settings.""" exposed_entities: ExposedEntities = hass.data[DATA_EXPOSED_ENTITIES] - exposed_entities.async_listen_entity_updates(assistant, listener) + return exposed_entities.async_listen_entity_updates(assistant, listener) @callback diff --git a/tests/components/cloud/conftest.py b/tests/components/cloud/conftest.py index 7421914d3d4..798b169393a 100644 --- a/tests/components/cloud/conftest.py +++ b/tests/components/cloud/conftest.py @@ -115,6 +115,13 @@ async def cloud_fixture() -> AsyncGenerator[MagicMock, None]: type(mock_cloud).is_connected = is_connected type(mock_cloud.iot).connected = is_connected + def mock_username() -> bool: + """Return the subscription username.""" + return "abcdefghjkl" + + username = PropertyMock(side_effect=mock_username) + type(mock_cloud).username = username + # Properties that we mock as attributes. mock_cloud.expiration_date = utcnow() mock_cloud.subscription_expired = False diff --git a/tests/components/cloud/test_client.py b/tests/components/cloud/test_client.py index 0cd605fd755..0dfa682c07d 100644 --- a/tests/components/cloud/test_client.py +++ b/tests/components/cloud/test_client.py @@ -428,3 +428,47 @@ async def test_async_create_repair_issue_unknown( ) issue = issue_registry.async_get_issue(domain=DOMAIN, issue_id=identifier) assert issue is None + + +async def test_disconnected(hass: HomeAssistant) -> None: + """Test cleanup when disconnected from the cloud.""" + prefs = MagicMock( + alexa_enabled=False, + google_enabled=True, + async_set_username=AsyncMock(return_value=None), + ) + client = CloudClient(hass, prefs, None, {}, {}) + client.cloud = MagicMock(is_logged_in=True, username="mock-username") + client._google_config = Mock() + client._google_config.async_disable_local_sdk.assert_not_called() + + await client.cloud_disconnected() + client._google_config.async_disable_local_sdk.assert_called_once_with() + + +async def test_logged_out( + hass: HomeAssistant, + cloud: MagicMock, +) -> None: + """Test cleanup when logged out from the cloud.""" + + assert await async_setup_component(hass, "cloud", {"cloud": {}}) + await hass.async_block_till_done() + await cloud.login("test-user", "test-pass") + + alexa_config_mock = Mock(async_enable_proactive_mode=AsyncMock()) + google_config_mock = Mock(async_sync_entities=AsyncMock()) + cloud.client._alexa_config = alexa_config_mock + cloud.client._google_config = google_config_mock + + await cloud.client.cloud_connected() + await hass.async_block_till_done() + + # Simulate logged out + await cloud.logout() + await hass.async_block_till_done() + + # Alexa is not cleaned up, Google is + assert cloud.client._alexa_config is alexa_config_mock + assert cloud.client._google_config is None + google_config_mock.async_deinitialize.assert_called_once_with() diff --git a/tests/components/cloud/test_google_config.py b/tests/components/cloud/test_google_config.py index bedc6b459c5..7fb2d1aff3e 100644 --- a/tests/components/cloud/test_google_config.py +++ b/tests/components/cloud/test_google_config.py @@ -441,8 +441,10 @@ def test_enabled_requires_valid_sub( assert not config.enabled -async def test_setup_integration(hass: HomeAssistant, mock_conf, cloud_prefs) -> None: - """Test that we set up the integration if used.""" +async def test_setup_google_assistant( + hass: HomeAssistant, mock_conf, cloud_prefs +) -> None: + """Test that we set up the google_assistant integration if enabled in cloud.""" assert await async_setup_component(hass, "homeassistant", {}) mock_conf._cloud.subscription_expired = False @@ -473,8 +475,10 @@ async def test_google_handle_logout( "homeassistant.components.google_assistant.report_state.async_enable_report_state", ) as mock_enable: gconf.async_enable_report_state() + await hass.async_block_till_done() assert len(mock_enable.mock_calls) == 1 + assert len(gconf._on_deinitialize) == 6 # This will trigger a prefs update when we logout. await cloud_prefs.get_cloud_user() @@ -484,8 +488,13 @@ async def test_google_handle_logout( "async_check_token", side_effect=AssertionError("Should not be called"), ): + # Fake logging out; CloudClient.logout_cleanups sets username to None + # and deinitializes the Google config. await cloud_prefs.async_set_username(None) + gconf.async_deinitialize() await hass.async_block_till_done() + # Check listeners are removed: + assert not gconf._on_deinitialize assert len(mock_enable.return_value.mock_calls) == 1 diff --git a/tests/components/cloud/test_init.py b/tests/components/cloud/test_init.py index c537169bf01..4cef8c8437e 100644 --- a/tests/components/cloud/test_init.py +++ b/tests/components/cloud/test_init.py @@ -103,8 +103,8 @@ async def test_remote_services( assert mock_disconnect.called is False -async def test_startup_shutdown_events(hass: HomeAssistant, mock_cloud_fixture) -> None: - """Test if the cloud will start on startup event.""" +async def test_shutdown_event(hass: HomeAssistant, mock_cloud_fixture) -> None: + """Test if the cloud will stop on shutdown event.""" with patch("hass_nabucasa.Cloud.stop") as mock_stop: hass.bus.async_fire(EVENT_HOMEASSISTANT_STOP) await hass.async_block_till_done()