Improve deCONZ services code quality (#56904)

* setup and unload services does not need to be async

* Only use DECONZ_DOMAIN to decide if service should be setup

* Consolidation of functionality

* Make a service to schema dictionary

homeassistant/components/deconz/__init__.py

@@ -32,12 +32,13 @@ async def async_setup_entry(hass, config_entry):
     if not await gateway.async_setup():
         return False
 
+    if not hass.data[DOMAIN]:
+        async_setup_services(hass)
+
     hass.data[DOMAIN][config_entry.entry_id] = gateway
 
     await gateway.async_update_device_registry()
 
-    await async_setup_services(hass)
-
     config_entry.async_on_unload(
         hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, gateway.shutdown)
     )
@@ -50,7 +51,7 @@ async def async_unload_entry(hass, config_entry):
     gateway = hass.data[DOMAIN].pop(config_entry.entry_id)
 
     if not hass.data[DOMAIN]:
-        await async_unload_services(hass)
+        async_unload_services(hass)
 
     elif gateway.master:
         await async_update_master_gateway(hass, config_entry)

homeassistant/components/deconz/services.py

@@ -5,6 +5,7 @@ import asyncio
 from pydeconz.utils import normalize_bridge_id
 import voluptuous as vol
 
+from homeassistant.core import callback
 from homeassistant.helpers import config_validation as cv
 from homeassistant.helpers.device_registry import CONNECTION_NETWORK_MAC
 from homeassistant.helpers.entity_registry import (
@@ -46,13 +47,22 @@ SERVICE_DEVICE_REFRESH = "device_refresh"
 SERVICE_REMOVE_ORPHANED_ENTRIES = "remove_orphaned_entries"
 SELECT_GATEWAY_SCHEMA = vol.All(vol.Schema({vol.Optional(CONF_BRIDGE_ID): str}))
 
+SUPPORTED_SERVICES = (
+    SERVICE_CONFIGURE_DEVICE,
+    SERVICE_DEVICE_REFRESH,
+    SERVICE_REMOVE_ORPHANED_ENTRIES,
+)
 
-async def async_setup_services(hass):
+SERVICE_TO_SCHEMA = {
+    SERVICE_CONFIGURE_DEVICE: SERVICE_CONFIGURE_DEVICE_SCHEMA,
+    SERVICE_DEVICE_REFRESH: SELECT_GATEWAY_SCHEMA,
+    SERVICE_REMOVE_ORPHANED_ENTRIES: SELECT_GATEWAY_SCHEMA,
+}
+
+
+@callback
+def async_setup_services(hass):
     """Set up services for deCONZ integration."""
-    if hass.data.get(DECONZ_SERVICES, False):
-        return
-
-    hass.data[DECONZ_SERVICES] = True
 
     async def async_call_deconz_service(service_call):
         """Call correct deCONZ service."""
@@ -83,38 +93,20 @@ async def async_setup_services(hass):
         elif service == SERVICE_REMOVE_ORPHANED_ENTRIES:
             await async_remove_orphaned_entries_service(gateway)
 
-    hass.services.async_register(
-        DOMAIN,
-        SERVICE_CONFIGURE_DEVICE,
-        async_call_deconz_service,
-        schema=SERVICE_CONFIGURE_DEVICE_SCHEMA,
-    )
-
-    hass.services.async_register(
-        DOMAIN,
-        SERVICE_DEVICE_REFRESH,
-        async_call_deconz_service,
-        schema=SELECT_GATEWAY_SCHEMA,
-    )
-
-    hass.services.async_register(
-        DOMAIN,
-        SERVICE_REMOVE_ORPHANED_ENTRIES,
-        async_call_deconz_service,
-        schema=SELECT_GATEWAY_SCHEMA,
-    )
+    for service in SUPPORTED_SERVICES:
+        hass.services.async_register(
+            DOMAIN,
+            service,
+            async_call_deconz_service,
+            schema=SERVICE_TO_SCHEMA[service],
+        )
 
 
-async def async_unload_services(hass):
+@callback
+def async_unload_services(hass):
     """Unload deCONZ services."""
-    if not hass.data.get(DECONZ_SERVICES):
-        return
-
-    hass.data[DECONZ_SERVICES] = False
-
-    hass.services.async_remove(DOMAIN, SERVICE_CONFIGURE_DEVICE)
-    hass.services.async_remove(DOMAIN, SERVICE_DEVICE_REFRESH)
-    hass.services.async_remove(DOMAIN, SERVICE_REMOVE_ORPHANED_ENTRIES)
+    for service in SUPPORTED_SERVICES:
+        hass.services.async_remove(DOMAIN, service)
 
 
 async def async_configure_service(gateway, data):

tests/components/deconz/test_services.py

@@ -1,5 +1,5 @@
 """deCONZ service tests."""
-from unittest.mock import Mock, patch
+from unittest.mock import patch
 
 import pytest
 import voluptuous as vol
@@ -10,15 +10,13 @@ from homeassistant.components.deconz.const import (
 )
 from homeassistant.components.deconz.deconz_event import CONF_DECONZ_EVENT
 from homeassistant.components.deconz.services import (
-    DECONZ_SERVICES,
     SERVICE_CONFIGURE_DEVICE,
     SERVICE_DATA,
     SERVICE_DEVICE_REFRESH,
     SERVICE_ENTITY,
     SERVICE_FIELD,
     SERVICE_REMOVE_ORPHANED_ENTRIES,
-    async_setup_services,
-    async_unload_services,
+    SUPPORTED_SERVICES,
 )
 from homeassistant.components.sensor import DOMAIN as SENSOR_DOMAIN
 from homeassistant.helpers import device_registry as dr, entity_registry as er
@@ -35,46 +33,33 @@ from .test_gateway import (
 from tests.common import async_capture_events
 
 
-async def test_service_setup(hass):
+async def test_service_setup_and_unload(hass, aioclient_mock):
     """Verify service setup works."""
-    assert DECONZ_SERVICES not in hass.data
-    with patch(
-        "homeassistant.core.ServiceRegistry.async_register", return_value=Mock(True)
-    ) as async_register:
-        await async_setup_services(hass)
-        assert hass.data[DECONZ_SERVICES] is True
-        assert async_register.call_count == 3
+    config_entry = await setup_deconz_integration(hass, aioclient_mock)
+    for service in SUPPORTED_SERVICES:
+        assert hass.services.has_service(DECONZ_DOMAIN, service)
+
+    assert await hass.config_entries.async_unload(config_entry.entry_id)
+    for service in SUPPORTED_SERVICES:
+        assert not hass.services.has_service(DECONZ_DOMAIN, service)
 
 
-async def test_service_setup_already_registered(hass):
-    """Make sure that services are only registered once."""
-    hass.data[DECONZ_SERVICES] = True
-    with patch(
-        "homeassistant.core.ServiceRegistry.async_register", return_value=Mock(True)
-    ) as async_register:
-        await async_setup_services(hass)
-        async_register.assert_not_called()
+@patch("homeassistant.core.ServiceRegistry.async_remove")
+@patch("homeassistant.core.ServiceRegistry.async_register")
+async def test_service_setup_and_unload_not_called_if_multiple_integrations_detected(
+    register_service_mock, remove_service_mock, hass, aioclient_mock
+):
+    """Make sure that services are only setup and removed once."""
+    config_entry = await setup_deconz_integration(hass, aioclient_mock)
+    register_service_mock.reset_mock()
+    config_entry_2 = await setup_deconz_integration(hass, aioclient_mock, entry_id=2)
+    register_service_mock.assert_not_called()
 
-
-async def test_service_unload(hass):
-    """Verify service unload works."""
-    hass.data[DECONZ_SERVICES] = True
-    with patch(
-        "homeassistant.core.ServiceRegistry.async_remove", return_value=Mock(True)
-    ) as async_remove:
-        await async_unload_services(hass)
-        assert hass.data[DECONZ_SERVICES] is False
-        assert async_remove.call_count == 3
-
-
-async def test_service_unload_not_registered(hass):
-    """Make sure that services can only be unloaded once."""
-    with patch(
-        "homeassistant.core.ServiceRegistry.async_remove", return_value=Mock(True)
-    ) as async_remove:
-        await async_unload_services(hass)
-        assert DECONZ_SERVICES not in hass.data
-        async_remove.assert_not_called()
+    register_service_mock.assert_not_called()
+    assert await hass.config_entries.async_unload(config_entry_2.entry_id)
+    remove_service_mock.assert_not_called()
+    assert await hass.config_entries.async_unload(config_entry.entry_id)
+    assert remove_service_mock.call_count == 3
 
 
 async def test_configure_service_with_field(hass, aioclient_mock):