Rework CloudhookURL setup for mobile app (home-assistant#156940)

Author: TimoPtr

homeassistant/components/cloud/__init__.py

@@ -7,7 +7,7 @@
 from datetime import datetime, timedelta
 from enum import Enum
 import logging
-from typing import cast
+from typing import Any, cast
 
 from hass_nabucasa import Cloud
 import voluptuous as vol
@@ -86,6 +86,10 @@
     "CLOUD_CONNECTION_STATE"
 )
 
+_SIGNAL_CLOUDHOOKS_UPDATED: SignalType[dict[str, Any]] = SignalType(
+    "CLOUDHOOKS_UPDATED"
+)
+
 STARTUP_REPAIR_DELAY = 1  # 1 hour
 
 ALEXA_ENTITY_SCHEMA = vol.Schema(
@@ -242,6 +246,24 @@ async def async_delete_cloudhook(hass: HomeAssistant, webhook_id: str) -> None:
     await hass.data[DATA_CLOUD].cloudhooks.async_delete(webhook_id)
 
 
+@callback
+def async_listen_cloudhook_change(
+    hass: HomeAssistant,
+    webhook_id: str,
+    on_change: Callable[[dict[str, Any] | None], None],
+) -> Callable[[], None]:
+    """Listen for cloudhook changes for the given webhook and notify when modified or deleted."""
+
+    @callback
+    def _handle_cloudhooks_updated(cloudhooks: dict[str, Any]) -> None:
+        """Handle cloudhooks updated signal."""
+        on_change(cloudhooks.get(webhook_id))
+
+    return async_dispatcher_connect(
+        hass, _SIGNAL_CLOUDHOOKS_UPDATED, _handle_cloudhooks_updated
+    )
+
+
 @bind_hass
 @callback
 def async_remote_ui_url(hass: HomeAssistant) -> str:
@@ -289,7 +311,7 @@ async def _shutdown(event: Event) -> None:
 
     hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, _shutdown)
 
-    _remote_handle_prefs_updated(cloud)
+    _handle_prefs_updated(hass, cloud)
     _setup_services(hass, prefs)
 
     async def async_startup_repairs(_: datetime) -> None:
@@ -373,26 +395,32 @@ async def _on_initialized() -> None:
 
 
 @callback
-def _remote_handle_prefs_updated(cloud: Cloud[CloudClient]) -> None:
-    """Handle remote preferences updated."""
-    cur_pref = cloud.client.prefs.remote_enabled
+def _handle_prefs_updated(hass: HomeAssistant, cloud: Cloud[CloudClient]) -> None:
+    """Register handler for cloud preferences updates."""
+    cur_remote_enabled = cloud.client.prefs.remote_enabled
+    cur_cloudhooks = cloud.client.prefs.cloudhooks
     lock = asyncio.Lock()
 
-    # Sync remote connection with prefs
-    async def remote_prefs_updated(prefs: CloudPreferences) -> None:
-        """Update remote status."""
-        nonlocal cur_pref
+    async def on_prefs_updated(prefs: CloudPreferences) -> None:
+        """Handle cloud preferences updates."""
+        nonlocal cur_remote_enabled
+        nonlocal cur_cloudhooks
 
+        # Lock protects cur_ state variables from concurrent updates
         async with lock:
-            if prefs.remote_enabled == cur_pref:
+            if cur_cloudhooks != prefs.cloudhooks:
+                cur_cloudhooks = prefs.cloudhooks
+                async_dispatcher_send(hass, _SIGNAL_CLOUDHOOKS_UPDATED, cur_cloudhooks)
+
+            if prefs.remote_enabled == cur_remote_enabled:
                 return
 
-            if cur_pref := prefs.remote_enabled:
+            if cur_remote_enabled := prefs.remote_enabled:
                 await cloud.remote.connect()
             else:
                 await cloud.remote.disconnect()
 
-    cloud.client.prefs.async_listen_updates(remote_prefs_updated)
+    cloud.client.prefs.async_listen_updates(on_prefs_updated)
 
 
 async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:

homeassistant/components/mobile_app/__init__.py

@@ -131,12 +131,41 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     registration_name = f"Mobile App: {registration[ATTR_DEVICE_NAME]}"
     webhook_register(hass, DOMAIN, registration_name, webhook_id, handle_webhook)
 
