From d3bf52c136f4803d09a033a6d7f551042b054225 Mon Sep 17 00:00:00 2001 From: Mike Heath Date: Mon, 19 Jun 2023 05:12:04 -0600 Subject: [PATCH] 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 * Assert HomeAssistantError when integration unloaded * Remove unused import * Set CONFIG_SCHEMA * Update homeassistant/components/fully_kiosk/__init__.py Co-authored-by: Martin Hjelmare * Add test for non fkb devices targets in service calls * Apply suggestions from code review --------- Co-authored-by: Martin Hjelmare --- .../components/fully_kiosk/__init__.py | 14 ++- .../components/fully_kiosk/services.py | 70 ++++++------ tests/components/fully_kiosk/test_services.py | 102 +++++++++++++++++- 3 files changed, 141 insertions(+), 45 deletions(-) diff --git a/homeassistant/components/fully_kiosk/__init__.py b/homeassistant/components/fully_kiosk/__init__.py index dd1cc70c9f4..8b350433858 100644 --- a/homeassistant/components/fully_kiosk/__init__.py +++ b/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 diff --git a/homeassistant/components/fully_kiosk/services.py b/homeassistant/components/fully_kiosk/services.py index 3fca9228735..b3c5886187a 100644 --- a/homeassistant/components/fully_kiosk/services.py +++ b/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( - call: ServiceCall, - fully_method: Callable, - *args: list[str], - **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 - ) + async def collect_coordinators( + device_ids: list[str], + ) -> list[FullyKioskDataUpdateCoordinator]: + config_entries = list[ConfigEntry]() 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: - entry = hass.config_entries.async_get_entry(key) - if not entry: - continue - if entry.domain != DOMAIN: - continue - coordinator = hass.data[DOMAIN][key] - # fully_method(coordinator.fully, *args, **kwargs) would make - # test_services.py fail. - await getattr(coordinator.fully, fully_method.__name__)( - *args, **kwargs + device_entries = list[ConfigEntry]() + for entry_id in device.config_entries: + entry = hass.config_entries.async_get_entry(entry_id) + if entry and entry.domain == DOMAIN: + device_entries.append(entry) + if not device_entries: + raise HomeAssistantError( + f"Device '{target}' is not a {DOMAIN} device" ) - 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( - call, FullyKiosk.startApplication, call.data[ATTR_APPLICATION] - ) + for coordinator in await collect_coordinators(call.data[ATTR_DEVICE_ID]): + await coordinator.fully.startApplication(call.data[ATTR_APPLICATION]) # Register all the above services service_mapping = [ diff --git a/tests/components/fully_kiosk/test_services.py b/tests/components/fully_kiosk/test_services.py index 386bc542e3c..504aa4893e6 100644 --- a/tests/components/fully_kiosk/test_services.py +++ b/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: "de.ozerov.fully"}, + {ATTR_DEVICE_ID: [device_entry.id], ATTR_APPLICATION: app}, blocking=True, ) - assert len(mock_fully_kiosk.startApplication.mock_calls) == 1 + 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() + + +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)