From 03fac0c529fea8ffaa5290a194c863423f923d5d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 17 Aug 2022 16:37:47 -1000 Subject: [PATCH] Fix race in notify setup (#76954) --- homeassistant/components/notify/__init__.py | 12 ++- homeassistant/components/notify/legacy.py | 21 ++--- tests/components/notify/test_init.py | 99 ++++++++++++++++++++- 3 files changed, 119 insertions(+), 13 deletions(-) diff --git a/homeassistant/components/notify/__init__.py b/homeassistant/components/notify/__init__.py index 788c698c0cac..60d245785932 100644 --- a/homeassistant/components/notify/__init__.py +++ b/homeassistant/components/notify/__init__.py @@ -1,6 +1,8 @@ """Provides functionality to notify people.""" from __future__ import annotations +import asyncio + import voluptuous as vol import homeassistant.components.persistent_notification as pn @@ -40,13 +42,19 @@ PLATFORM_SCHEMA = vol.Schema( async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: """Set up the notify services.""" + platform_setups = async_setup_legacy(hass, config) + # We need to add the component here break the deadlock # when setting up integrations from config entries as # they would otherwise wait for notify to be # setup and thus the config entries would not be able to - # setup their platforms. + # setup their platforms, but we need to do it after + # the dispatcher is connected so we don't miss integrations + # that are registered before the dispatcher is connected hass.config.components.add(DOMAIN) - await async_setup_legacy(hass, config) + + if platform_setups: + await asyncio.wait([asyncio.create_task(setup) for setup in platform_setups]) async def persistent_notification(service: ServiceCall) -> None: """Send notification via the built-in persistsent_notify integration.""" diff --git a/homeassistant/components/notify/legacy.py b/homeassistant/components/notify/legacy.py index 50b023248275..f9066b7dff99 100644 --- a/homeassistant/components/notify/legacy.py +++ b/homeassistant/components/notify/legacy.py @@ -2,6 +2,7 @@ from __future__ import annotations import asyncio +from collections.abc import Coroutine from functools import partial from typing import Any, cast @@ -32,7 +33,10 @@ NOTIFY_SERVICES = "notify_services" NOTIFY_DISCOVERY_DISPATCHER = "notify_discovery_dispatcher" -async def async_setup_legacy(hass: HomeAssistant, config: ConfigType) -> None: +@callback +def async_setup_legacy( + hass: HomeAssistant, config: ConfigType +) -> list[Coroutine[Any, Any, None]]: """Set up legacy notify services.""" hass.data.setdefault(NOTIFY_SERVICES, {}) hass.data.setdefault(NOTIFY_DISCOVERY_DISPATCHER, None) @@ -101,15 +105,6 @@ async def async_setup_legacy(hass: HomeAssistant, config: ConfigType) -> None: ) hass.config.components.add(f"{DOMAIN}.{integration_name}") - setup_tasks = [ - asyncio.create_task(async_setup_platform(integration_name, p_config)) - for integration_name, p_config in config_per_platform(config, DOMAIN) - if integration_name is not None - ] - - if setup_tasks: - await asyncio.wait(setup_tasks) - async def async_platform_discovered( platform: str, info: DiscoveryInfoType | None ) -> None: @@ -120,6 +115,12 @@ async def async_setup_legacy(hass: HomeAssistant, config: ConfigType) -> None: hass, DOMAIN, async_platform_discovered ) + return [ + async_setup_platform(integration_name, p_config) + for integration_name, p_config in config_per_platform(config, DOMAIN) + if integration_name is not None + ] + @callback def check_templates_warn(hass: HomeAssistant, tpl: template.Template) -> None: diff --git a/tests/components/notify/test_init.py b/tests/components/notify/test_init.py index ae32884add7f..b691ed7a0519 100644 --- a/tests/components/notify/test_init.py +++ b/tests/components/notify/test_init.py @@ -1,12 +1,13 @@ """The tests for notify services that change targets.""" +import asyncio from unittest.mock import Mock, patch import yaml from homeassistant import config as hass_config from homeassistant.components import notify -from homeassistant.const import SERVICE_RELOAD +from homeassistant.const import SERVICE_RELOAD, Platform from homeassistant.core import HomeAssistant from homeassistant.helpers.discovery import async_load_platform from homeassistant.helpers.reload import async_setup_reload_service @@ -330,3 +331,99 @@ async def test_setup_platform_and_reload(hass, caplog, tmp_path): # Check if the dynamically notify services from setup were removed assert not hass.services.has_service(notify.DOMAIN, "testnotify2_c") assert not hass.services.has_service(notify.DOMAIN, "testnotify2_d") + + +async def test_setup_platform_before_notify_setup(hass, caplog, tmp_path): + """Test trying to setup a platform before notify is setup.""" + get_service_called = Mock() + + async def async_get_service(hass, config, discovery_info=None): + """Get notify service for mocked platform.""" + get_service_called(config, discovery_info) + targetlist = {"a": 1, "b": 2} + return NotificationService(hass, targetlist, "testnotify") + + async def async_get_service2(hass, config, discovery_info=None): + """Get notify service for mocked platform.""" + get_service_called(config, discovery_info) + targetlist = {"c": 3, "d": 4} + return NotificationService(hass, targetlist, "testnotify2") + + # Mock first platform + mock_notify_platform( + hass, tmp_path, "testnotify", async_get_service=async_get_service + ) + + # Initialize a second platform testnotify2 + mock_notify_platform( + hass, tmp_path, "testnotify2", async_get_service=async_get_service2 + ) + + hass_config = {"notify": [{"platform": "testnotify"}]} + + # Setup the second testnotify2 platform from discovery + load_coro = async_load_platform( + hass, Platform.NOTIFY, "testnotify2", {}, hass_config=hass_config + ) + + # Setup the testnotify platform + setup_coro = async_setup_component(hass, "notify", hass_config) + + load_task = asyncio.create_task(load_coro) + setup_task = asyncio.create_task(setup_coro) + + await asyncio.gather(load_task, setup_task) + + await hass.async_block_till_done() + assert hass.services.has_service(notify.DOMAIN, "testnotify_a") + assert hass.services.has_service(notify.DOMAIN, "testnotify_b") + assert hass.services.has_service(notify.DOMAIN, "testnotify2_c") + assert hass.services.has_service(notify.DOMAIN, "testnotify2_d") + + +async def test_setup_platform_after_notify_setup(hass, caplog, tmp_path): + """Test trying to setup a platform after notify is setup.""" + get_service_called = Mock() + + async def async_get_service(hass, config, discovery_info=None): + """Get notify service for mocked platform.""" + get_service_called(config, discovery_info) + targetlist = {"a": 1, "b": 2} + return NotificationService(hass, targetlist, "testnotify") + + async def async_get_service2(hass, config, discovery_info=None): + """Get notify service for mocked platform.""" + get_service_called(config, discovery_info) + targetlist = {"c": 3, "d": 4} + return NotificationService(hass, targetlist, "testnotify2") + + # Mock first platform + mock_notify_platform( + hass, tmp_path, "testnotify", async_get_service=async_get_service + ) + + # Initialize a second platform testnotify2 + mock_notify_platform( + hass, tmp_path, "testnotify2", async_get_service=async_get_service2 + ) + + hass_config = {"notify": [{"platform": "testnotify"}]} + + # Setup the second testnotify2 platform from discovery + load_coro = async_load_platform( + hass, Platform.NOTIFY, "testnotify2", {}, hass_config=hass_config + ) + + # Setup the testnotify platform + setup_coro = async_setup_component(hass, "notify", hass_config) + + setup_task = asyncio.create_task(setup_coro) + load_task = asyncio.create_task(load_coro) + + await asyncio.gather(load_task, setup_task) + + await hass.async_block_till_done() + assert hass.services.has_service(notify.DOMAIN, "testnotify_a") + assert hass.services.has_service(notify.DOMAIN, "testnotify_b") + assert hass.services.has_service(notify.DOMAIN, "testnotify2_c") + assert hass.services.has_service(notify.DOMAIN, "testnotify2_d")