+    def clean_cloudhook() -> None:
+        """Clean up cloudhook from config entry."""
+        if CONF_CLOUDHOOK_URL in entry.data:
+            data = dict(entry.data)
+            data.pop(CONF_CLOUDHOOK_URL)
+            hass.config_entries.async_update_entry(entry, data=data)
+
+    def on_cloudhook_change(cloudhook: dict[str, Any] | None) -> None:
+        """Handle cloudhook changes."""
+        if cloudhook:
+            if entry.data.get(CONF_CLOUDHOOK_URL) == cloudhook[CONF_CLOUDHOOK_URL]:
+                return
+
+            hass.config_entries.async_update_entry(
+                entry,
+                data={**entry.data, CONF_CLOUDHOOK_URL: cloudhook[CONF_CLOUDHOOK_URL]},
+            )
+        else:
+            clean_cloudhook()
+
     async def manage_cloudhook(state: cloud.CloudConnectionState) -> None:
         if (
             state is cloud.CloudConnectionState.CLOUD_CONNECTED
             and CONF_CLOUDHOOK_URL not in entry.data
         ):
             await async_create_cloud_hook(hass, webhook_id, entry)
+        elif (
+            state is cloud.CloudConnectionState.CLOUD_DISCONNECTED
+            and not cloud.async_is_logged_in(hass)
+        ):
+            clean_cloudhook()
+
+    entry.async_on_unload(
+        cloud.async_listen_cloudhook_change(hass, webhook_id, on_cloudhook_change)
+    )
 
     if cloud.async_is_logged_in(hass):
         if (
@@ -147,9 +176,7 @@ async def manage_cloudhook(state: cloud.CloudConnectionState) -> None:
             await async_create_cloud_hook(hass, webhook_id, entry)
     elif CONF_CLOUDHOOK_URL in entry.data:
         # If we have a cloudhook but no longer logged in to the cloud, remove it from the entry
-        data = dict(entry.data)
-        data.pop(CONF_CLOUDHOOK_URL)
-        hass.config_entries.async_update_entry(entry, data=data)
+        clean_cloudhook()
 
     entry.async_on_unload(cloud.async_listen_connection_change(hass, manage_cloudhook))
 

homeassistant/components/mobile_app/webhook.py

@@ -756,10 +756,9 @@ async def webhook_get_config(
         "theme_color": MANIFEST_JSON["theme_color"],
     }
 
-    if CONF_CLOUDHOOK_URL in config_entry.data:
-        resp[CONF_CLOUDHOOK_URL] = config_entry.data[CONF_CLOUDHOOK_URL]
-
     if cloud.async_active_subscription(hass):
+        if CONF_CLOUDHOOK_URL in config_entry.data:
+            resp[CONF_CLOUDHOOK_URL] = config_entry.data[CONF_CLOUDHOOK_URL]
         with suppress(cloud.CloudNotAvailable):
             resp[CONF_REMOTE_UI_URL] = cloud.async_remote_ui_url(hass)
 

tests/components/cloud/test_init.py

@@ -11,6 +11,7 @@
     CloudNotAvailable,
     CloudNotConnected,
     async_get_or_create_cloudhook,
+    async_listen_cloudhook_change,
     async_listen_connection_change,
     async_remote_ui_url,
 )
