From 16eb62b6b0cfd2a5f95bfda0ddc2f71ea6f13481 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Sarzy=C5=84ski?= <3097496+Permutoto@users.noreply.github.com> Date: Fri, 14 Aug 2026 15:19:21 +0200 Subject: [PATCH] Fix prefer internal URL for LOQED webhooks (#178667) --- homeassistant/components/loqed/coordinator.py | 44 ++++-- tests/components/loqed/test_init.py | 134 +++++++++++++++++- 2 files changed, 168 insertions(+), 10 deletions(-) diff --git a/homeassistant/components/loqed/coordinator.py b/homeassistant/components/loqed/coordinator.py index 9b2bd8c4a974..ce524cc551bc 100644 --- a/homeassistant/components/loqed/coordinator.py +++ b/homeassistant/components/loqed/coordinator.py @@ -133,7 +133,9 @@ class LoqedDataCoordinator(DataUpdateCoordinator[StatusMessage]): ) else: webhook_url = webhook.async_generate_url( - self.hass, self.config_entry.data[CONF_WEBHOOK_ID] + self.hass, + self.config_entry.data[CONF_WEBHOOK_ID], + prefer_external=False, ) _LOGGER.debug("Webhook URL: %s", webhook_url) @@ -151,20 +153,46 @@ class LoqedDataCoordinator(DataUpdateCoordinator[StatusMessage]): _LOGGER.debug("Webhook got index %s", webhook_index) + if webhook_index: + await self._remove_stale_webhooks(webhook_id, webhook_index, webhooks) + + async def _remove_stale_webhooks( + self, webhook_id: str, webhook_index: int, webhooks: list[dict] + ) -> None: + cloudhook_url = self.config_entry.data.get(CONF_CLOUDHOOK_URL) + + for existing_webhook in webhooks: + url = existing_webhook["url"] + index = existing_webhook["id"] + is_integration_webhook = url.endswith(f"/{webhook_id}") or ( + cloudhook_url and url == cloudhook_url + ) + + if is_integration_webhook and webhook_index != index: + _LOGGER.debug("Removing stale webhook with URL: %s", url) + try: + await self.lock.deleteWebhook(index) + except (TimeoutError, aiohttp.ClientError) as err: + _LOGGER.warning( + "Could not remove stale webhook from LOQED bridge: %s", err + ) + async def remove_webhooks(self) -> None: """Remove webhook from LOQED bridge.""" webhook_id = self.config_entry.data[CONF_WEBHOOK_ID] - if CONF_CLOUDHOOK_URL in self.config_entry.data: - webhook_url = self.config_entry.data[CONF_CLOUDHOOK_URL] - else: - webhook_url = webhook.async_generate_url(self.hass, webhook_id) - - _LOGGER.debug("Webhook URL: %s", webhook_url) - try: webhooks = await self.lock.getWebhooks() + if CONF_CLOUDHOOK_URL in self.config_entry.data: + webhook_url = self.config_entry.data[CONF_CLOUDHOOK_URL] + else: + webhook_url = next( + (x["url"] for x in webhooks if x["url"].endswith(f"/{webhook_id}")), + None, + ) + _LOGGER.debug("Webhook URL: %s", webhook_url) + webhook_index = next( (x["id"] for x in webhooks if x["url"] == webhook_url), None ) diff --git a/tests/components/loqed/test_init.py b/tests/components/loqed/test_init.py index b75fc478804a..39161fd911ac 100644 --- a/tests/components/loqed/test_init.py +++ b/tests/components/loqed/test_init.py @@ -3,7 +3,7 @@ from datetime import timedelta import json from typing import Any -from unittest.mock import AsyncMock, patch +from unittest.mock import AsyncMock, call, patch import aiohttp from freezegun.api import FrozenDateTimeFactory @@ -18,7 +18,12 @@ from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.helpers.network import get_url from homeassistant.setup import async_setup_component -from tests.common import MockConfigEntry, async_fire_time_changed, async_load_fixture +from tests.common import ( + MockConfigEntry, + async_fire_time_changed, + async_load_fixture, + async_load_json_object_fixture, +) from tests.typing import ClientSessionGenerator @@ -71,6 +76,131 @@ async def test_setup_webhook_in_bridge( lock.registerWebhook.assert_called_with(f"{get_url(hass)}/api/webhook/Webhook_id") +async def test_webhook_prefers_internal_url( + hass: HomeAssistant, config_entry: MockConfigEntry, lock: loqed.Lock +) -> None: + """Test webhook actually prefers internal url.""" + await hass.config.async_update( + internal_url="http://192.168.1.10:8123", + external_url="https://this-is-external-url.hass.nabu.casa", + ) + + config_entry.add_to_hass(hass) + + lock_status = await async_load_json_object_fixture(hass, "status_ok.json", DOMAIN) + + webhooks_fixture = json.loads( + await async_load_fixture(hass, "get_all_webhooks.json", DOMAIN) + ) + webhooks_fixture[0]["url"] = f"{hass.config.internal_url}/api/webhook/Webhook_id" + + lock.getWebhooks = AsyncMock(side_effect=[[], webhooks_fixture]) + + with ( + patch("loqedAPI.loqed.LoqedAPI.async_get_lock", return_value=lock), + patch( + "loqedAPI.loqed.LoqedAPI.async_get_lock_details", return_value=lock_status + ), + ): + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + lock.registerWebhook.assert_called_with( + f"{hass.config.internal_url}/api/webhook/Webhook_id" + ) + + +async def test_ensure_webhooks_removes_stale_webhooks( + hass: HomeAssistant, config_entry: MockConfigEntry, lock: loqed.Lock +) -> None: + """Test that stale webhooks with the same ID but different URL are removed.""" + await hass.config.async_update( + internal_url="http://192.168.1.10:8123", + external_url="https://this-is-external-url.hass.nabu.casa", + ) + + config_entry.add_to_hass(hass) + webhook_id = config_entry.data[CONF_WEBHOOK_ID] + + lock_status = await async_load_json_object_fixture(hass, "status_ok.json", DOMAIN) + + stale_webhook = { + "id": 14, + "url": f"192.168.14.233/api/webhook/{webhook_id}", + } + another_stale_webhook = { + "id": 4, + "url": f"{hass.config.external_url}/api/webhook/{webhook_id}", + } + new_webhook = { + "id": 15, + "url": f"{hass.config.internal_url}/api/webhook/{webhook_id}", + } + + lock.getWebhooks = AsyncMock( + side_effect=[ + [stale_webhook, another_stale_webhook], + [stale_webhook, another_stale_webhook, new_webhook], + ] + ) + + with ( + patch("loqedAPI.loqed.LoqedAPI.async_get_lock", return_value=lock), + patch( + "loqedAPI.loqed.LoqedAPI.async_get_lock_details", return_value=lock_status + ), + ): + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + lock.registerWebhook.assert_called_once() + assert lock.deleteWebhook.call_count == 2 + lock.deleteWebhook.assert_has_calls([call(14), call(4)], any_order=True) + + +async def test_ensure_webhooks_handles_bridge_error_on_cleanup( + hass: HomeAssistant, + config_entry: MockConfigEntry, + lock: loqed.Lock, + caplog: pytest.LogCaptureFixture, +) -> None: + """Test that stale webhook cleanup handles bridge connection errors smoothly.""" + await hass.config.async_update(internal_url="http://192.168.1.10:8123") + + config_entry.add_to_hass(hass) + webhook_id = config_entry.data[CONF_WEBHOOK_ID] + + lock_status = await async_load_json_object_fixture(hass, "status_ok.json", DOMAIN) + + stale_webhook = { + "id": 14, + "url": f"https://this-is-external-url.hass.nabu.casa/api/webhook/{webhook_id}", + } + new_webhook = { + "id": 15, + "url": f"{hass.config.internal_url}/api/webhook/{webhook_id}", + } + + lock.getWebhooks = AsyncMock( + side_effect=[[stale_webhook], [stale_webhook, new_webhook]] + ) + + lock.deleteWebhook = AsyncMock(side_effect=aiohttp.ClientError) + + with ( + patch("loqedAPI.loqed.LoqedAPI.async_get_lock", return_value=lock), + patch( + "loqedAPI.loqed.LoqedAPI.async_get_lock_details", return_value=lock_status + ), + ): + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + lock.deleteWebhook.assert_called_once_with(14) + + assert "Could not remove stale webhook from LOQED bridge" in caplog.text + + async def test_cannot_connect_to_bridge_will_retry( hass: HomeAssistant, config_entry: MockConfigEntry, lock: loqed.Lock ) -> None: