Register Fully Kiosk services regardless of setup result (#88647)

* Register services at integration level

If HA is unable to connect to Fully Kiosk, the services don't get
registered. This can cause repair to create notifications saying
that the 'fully_kiosk.load_url' service is unknown.

Fixes #85444

* Validate config entry is loaded

* Refactor service invocation

Raises `HomeAssistantError` when the user provides an device id that is
not in the device registry or a device that is not a Fully Kiosk
device. If the device's config entry is not loaded, a warning is
logged.

* Update homeassistant/components/fully_kiosk/services.py

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

* Assert HomeAssistantError when integration unloaded

* Remove unused import

* Set CONFIG_SCHEMA

* Update homeassistant/components/fully_kiosk/__init__.py

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

* Add test for non fkb devices targets in service calls

* Apply suggestions from code review

---------

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

homeassistant/components/fully_kiosk/__init__.py

@@ -2,6 +2,8 @@
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import Platform
 from homeassistant.core import HomeAssistant
+from homeassistant.helpers import config_validation as cv
+from homeassistant.helpers.typing import ConfigType
 
 from .const import DOMAIN
 from .coordinator import FullyKioskDataUpdateCoordinator
@@ -16,6 +18,16 @@ PLATFORMS = [
     Platform.SWITCH,
 ]
 
+CONFIG_SCHEMA = cv.config_entry_only_config_schema(DOMAIN)
+
+
+async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool:
+    """Set up Fully Kiosk Browser."""
+
+    await async_setup_services(hass)
+
+    return True
+
 
 async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     """Set up Fully Kiosk Browser from a config entry."""
@@ -28,8 +40,6 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS)
     coordinator.async_update_listeners()
 
-    await async_setup_services(hass)
-
     return True
 
 

homeassistant/components/fully_kiosk/services.py

@@ -1,14 +1,12 @@
 """Services for the Fully Kiosk Browser integration."""
 from __future__ import annotations
 
-from collections.abc import Callable
-from typing import Any
-
-from fullykiosk import FullyKiosk
 import voluptuous as vol
 
+from homeassistant.config_entries import ConfigEntry, ConfigEntryState
 from homeassistant.const import ATTR_DEVICE_ID
 from homeassistant.core import HomeAssistant, ServiceCall
+from homeassistant.exceptions import HomeAssistantError
 import homeassistant.helpers.config_validation as cv
 import homeassistant.helpers.device_registry as dr
 
@@ -16,59 +14,53 @@ from .const import (
     ATTR_APPLICATION,
     ATTR_URL,
     DOMAIN,
-    LOGGER,
     SERVICE_LOAD_URL,
     SERVICE_START_APPLICATION,
 )
+from .coordinator import FullyKioskDataUpdateCoordinator
 
 
 async def async_setup_services(hass: HomeAssistant) -> None:
     """Set up the services for the Fully Kiosk Browser integration."""
 
-    async def execute_service(
+    async def collect_coordinators(
-        call: ServiceCall,
+        device_ids: list[str],
-        fully_method: Callable,
+    ) -> list[FullyKioskDataUpdateCoordinator]:
-        *args: list[str],
+        config_entries = list[ConfigEntry]()
-        **kwargs: dict[str, Any],
-    ) -> None:
-        """Execute a Fully service call.
-
-        :param call: {ServiceCall} HA service call.
-        :param fully_method: {Callable} A method of the FullyKiosk class.
-        :param args: Arguments for fully_method.
-        :param kwargs: Key-word arguments for fully_method.
-        :return: None
-        """
-        LOGGER.debug(
-            "Calling Fully service %s with args: %s, %s", ServiceCall, args, kwargs
-        )
         registry = dr.async_get(hass)
-        for target in call.data[ATTR_DEVICE_ID]:
+        for target in device_ids:
             device = registry.async_get(target)
             if device:
-                for key in device.config_entries:
+                device_entries = list[ConfigEntry]()
-                    entry = hass.config_entries.async_get_entry(key)
+                for entry_id in device.config_entries:
-                    if not entry:
+                    entry = hass.config_entries.async_get_entry(entry_id)
-                        continue
+                    if entry and entry.domain == DOMAIN:
-                    if entry.domain != DOMAIN:
+                        device_entries.append(entry)
-                        continue
+                if not device_entries:
-                    coordinator = hass.data[DOMAIN][key]
+                    raise HomeAssistantError(
-                    # fully_method(coordinator.fully, *args, **kwargs) would make
+                        f"Device '{target}' is not a {DOMAIN} device"
-                    # test_services.py fail.
-                    await getattr(coordinator.fully, fully_method.__name__)(
-                        *args, **kwargs
                     )
-                    break
+                config_entries.extend(device_entries)
+            else:
+                raise HomeAssistantError(
+                    f"Device '{target}' not found in device registry"
+                )
+        coordinators = list[FullyKioskDataUpdateCoordinator]()
+        for config_entry in config_entries:
+            if config_entry.state != ConfigEntryState.LOADED:
+                raise HomeAssistantError(f"{config_entry.title} is not loaded")
+            coordinators.append(hass.data[DOMAIN][config_entry.entry_id])
+        return coordinators
 
     async def async_load_url(call: ServiceCall) -> None:
         """Load a URL on the Fully Kiosk Browser."""
-        await execute_service(call, FullyKiosk.loadUrl, call.data[ATTR_URL])
+        for coordinator in await collect_coordinators(call.data[ATTR_DEVICE_ID]):
+            await coordinator.fully.loadUrl(call.data[ATTR_URL])
 
     async def async_start_app(call: ServiceCall) -> None:
         """Start an app on the device."""
-        await execute_service(
+        for coordinator in await collect_coordinators(call.data[ATTR_DEVICE_ID]):
-            call, FullyKiosk.startApplication, call.data[ATTR_APPLICATION]
+            await coordinator.fully.startApplication(call.data[ATTR_APPLICATION])
-        )
 
     # Register all the above services
     service_mapping = [

tests/components/fully_kiosk/test_services.py

@@ -1,6 +1,8 @@
 """Test Fully Kiosk Browser services."""
 from unittest.mock import MagicMock
 
+import pytest
+
 from homeassistant.components.fully_kiosk.const import (
     ATTR_APPLICATION,
     ATTR_URL,
@@ -10,6 +12,7 @@ from homeassistant.components.fully_kiosk.const import (
 )
 from homeassistant.const import ATTR_DEVICE_ID
 from homeassistant.core import HomeAssistant
+from homeassistant.exceptions import HomeAssistantError
 from homeassistant.helpers import device_registry as dr
 
 from tests.common import MockConfigEntry
@@ -28,20 +31,111 @@ async def test_services(
 
     assert device_entry
 
+    url = "https://example.com"
     await hass.services.async_call(
         DOMAIN,
         SERVICE_LOAD_URL,
-        {ATTR_DEVICE_ID: [device_entry.id], ATTR_URL: "https://example.com"},
+        {ATTR_DEVICE_ID: [device_entry.id], ATTR_URL: url},
         blocking=True,
     )
 
-    assert len(mock_fully_kiosk.loadUrl.mock_calls) == 1
+    mock_fully_kiosk.loadUrl.assert_called_once_with(url)
 
+    app = "de.ozerov.fully"
+    await hass.services.async_call(
+        DOMAIN,
+        SERVICE_START_APPLICATION,
+        {ATTR_DEVICE_ID: [device_entry.id], ATTR_APPLICATION: app},
+        blocking=True,
+    )
+
+    mock_fully_kiosk.startApplication.assert_called_once_with(app)
+
+
+async def test_service_unloaded_entry(
+    hass: HomeAssistant,
+    mock_fully_kiosk: MagicMock,
+    init_integration: MockConfigEntry,
+) -> None:
+    """Test service not called when config entry unloaded."""
+    await init_integration.async_unload(hass)
+
+    device_registry = dr.async_get(hass)
+    device_entry = device_registry.async_get_device(
+        identifiers={(DOMAIN, "abcdef-123456")}
+    )
+
+    assert device_entry
+
+    with pytest.raises(HomeAssistantError) as excinfo:
+        await hass.services.async_call(
+            DOMAIN,
+            SERVICE_LOAD_URL,
+            {ATTR_DEVICE_ID: [device_entry.id], ATTR_URL: "https://nabucasa.com"},
+            blocking=True,
+        )
+    assert "Test device is not loaded" in str(excinfo)
+    mock_fully_kiosk.loadUrl.assert_not_called()
+
+    with pytest.raises(HomeAssistantError) as excinfo:
         await hass.services.async_call(
             DOMAIN,
             SERVICE_START_APPLICATION,
             {ATTR_DEVICE_ID: [device_entry.id], ATTR_APPLICATION: "de.ozerov.fully"},
             blocking=True,
         )
+    assert "Test device is not loaded" in str(excinfo)
+    mock_fully_kiosk.startApplication.assert_not_called()
 
-    assert len(mock_fully_kiosk.startApplication.mock_calls) == 1
+
+async def test_service_bad_device_id(
+    hass: HomeAssistant,
+    mock_fully_kiosk: MagicMock,
+    init_integration: MockConfigEntry,
+) -> None:
+    """Test Fully Kiosk Browser service invocation with bad device id."""
+    with pytest.raises(HomeAssistantError) as excinfo:
+        await hass.services.async_call(
+            DOMAIN,
+            SERVICE_LOAD_URL,
+            {ATTR_DEVICE_ID: ["bad-device_id"], ATTR_URL: "https://example.com"},
+            blocking=True,
+        )
+
+    assert "Device 'bad-device_id' not found in device registry" in str(excinfo)
+
+
+async def test_service_called_with_non_fkb_target_devices(
+    hass: HomeAssistant,
+    mock_fully_kiosk: MagicMock,
+    init_integration: MockConfigEntry,
+) -> None:
+    """Services raise exception when no valid devices provided."""
+    device_registry = dr.async_get(hass)
+
+    other_domain = "NotFullyKiosk"
+    other_config_id = "555"
+    await hass.config_entries.async_add(
+        MockConfigEntry(
+            title="Not Fully Kiosk", domain=other_domain, entry_id=other_config_id
+        )
+    )
+    device_entry = device_registry.async_get_or_create(
+        config_entry_id=other_config_id,
+        identifiers={
+            (other_domain, 1),
+        },
+    )
+
+    with pytest.raises(HomeAssistantError) as excinfo:
+        await hass.services.async_call(
+            DOMAIN,
+            SERVICE_LOAD_URL,
+            {
+                ATTR_DEVICE_ID: [device_entry.id],
+                ATTR_URL: "https://example.com",
+            },
+            blocking=True,
+        )
+
+    assert f"Device '{device_entry.id}' is not a fully_kiosk device" in str(excinfo)