@@ -311,3 +312,149 @@ async def test_cloud_logout(
     await hass.async_block_till_done()
 
     assert cloud.is_logged_in is False
+
+
+async def test_async_listen_cloudhook_change(
+    hass: HomeAssistant,
+    cloud: MagicMock,
+    set_cloud_prefs: Callable[[dict[str, Any]], Coroutine[Any, Any, None]],
+) -> None:
+    """Test async_listen_cloudhook_change."""
+    assert await async_setup_component(hass, "cloud", {"cloud": {}})
+    await hass.async_block_till_done()
+    await cloud.login("test-user", "test-pass")
+
+    webhook_id = "mock-webhook-id"
+    cloudhook_url = "https://cloudhook.nabu.casa/abcdefg"
+
+    # Set up initial cloudhooks state
+    await set_cloud_prefs(
+        {
+            PREF_CLOUDHOOKS: {
+                webhook_id: {
+                    "webhook_id": webhook_id,
+                    "cloudhook_id": "random-id",
+                    "cloudhook_url": cloudhook_url,
+                    "managed": True,
+                }
+            }
+        }
+    )
+
+    # Track cloudhook changes
+    changes = []
+    changeInvoked = False
+
+    def on_change(cloudhook: dict[str, Any] | None) -> None:
+        """Handle cloudhook change."""
+        nonlocal changeInvoked
+        changes.append(cloudhook)
+        changeInvoked = True
+
+    # Register the change listener
+    unsubscribe = async_listen_cloudhook_change(hass, webhook_id, on_change)
+
+    # Verify no changes yet
+    assert len(changes) == 0
+    assert changeInvoked is False
+
+    # Delete the cloudhook by updating prefs
+    await set_cloud_prefs({PREF_CLOUDHOOKS: {}})
+    await hass.async_block_till_done()
+
+    # Verify deletion callback was called with None
+    assert len(changes) == 1
+    assert changes[-1] is None
+    assert changeInvoked is True
+
+    # Reset changeInvoked to detect next change
+    changeInvoked = False
+
+    # Add cloudhook back
+    cloudhook_data = {
+        "webhook_id": webhook_id,
+        "cloudhook_id": "random-id",
+        "cloudhook_url": cloudhook_url,
+        "managed": True,
+    }
+    await set_cloud_prefs({PREF_CLOUDHOOKS: {webhook_id: cloudhook_data}})
+    await hass.async_block_till_done()
+
+    # Verify callback called with cloudhook data
+    assert len(changes) == 2
+    assert changes[-1] == cloudhook_data
+    assert changeInvoked is True
+
+    # Reset changeInvoked to detect next change
+    changeInvoked = False
+
+    # Update cloudhook data with same cloudhook should not trigger callback
+    await set_cloud_prefs({PREF_CLOUDHOOKS: {webhook_id: cloudhook_data}})
+    await hass.async_block_till_done()
+
+    assert changeInvoked is False
+
+    # Unsubscribe from listener
+    unsubscribe()
+
+    # Delete cloudhook again
+    await set_cloud_prefs({PREF_CLOUDHOOKS: {}})
+    await hass.async_block_till_done()
+
+    # Verify change callback was NOT called after unsubscribe
+    assert len(changes) == 2
+    assert changeInvoked is False
+
+
+async def test_async_listen_cloudhook_change_cloud_setup_later(
+    hass: HomeAssistant,
+    cloud: MagicMock,
+    set_cloud_prefs: Callable[[dict[str, Any]], Coroutine[Any, Any, None]],
+) -> None:
+    """Test async_listen_cloudhook_change works when cloud is set up after listener registration."""
+    webhook_id = "mock-webhook-id"
+    cloudhook_url = "https://cloudhook.nabu.casa/abcdefg"
+
+    # Track cloudhook changes
+    changes: list[dict[str, Any] | None] = []
+
+    def on_change(cloudhook: dict[str, Any] | None) -> None:
+        """Handle cloudhook change."""
+        changes.append(cloudhook)
+
+    # Register listener BEFORE cloud is set up
+    unsubscribe = async_listen_cloudhook_change(hass, webhook_id, on_change)
+
+    # Verify it returns a callable
+    assert callable(unsubscribe)
+
+    # No changes yet since cloud isn't set up
+    assert len(changes) == 0
+
+    # Now set up cloud
+    assert await async_setup_component(hass, "cloud", {"cloud": {}})
+    await hass.async_block_till_done()
+    await cloud.login("test-user", "test-pass")
+
+    # Add a cloudhook - this should trigger the listener
+    cloudhook_data = {
+        "webhook_id": webhook_id,
+        "cloudhook_id": "random-id",
+        "cloudhook_url": cloudhook_url,
+        "managed": True,
+    }
+    await set_cloud_prefs({PREF_CLOUDHOOKS: {webhook_id: cloudhook_data}})
+    await hass.async_block_till_done()
+
+    # Verify the listener received the update
+    assert len(changes) == 1
+    assert changes[-1] == cloudhook_data
+
+    # Unsubscribe and verify no more updates
+    unsubscribe()
+
+    await set_cloud_prefs({PREF_CLOUDHOOKS: {}})
+    await hass.async_block_till_done()
+
+    # Should not receive update after unsubscribe
+    assert len(changes) == 1