From 8407a33e822e5e054d4bd9d058e32c302a36804c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 31 Aug 2020 10:17:37 -0500 Subject: [PATCH 01/20] Support reloading the group notify platform Add support to resetup notify platforms --- homeassistant/components/group/__init__.py | 2 +- homeassistant/components/notify/__init__.py | 46 +++++++++++++-- homeassistant/helpers/reload.py | 63 ++++++++++++++++----- tests/components/group/test_notify.py | 60 +++++++++++++++++++- tests/fixtures/group/configuration.yaml | 7 +++ 5 files changed, 156 insertions(+), 22 deletions(-) diff --git a/homeassistant/components/group/__init__.py b/homeassistant/components/group/__init__.py index cda49591d5c8ab..87eb2cd615b0fa 100644 --- a/homeassistant/components/group/__init__.py +++ b/homeassistant/components/group/__init__.py @@ -58,7 +58,7 @@ SERVICE_SET = "set" SERVICE_REMOVE = "remove" -PLATFORMS = ["light", "cover"] +PLATFORMS = ["light", "cover", "notify"] _LOGGER = logging.getLogger(__name__) diff --git a/homeassistant/components/notify/__init__.py b/homeassistant/components/notify/__init__.py index 77ec48c543562c..226501133cbfb7 100644 --- a/homeassistant/components/notify/__init__.py +++ b/homeassistant/components/notify/__init__.py @@ -60,17 +60,53 @@ @bind_hass async def async_reload(hass, integration_name): """Register notify services for an integration.""" + await _do_with_notify_services(hass, integration_name, _async_setup_notify_services) + + +@bind_hass +async def async_reset_platform(hass, integration_name): + """Unregister notify services for an integration.""" + if not await _do_with_notify_services( + hass, integration_name, _async_unregister_notify_services + ): + return + del hass.data[NOTIFY_SERVICES][integration_name] + + +async def _do_with_notify_services(hass, integration_name, call): + """Call a method on all notify services for an integration.""" if ( NOTIFY_SERVICES not in hass.data or integration_name not in hass.data[NOTIFY_SERVICES] ): - return + return False - tasks = [ - _async_setup_notify_services(hass, data) - for data in hass.data[NOTIFY_SERVICES][integration_name] - ] + tasks = [call(hass, data) for data in hass.data[NOTIFY_SERVICES][integration_name]] await asyncio.gather(*tasks) + return True + + +async def _async_unregister_notify_services(hass, data): + """Remove the notify services.""" + targets = data[TARGETS] + + if targets: + stale_targets = set(targets) + for stale_target_name, target in stale_targets: + del targets[stale_target_name] + hass.services.async_remove( + DOMAIN, + stale_target_name, + ) + + friendly_name_slug = slugify(data[FRIENDLY_NAME]) + if not hass.services.has_service(DOMAIN, friendly_name_slug): + return + + hass.services.async_remove( + DOMAIN, + friendly_name_slug, + ) async def _async_setup_notify_services(hass, data): diff --git a/homeassistant/helpers/reload.py b/homeassistant/helpers/reload.py index 1ffba25ce15d8b..4738e5faaa94f5 100644 --- a/homeassistant/helpers/reload.py +++ b/homeassistant/helpers/reload.py @@ -34,29 +34,62 @@ async def async_reload_integration_platforms( _LOGGER.error(err) return - for integration_platform in integration_platforms: - platform = async_get_platform(hass, integration_name, integration_platform) + tasks = [ + _resetup_platform( + hass, integration_name, integration_platform, unprocessed_conf + ) + for integration_platform in integration_platforms + ] - if not platform: - continue + await asyncio.gather(*tasks) - integration = await async_get_integration(hass, integration_platform) - conf = await conf_util.async_process_component_config( - hass, unprocessed_conf, integration - ) +async def _resetup_platform( + hass: HomeAssistantType, + integration_name: str, + integration_platform: str, + unprocessed_conf: Dict, +) -> None: + """Resetup a platform.""" + integration = await async_get_integration(hass, integration_platform) + + conf = await conf_util.async_process_component_config( + hass, unprocessed_conf, integration + ) + + if not conf: + return - if not conf: + root_config: Dict = {integration_platform: []} + # Extract only the config for template, ignore the rest. + for p_type, p_config in config_per_platform(conf, integration_platform): + if p_type != integration_name: continue - await platform.async_reset() + root_config[integration_platform].append(p_config) + + if not root_config[integration_platform]: + return + + component = integration.get_component() + + if hasattr(component, "async_reset_platform"): + # If the integration has its own way to reset + # use this method. + await component.async_reset_platform(hass, integration_name) # type: ignore + await component.async_setup(hass, root_config) # type: ignore + return + + # If its an entity platform, we use the entity_platform + # async_reset method + platform = async_get_platform(hass, integration_name, integration_platform) + if not platform: + return - # Extract only the config for template, ignore the rest. - for p_type, p_config in config_per_platform(conf, integration_platform): - if p_type != integration_name: - continue + await platform.async_reset() - await platform.async_setup(p_config) # type: ignore + tasks = [platform.async_setup(p_config) for p_config in root_config[integration_platform]] # type: ignore + await asyncio.gather(*tasks) async def async_integration_yaml_config( diff --git a/tests/components/group/test_notify.py b/tests/components/group/test_notify.py index b120cf2cea4c8b..62b395dcf4dc2e 100644 --- a/tests/components/group/test_notify.py +++ b/tests/components/group/test_notify.py @@ -1,11 +1,14 @@ """The tests for the notify.group platform.""" import asyncio +from os import path import unittest +from homeassistant import config as hass_config import homeassistant.components.demo.notify as demo +from homeassistant.components.group import SERVICE_RELOAD import homeassistant.components.group.notify as group import homeassistant.components.notify as notify -from homeassistant.setup import setup_component +from homeassistant.setup import async_setup_component, setup_component from tests.async_mock import MagicMock, patch from tests.common import assert_setup_component, get_test_home_assistant @@ -90,3 +93,58 @@ def test_send_message_with_data(self): "title": "Test notification", "data": {"hello": "world", "test": "message"}, } + + +async def test_reload_notify(hass): + """Verify we can reload the notify service.""" + + assert await async_setup_component( + hass, + "group", + {}, + ) + await hass.async_block_till_done() + + assert await async_setup_component( + hass, + notify.DOMAIN, + { + notify.DOMAIN: [ + {"name": "demo1", "platform": "demo"}, + {"name": "demo2", "platform": "demo"}, + { + "name": "group_notify", + "platform": "group", + "services": [{"service": "demo1"}], + }, + ] + }, + ) + await hass.async_block_till_done() + + assert hass.services.has_service(notify.DOMAIN, "demo1") + assert hass.services.has_service(notify.DOMAIN, "demo2") + assert hass.services.has_service(notify.DOMAIN, "group_notify") + + yaml_path = path.join( + _get_fixtures_base_path(), + "fixtures", + "group/configuration.yaml", + ) + with patch.object(hass_config, "YAML_CONFIG_FILE", yaml_path): + await hass.services.async_call( + "group", + SERVICE_RELOAD, + {}, + blocking=True, + ) + await hass.async_block_till_done() + + assert hass.services.has_service(notify.DOMAIN, "demo1") + assert hass.services.has_service(notify.DOMAIN, "demo2") + assert not hass.services.has_service(notify.DOMAIN, "group_notify") + assert hass.services.has_service(notify.DOMAIN, "new_group_notify") + + +def _get_fixtures_base_path(): + return path.dirname(path.dirname(path.dirname(__file__))) diff --git a/tests/fixtures/group/configuration.yaml b/tests/fixtures/group/configuration.yaml index 9047024e3de02b..0a5c9e18bd187d 100644 --- a/tests/fixtures/group/configuration.yaml +++ b/tests/fixtures/group/configuration.yaml @@ -9,3 +9,10 @@ light: entities: - light.outside_patio_lights - light.outside_patio_lights_2 + +notify: + - platform: group + name: new_group_notify + services: + - service: demo1 + - service: demo2 From b1eda7e4a8e117c3c01d9c3ae63cd0e733cbba56 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 31 Aug 2020 10:38:12 -0500 Subject: [PATCH 02/20] Make sure removals still work --- homeassistant/helpers/reload.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/homeassistant/helpers/reload.py b/homeassistant/helpers/reload.py index 4738e5faaa94f5..3a191e64d08d1e 100644 --- a/homeassistant/helpers/reload.py +++ b/homeassistant/helpers/reload.py @@ -68,9 +68,6 @@ async def _resetup_platform( root_config[integration_platform].append(p_config) - if not root_config[integration_platform]: - return - component = integration.get_component() if hasattr(component, "async_reset_platform"): From 2c30f95c30c14ce0e284823708ea438d403ff007 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 31 Aug 2020 11:47:38 -0500 Subject: [PATCH 03/20] remove unused var --- homeassistant/components/notify/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/homeassistant/components/notify/__init__.py b/homeassistant/components/notify/__init__.py index 226501133cbfb7..ed66d16b30c316 100644 --- a/homeassistant/components/notify/__init__.py +++ b/homeassistant/components/notify/__init__.py @@ -91,12 +91,12 @@ async def _async_unregister_notify_services(hass, data): targets = data[TARGETS] if targets: - stale_targets = set(targets) - for stale_target_name, target in stale_targets: - del targets[stale_target_name] + remove_targets = set(targets) + for remove_target_name in remove_targets: + del targets[remove_target_name] hass.services.async_remove( DOMAIN, - stale_target_name, + remove_target_name, ) friendly_name_slug = slugify(data[FRIENDLY_NAME]) From ccc936220d445a5cdb716efe0359ce4a665ca51c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 31 Aug 2020 12:41:11 -0500 Subject: [PATCH 04/20] coverage --- tests/helpers/test_reload.py | 101 ++++++++++++++++++++++++++++++++++- 1 file changed, 100 insertions(+), 1 deletion(-) diff --git a/tests/helpers/test_reload.py b/tests/helpers/test_reload.py index dafcbebdb6ec9f..25844151533fc6 100644 --- a/tests/helpers/test_reload.py +++ b/tests/helpers/test_reload.py @@ -13,8 +13,9 @@ async_reload_integration_platforms, async_setup_reload_service, ) +from homeassistant.loader import async_get_integration -from tests.async_mock import Mock, patch +from tests.async_mock import AsyncMock, Mock, patch from tests.common import ( MockModule, MockPlatform, @@ -109,6 +110,104 @@ async def setup_platform(*args): assert len(setup_called) == 2 +async def test_setup_reload_service_when_async_process_component_config_fails(hass): + """Test setting up a reload service with the config processing failing.""" + component_setup = Mock(return_value=True) + + setup_called = [] + + async def setup_platform(*args): + setup_called.append(args) + + mock_integration(hass, MockModule(DOMAIN, setup=component_setup)) + mock_integration(hass, MockModule(PLATFORM, dependencies=[DOMAIN])) + + mock_platform = MockPlatform(async_setup_platform=setup_platform) + mock_entity_platform(hass, f"{DOMAIN}.{PLATFORM}", mock_platform) + + component = EntityComponent(_LOGGER, DOMAIN, hass) + + await component.async_setup({DOMAIN: {"platform": PLATFORM, "sensors": None}}) + await hass.async_block_till_done() + assert component_setup.called + + assert f"{DOMAIN}.{PLATFORM}" in hass.config.components + assert len(setup_called) == 1 + + await async_setup_reload_service(hass, PLATFORM, [DOMAIN]) + + yaml_path = path.join( + _get_fixtures_base_path(), + "fixtures", + "helpers/reload_configuration.yaml", + ) + with patch.object(config, "YAML_CONFIG_FILE", yaml_path), patch.object( + config, "async_process_component_config", return_value=None + ): + await hass.services.async_call( + PLATFORM, + SERVICE_RELOAD, + {}, + blocking=True, + ) + await hass.async_block_till_done() + + assert len(setup_called) == 1 + + +async def test_setup_reload_service_with_platform_that_provides_async_reset_platform( + hass, +): + """Test setting up a reload service using a platform that has its own async_reset_platform.""" + component_setup = AsyncMock(return_value=True) + + setup_called = [] + async_reset_platform_called = [] + + async def setup_platform(*args): + setup_called.append(args) + + async def async_reset_platform(*args): + async_reset_platform_called.append(args) + + mock_integration(hass, MockModule(DOMAIN, async_setup=component_setup)) + integration = await async_get_integration(hass, DOMAIN) + integration.get_component().async_reset_platform = async_reset_platform + + mock_integration(hass, MockModule(PLATFORM, dependencies=[DOMAIN])) + + mock_platform = MockPlatform(async_setup_platform=setup_platform) + mock_entity_platform(hass, f"{DOMAIN}.{PLATFORM}", mock_platform) + + component = EntityComponent(_LOGGER, DOMAIN, hass) + + await component.async_setup({DOMAIN: {"platform": PLATFORM, "name": "xyz"}}) + await hass.async_block_till_done() + assert component_setup.called + + assert f"{DOMAIN}.{PLATFORM}" in hass.config.components + assert len(setup_called) == 1 + + await async_setup_reload_service(hass, PLATFORM, [DOMAIN]) + + yaml_path = path.join( + _get_fixtures_base_path(), + "fixtures", + "helpers/reload_configuration.yaml", + ) + with patch.object(config, "YAML_CONFIG_FILE", yaml_path): + await hass.services.async_call( + PLATFORM, + SERVICE_RELOAD, + {}, + blocking=True, + ) + await hass.async_block_till_done() + + assert len(setup_called) == 1 + assert len(async_reset_platform_called) == 1 + + async def test_async_integration_yaml_config(hass): """Test loading yaml config for an integration.""" mock_integration(hass, MockModule(DOMAIN)) From d16fe1a334045448fbb22801178e4f8a90b1a4d1 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 31 Aug 2020 16:05:15 -0500 Subject: [PATCH 05/20] update services.yaml --- homeassistant/components/group/services.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/group/services.yaml b/homeassistant/components/group/services.yaml index cec4f187ca682c..57e11d672dc0cd 100644 --- a/homeassistant/components/group/services.yaml +++ b/homeassistant/components/group/services.yaml @@ -1,6 +1,6 @@ # Describes the format for available group services reload: - description: Reload group configuration. + description: Reload group configuration, entities, and notify services. set: description: Create/Update a user group. From b109654a4a3b8c8bd22228fd37875d904143000f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 2 Sep 2020 09:25:32 -0500 Subject: [PATCH 06/20] review adjustments --- homeassistant/components/notify/__init__.py | 33 ++++----- homeassistant/helpers/reload.py | 44 ++++++++++-- tests/components/group/test_light.py | 74 +++++++++++++++++++++ 3 files changed, 128 insertions(+), 23 deletions(-) diff --git a/homeassistant/components/notify/__init__.py b/homeassistant/components/notify/__init__.py index ed66d16b30c316..2bbc31838bcd54 100644 --- a/homeassistant/components/notify/__init__.py +++ b/homeassistant/components/notify/__init__.py @@ -39,8 +39,8 @@ NOTIFY_SERVICES = "notify_services" SERVICE = "service" TARGETS = "targets" -FRIENDLY_NAME = "friendly_name" -TARGET_FRIENDLY_NAME = "target_friendly_name" +SERVICE_NAME = "service_name" +TARGET_SERVICE_NAME_PREFIX = "target_service_name_prefix" PLATFORM_SCHEMA = vol.Schema( {vol.Required(CONF_PLATFORM): cv.string, vol.Optional(CONF_NAME): cv.string}, @@ -99,20 +99,19 @@ async def _async_unregister_notify_services(hass, data): remove_target_name, ) - friendly_name_slug = slugify(data[FRIENDLY_NAME]) - if not hass.services.has_service(DOMAIN, friendly_name_slug): + service_name = data[SERVICE_NAME] + if not hass.services.has_service(DOMAIN, service_name): return hass.services.async_remove( DOMAIN, - friendly_name_slug, + service_name, ) async def _async_setup_notify_services(hass, data): """Create or remove the notify services.""" notify_service = data[SERVICE] - friendly_name = data[FRIENDLY_NAME] targets = data[TARGETS] async def _async_notify_message(service): @@ -120,11 +119,11 @@ async def _async_notify_message(service): await _async_notify_message_service(hass, service, notify_service, targets) if hasattr(notify_service, "targets"): - target_friendly_name = data[TARGET_FRIENDLY_NAME] + target_service_name_prefix = data[TARGET_SERVICE_NAME_PREFIX] stale_targets = set(targets) for name, target in notify_service.targets.items(): - target_name = slugify(f"{target_friendly_name}_{name}") + target_name = slugify(f"{target_service_name_prefix}_{name}") if target_name in stale_targets: stale_targets.remove(target_name) if target_name in targets: @@ -144,13 +143,12 @@ async def _async_notify_message(service): stale_target_name, ) - friendly_name_slug = slugify(friendly_name) - if hass.services.has_service(DOMAIN, friendly_name_slug): + if hass.services.has_service(DOMAIN, data[SERVICE_NAME]): return hass.services.async_register( DOMAIN, - friendly_name_slug, + data[SERVICE_NAME], _async_notify_message, schema=NOTIFY_SERVICE_SCHEMA, ) @@ -229,18 +227,15 @@ async def async_setup_platform( if discovery_info is None: discovery_info = {} - target_friendly_name = ( - p_config.get(CONF_NAME) or discovery_info.get(CONF_NAME) or integration_name - ) - friendly_name = ( - p_config.get(CONF_NAME) or discovery_info.get(CONF_NAME) or SERVICE_NOTIFY - ) + conf_name = p_config.get(CONF_NAME) or discovery_info.get(CONF_NAME) + target_service_name_prefix = conf_name or integration_name + service_name = conf_name or SERVICE_NOTIFY data = { - FRIENDLY_NAME: friendly_name, + SERVICE_NAME: slugify(service_name), # The targets use a slightly different friendly name # selection pattern than the base service - TARGET_FRIENDLY_NAME: target_friendly_name, + TARGET_SERVICE_NAME_PREFIX: target_service_name_prefix, SERVICE: notify_service, TARGETS: {}, } diff --git a/homeassistant/helpers/reload.py b/homeassistant/helpers/reload.py index 3a191e64d08d1e..19a1e46a6d45a3 100644 --- a/homeassistant/helpers/reload.py +++ b/homeassistant/helpers/reload.py @@ -2,7 +2,7 @@ import asyncio import logging -from typing import Any, Dict, Iterable, Optional +from typing import Any, Dict, Iterable, List, Optional from homeassistant import config as conf_util from homeassistant.const import SERVICE_RELOAD @@ -12,6 +12,7 @@ from homeassistant.helpers.entity_platform import DATA_ENTITY_PLATFORM, EntityPlatform from homeassistant.helpers.typing import HomeAssistantType from homeassistant.loader import async_get_integration +from homeassistant.setup import async_setup_component _LOGGER = logging.getLogger(__name__) @@ -80,12 +81,47 @@ async def _resetup_platform( # If its an entity platform, we use the entity_platform # async_reset method platform = async_get_platform(hass, integration_name, integration_platform) - if not platform: + if platform: + await _async_reconfig_platform(platform, root_config[integration_platform]) return - await platform.async_reset() + if not root_config[integration_platform]: + # No config for this platform + # and its not loaded. Nothing to do + return + + await _async_setup_platform( + hass, integration_name, integration_platform, root_config[integration_platform] + ) + - tasks = [platform.async_setup(p_config) for p_config in root_config[integration_platform]] # type: ignore +async def _async_setup_platform( + hass: HomeAssistantType, + integration_name: str, + integration_platform: str, + platform_configs: List[Dict], +) -> None: + """Platform for the first time when new configuration is added.""" + if integration_platform not in hass.data: + await async_setup_component( + hass, integration_platform, {integration_platform: platform_configs} + ) + return + + entity_component = hass.data[integration_platform] + tasks = [ + entity_component.async_setup_platform(integration_name, p_config) + for p_config in platform_configs + ] + await asyncio.gather(*tasks) + + +async def _async_reconfig_platform( + platform: EntityPlatform, platform_configs: List[Dict] +) -> None: + """Reconfigure an already loaded platform.""" + await platform.async_reset() + tasks = [platform.async_setup(p_config) for p_config in platform_configs] # type: ignore await asyncio.gather(*tasks) diff --git a/tests/components/group/test_light.py b/tests/components/group/test_light.py index 8855ca9762619e..a22c56b4bfc017 100644 --- a/tests/components/group/test_light.py +++ b/tests/components/group/test_light.py @@ -683,5 +683,79 @@ async def test_reload(hass): assert hass.states.get("light.outside_patio_lights_g") is not None +async def test_reload_with_platform_not_setup(hass): + """Test the ability to reload lights.""" + hass.states.async_set("light.bowl", STATE_ON) + await async_setup_component( + hass, + LIGHT_DOMAIN, + { + LIGHT_DOMAIN: [ + {"platform": "demo"}, + ] + }, + ) + assert await async_setup_component( + hass, + "group", + { + "group": { + "group_zero": {"entities": "light.Bowl", "icon": "mdi:work"}, + } + }, + ) + await hass.async_block_till_done() + + yaml_path = path.join( + _get_fixtures_base_path(), + "fixtures", + "group/configuration.yaml", + ) + with patch.object(hass_config, "YAML_CONFIG_FILE", yaml_path): + await hass.services.async_call( + DOMAIN, + SERVICE_RELOAD, + {}, + blocking=True, + ) + await hass.async_block_till_done() + + assert hass.states.get("light.light_group") is None + assert hass.states.get("light.master_hall_lights_g") is not None + assert hass.states.get("light.outside_patio_lights_g") is not None + + +async def test_reload_with_base_integration_platform_not_setup(hass): + """Test the ability to reload lights.""" + assert await async_setup_component( + hass, + "group", + { + "group": { + "group_zero": {"entities": "light.Bowl", "icon": "mdi:work"}, + } + }, + ) + await hass.async_block_till_done() + + yaml_path = path.join( + _get_fixtures_base_path(), + "fixtures", + "group/configuration.yaml", + ) + with patch.object(hass_config, "YAML_CONFIG_FILE", yaml_path): + await hass.services.async_call( + DOMAIN, + SERVICE_RELOAD, + {}, + blocking=True, + ) + await hass.async_block_till_done() + + assert hass.states.get("light.light_group") is None + assert hass.states.get("light.master_hall_lights_g") is not None + assert hass.states.get("light.outside_patio_lights_g") is not None + + def _get_fixtures_base_path(): return path.dirname(path.dirname(path.dirname(__file__))) From eda57542e0621dd0b0a081cdf7f40a8db067d7b6 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 2 Sep 2020 11:32:00 -0500 Subject: [PATCH 07/20] refactor --- homeassistant/components/notify/__init__.py | 211 +++++++++++--------- 1 file changed, 113 insertions(+), 98 deletions(-) diff --git a/homeassistant/components/notify/__init__.py b/homeassistant/components/notify/__init__.py index 2bbc31838bcd54..fd89cc513834f0 100644 --- a/homeassistant/components/notify/__init__.py +++ b/homeassistant/components/notify/__init__.py @@ -60,120 +60,51 @@ @bind_hass async def async_reload(hass, integration_name): """Register notify services for an integration.""" - await _do_with_notify_services(hass, integration_name, _async_setup_notify_services) - - -@bind_hass -async def async_reset_platform(hass, integration_name): - """Unregister notify services for an integration.""" - if not await _do_with_notify_services( - hass, integration_name, _async_unregister_notify_services - ): + if not _async_integration_has_notify_services(hass, integration_name): return - del hass.data[NOTIFY_SERVICES][integration_name] - - -async def _do_with_notify_services(hass, integration_name, call): - """Call a method on all notify services for an integration.""" - if ( - NOTIFY_SERVICES not in hass.data - or integration_name not in hass.data[NOTIFY_SERVICES] - ): - return False - tasks = [call(hass, data) for data in hass.data[NOTIFY_SERVICES][integration_name]] - await asyncio.gather(*tasks) - return True + await _async_call_notify_service_tasks( + hass, integration_name, _async_register_from_notify_data + ) -async def _async_unregister_notify_services(hass, data): - """Remove the notify services.""" - targets = data[TARGETS] +def _async_register_from_notify_data(data): + return data[SERVICE].async_register_services( + data[SERVICE_NAME], data[TARGET_SERVICE_NAME_PREFIX], data[TARGETS] + ) - if targets: - remove_targets = set(targets) - for remove_target_name in remove_targets: - del targets[remove_target_name] - hass.services.async_remove( - DOMAIN, - remove_target_name, - ) - service_name = data[SERVICE_NAME] - if not hass.services.has_service(DOMAIN, service_name): +@bind_hass +async def async_reset_platform(hass, integration_name): + """Unregister notify services for an integration.""" + if not _async_integration_has_notify_services(hass, integration_name): return - hass.services.async_remove( - DOMAIN, - service_name, + await _async_call_notify_service_tasks( + hass, integration_name, _async_unregister_from_notify_data ) + del hass.data[NOTIFY_SERVICES][integration_name] -async def _async_setup_notify_services(hass, data): - """Create or remove the notify services.""" - notify_service = data[SERVICE] - targets = data[TARGETS] - - async def _async_notify_message(service): - """Handle sending notification message service calls.""" - await _async_notify_message_service(hass, service, notify_service, targets) - - if hasattr(notify_service, "targets"): - target_service_name_prefix = data[TARGET_SERVICE_NAME_PREFIX] - stale_targets = set(targets) - - for name, target in notify_service.targets.items(): - target_name = slugify(f"{target_service_name_prefix}_{name}") - if target_name in stale_targets: - stale_targets.remove(target_name) - if target_name in targets: - continue - targets[target_name] = target - hass.services.async_register( - DOMAIN, - target_name, - _async_notify_message, - schema=NOTIFY_SERVICE_SCHEMA, - ) - - for stale_target_name in stale_targets: - del targets[stale_target_name] - hass.services.async_remove( - DOMAIN, - stale_target_name, - ) - - if hass.services.has_service(DOMAIN, data[SERVICE_NAME]): - return +def _async_unregister_from_notify_data(data): + return data[SERVICE].async_unregister_services(data[SERVICE_NAME], data[TARGETS]) - hass.services.async_register( - DOMAIN, - data[SERVICE_NAME], - _async_notify_message, - schema=NOTIFY_SERVICE_SCHEMA, - ) +async def _async_call_notify_service_tasks(hass, integration_name, call): + tasks = [call(data) for data in hass.data[NOTIFY_SERVICES][integration_name]] -async def _async_notify_message_service(hass, service, notify_service, targets): - """Handle sending notification message service calls.""" - kwargs = {} - message = service.data[ATTR_MESSAGE] - title = service.data.get(ATTR_TITLE) - - if title: - title.hass = hass - kwargs[ATTR_TITLE] = title.async_render() + await asyncio.gather(*tasks) - if targets.get(service.service) is not None: - kwargs[ATTR_TARGET] = [targets[service.service]] - elif service.data.get(ATTR_TARGET) is not None: - kwargs[ATTR_TARGET] = service.data.get(ATTR_TARGET) - message.hass = hass - kwargs[ATTR_MESSAGE] = message.async_render() - kwargs[ATTR_DATA] = service.data.get(ATTR_DATA) +def _async_integration_has_notify_services(hass, integration_name): + """Call a method on all notify services for an integration.""" + if ( + NOTIFY_SERVICES not in hass.data + or integration_name not in hass.data[NOTIFY_SERVICES] + ): + return False - await notify_service.async_send_message(**kwargs) + return True async def async_setup(hass, config): @@ -242,7 +173,7 @@ async def async_setup_platform( hass.data[NOTIFY_SERVICES].setdefault(integration_name, []) hass.data[NOTIFY_SERVICES][integration_name].append(data) - await _async_setup_notify_services(hass, data) + await _async_register_from_notify_data(data) hass.config.components.add(f"{DOMAIN}.{integration_name}") @@ -265,6 +196,28 @@ async def async_platform_discovered(platform, info): return True +async def _async_notify_message_service(hass, service, notify_service, targets): + """Handle sending notification message service calls.""" + kwargs = {} + message = service.data[ATTR_MESSAGE] + title = service.data.get(ATTR_TITLE) + + if title: + title.hass = hass + kwargs[ATTR_TITLE] = title.async_render() + + if targets.get(service.service) is not None: + kwargs[ATTR_TARGET] = [targets[service.service]] + elif service.data.get(ATTR_TARGET) is not None: + kwargs[ATTR_TARGET] = service.data.get(ATTR_TARGET) + + message.hass = hass + kwargs[ATTR_MESSAGE] = message.async_render() + kwargs[ATTR_DATA] = service.data.get(ATTR_DATA) + + await notify_service.async_send_message(**kwargs) + + class BaseNotificationService: """An abstract class for notification services.""" @@ -283,3 +236,65 @@ async def async_send_message(self, message, **kwargs): kwargs can contain ATTR_TITLE to specify a title. """ await self.hass.async_add_job(partial(self.send_message, message, **kwargs)) + + async def async_register_services( + self, service_name, target_service_name_prefix, targets + ): + """Create or remove the notify services.""" + + async def _async_notify_message(service): + """Handle sending notification message service calls.""" + await _async_notify_message_service(self.hass, service, self, targets) + + if hasattr(self, "targets"): + stale_targets = set(targets) + + for name, target in self.targets.items(): # pylint: disable=no-member + target_name = slugify(f"{target_service_name_prefix}_{name}") + if target_name in stale_targets: + stale_targets.remove(target_name) + if target_name in targets: + continue + targets[target_name] = target + self.hass.services.async_register( + DOMAIN, + target_name, + _async_notify_message, + schema=NOTIFY_SERVICE_SCHEMA, + ) + + for stale_target_name in stale_targets: + del targets[stale_target_name] + self.hass.services.async_remove( + DOMAIN, + stale_target_name, + ) + + if self.hass.services.has_service(DOMAIN, service_name): + return + + self.hass.services.async_register( + DOMAIN, + service_name, + _async_notify_message, + schema=NOTIFY_SERVICE_SCHEMA, + ) + + async def async_unregister_services(self, service_name, targets): + """Unregister the notify services.""" + if targets: + remove_targets = set(targets) + for remove_target_name in remove_targets: + del targets[remove_target_name] + self.hass.services.async_remove( + DOMAIN, + remove_target_name, + ) + + if not self.hass.services.has_service(DOMAIN, service_name): + return + + self.hass.services.async_remove( + DOMAIN, + service_name, + ) From ed0e742dce4b9dbd3a4895201c11a1039398f3a2 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 2 Sep 2020 11:54:47 -0500 Subject: [PATCH 08/20] refactor again --- homeassistant/components/notify/__init__.py | 278 ++++++++++---------- 1 file changed, 142 insertions(+), 136 deletions(-) diff --git a/homeassistant/components/notify/__init__.py b/homeassistant/components/notify/__init__.py index fd89cc513834f0..f032659081e5a4 100644 --- a/homeassistant/components/notify/__init__.py +++ b/homeassistant/components/notify/__init__.py @@ -1,8 +1,9 @@ """Provides functionality to notify people.""" import asyncio +from dataclasses import dataclass from functools import partial import logging -from typing import Optional +from typing import Dict, Optional import voluptuous as vol @@ -37,10 +38,6 @@ SERVICE_NOTIFY = "notify" NOTIFY_SERVICES = "notify_services" -SERVICE = "service" -TARGETS = "targets" -SERVICE_NAME = "service_name" -TARGET_SERVICE_NAME_PREFIX = "target_service_name_prefix" PLATFORM_SCHEMA = vol.Schema( {vol.Required(CONF_PLATFORM): cv.string, vol.Optional(CONF_NAME): cv.string}, @@ -63,15 +60,14 @@ async def async_reload(hass, integration_name): if not _async_integration_has_notify_services(hass, integration_name): return - await _async_call_notify_service_tasks( - hass, integration_name, _async_register_from_notify_data - ) - + tasks = [ + data.service.async_register_services( + data.service_name, data.target_service_name_prefix, data.targets + ) + for data in hass.data[NOTIFY_SERVICES][integration_name] + ] -def _async_register_from_notify_data(data): - return data[SERVICE].async_register_services( - data[SERVICE_NAME], data[TARGET_SERVICE_NAME_PREFIX], data[TARGETS] - ) + await asyncio.gather(*tasks) @bind_hass @@ -80,20 +76,137 @@ async def async_reset_platform(hass, integration_name): if not _async_integration_has_notify_services(hass, integration_name): return - await _async_call_notify_service_tasks( - hass, integration_name, _async_unregister_from_notify_data - ) + tasks = [ + data.service.async_unregister_services(data.service_name, data.targets) + for data in hass.data[NOTIFY_SERVICES][integration_name] + ] + + await asyncio.gather(*tasks) + del hass.data[NOTIFY_SERVICES][integration_name] -def _async_unregister_from_notify_data(data): - return data[SERVICE].async_unregister_services(data[SERVICE_NAME], data[TARGETS]) +class BaseNotificationService: + """An abstract class for notification services.""" + hass: Optional[HomeAssistantType] = None -async def _async_call_notify_service_tasks(hass, integration_name, call): - tasks = [call(data) for data in hass.data[NOTIFY_SERVICES][integration_name]] + def send_message(self, message, **kwargs): + """Send a message. - await asyncio.gather(*tasks) + kwargs can contain ATTR_TITLE to specify a title. + """ + raise NotImplementedError() + + async def async_send_message(self, message, **kwargs): + """Send a message. + + kwargs can contain ATTR_TITLE to specify a title. + """ + await self.hass.async_add_job(partial(self.send_message, message, **kwargs)) + + async def _async_notify_message_service(self, service, targets): + """Handle sending notification message service calls.""" + kwargs = {} + message = service.data[ATTR_MESSAGE] + title = service.data.get(ATTR_TITLE) + + if title: + title.hass = self.hass + kwargs[ATTR_TITLE] = title.async_render() + + if targets.get(service.service) is not None: + kwargs[ATTR_TARGET] = [targets[service.service]] + elif service.data.get(ATTR_TARGET) is not None: + kwargs[ATTR_TARGET] = service.data.get(ATTR_TARGET) + + message.hass = self.hass + kwargs[ATTR_MESSAGE] = message.async_render() + kwargs[ATTR_DATA] = service.data.get(ATTR_DATA) + + await self.async_send_message(**kwargs) + + async def async_register_services( + self, service_name, target_service_name_prefix, targets + ): + """Create or remove the notify services.""" + + async def _async_notify_message(service): + """Handle sending notification message service calls.""" + await self._async_notify_message_service(service, targets) + + if hasattr(self, "targets"): + stale_targets = set(targets) + + for name, target in self.targets.items(): # pylint: disable=no-member + target_name = slugify(f"{target_service_name_prefix}_{name}") + if target_name in stale_targets: + stale_targets.remove(target_name) + if target_name in targets: + continue + targets[target_name] = target + self.hass.services.async_register( + DOMAIN, + target_name, + _async_notify_message, + schema=NOTIFY_SERVICE_SCHEMA, + ) + + for stale_target_name in stale_targets: + del targets[stale_target_name] + self.hass.services.async_remove( + DOMAIN, + stale_target_name, + ) + + if self.hass.services.has_service(DOMAIN, service_name): + return + + self.hass.services.async_register( + DOMAIN, + service_name, + _async_notify_message, + schema=NOTIFY_SERVICE_SCHEMA, + ) + + async def async_unregister_services(self, service_name, targets): + """Unregister the notify services.""" + if targets: + remove_targets = set(targets) + for remove_target_name in remove_targets: + del targets[remove_target_name] + self.hass.services.async_remove( + DOMAIN, + remove_target_name, + ) + + if not self.hass.services.has_service(DOMAIN, service_name): + return + + self.hass.services.async_remove( + DOMAIN, + service_name, + ) + + +@dataclass +class NotifyServiceData: + """Class for storing notify service data. + + service + The NotifyService + service_name + The name of the notify service. + target_service_name_prefix + The prefix used to create new service for tagets. + targets + A dict of targets index by service names. + """ + + service: BaseNotificationService + service_name: str + target_service_name_prefix: str + targets: Dict def _async_integration_has_notify_services(hass, integration_name): @@ -161,19 +274,16 @@ async def async_setup_platform( conf_name = p_config.get(CONF_NAME) or discovery_info.get(CONF_NAME) target_service_name_prefix = conf_name or integration_name service_name = conf_name or SERVICE_NOTIFY + targets = {} - data = { - SERVICE_NAME: slugify(service_name), - # The targets use a slightly different friendly name - # selection pattern than the base service - TARGET_SERVICE_NAME_PREFIX: target_service_name_prefix, - SERVICE: notify_service, - TARGETS: {}, - } - hass.data[NOTIFY_SERVICES].setdefault(integration_name, []) - hass.data[NOTIFY_SERVICES][integration_name].append(data) + data = NotifyServiceData( + notify_service, slugify(service_name), target_service_name_prefix, targets + ) + hass.data[NOTIFY_SERVICES].setdefault(integration_name, []).append(data) - await _async_register_from_notify_data(data) + await notify_service.async_register_services( + service_name, target_service_name_prefix, targets + ) hass.config.components.add(f"{DOMAIN}.{integration_name}") @@ -194,107 +304,3 @@ async def async_platform_discovered(platform, info): discovery.async_listen_platform(hass, DOMAIN, async_platform_discovered) return True - - -async def _async_notify_message_service(hass, service, notify_service, targets): - """Handle sending notification message service calls.""" - kwargs = {} - message = service.data[ATTR_MESSAGE] - title = service.data.get(ATTR_TITLE) - - if title: - title.hass = hass - kwargs[ATTR_TITLE] = title.async_render() - - if targets.get(service.service) is not None: - kwargs[ATTR_TARGET] = [targets[service.service]] - elif service.data.get(ATTR_TARGET) is not None: - kwargs[ATTR_TARGET] = service.data.get(ATTR_TARGET) - - message.hass = hass - kwargs[ATTR_MESSAGE] = message.async_render() - kwargs[ATTR_DATA] = service.data.get(ATTR_DATA) - - await notify_service.async_send_message(**kwargs) - - -class BaseNotificationService: - """An abstract class for notification services.""" - - hass: Optional[HomeAssistantType] = None - - def send_message(self, message, **kwargs): - """Send a message. - - kwargs can contain ATTR_TITLE to specify a title. - """ - raise NotImplementedError() - - async def async_send_message(self, message, **kwargs): - """Send a message. - - kwargs can contain ATTR_TITLE to specify a title. - """ - await self.hass.async_add_job(partial(self.send_message, message, **kwargs)) - - async def async_register_services( - self, service_name, target_service_name_prefix, targets - ): - """Create or remove the notify services.""" - - async def _async_notify_message(service): - """Handle sending notification message service calls.""" - await _async_notify_message_service(self.hass, service, self, targets) - - if hasattr(self, "targets"): - stale_targets = set(targets) - - for name, target in self.targets.items(): # pylint: disable=no-member - target_name = slugify(f"{target_service_name_prefix}_{name}") - if target_name in stale_targets: - stale_targets.remove(target_name) - if target_name in targets: - continue - targets[target_name] = target - self.hass.services.async_register( - DOMAIN, - target_name, - _async_notify_message, - schema=NOTIFY_SERVICE_SCHEMA, - ) - - for stale_target_name in stale_targets: - del targets[stale_target_name] - self.hass.services.async_remove( - DOMAIN, - stale_target_name, - ) - - if self.hass.services.has_service(DOMAIN, service_name): - return - - self.hass.services.async_register( - DOMAIN, - service_name, - _async_notify_message, - schema=NOTIFY_SERVICE_SCHEMA, - ) - - async def async_unregister_services(self, service_name, targets): - """Unregister the notify services.""" - if targets: - remove_targets = set(targets) - for remove_target_name in remove_targets: - del targets[remove_target_name] - self.hass.services.async_remove( - DOMAIN, - remove_target_name, - ) - - if not self.hass.services.has_service(DOMAIN, service_name): - return - - self.hass.services.async_remove( - DOMAIN, - service_name, - ) From 0674fa275aaea4b742cdab3b3ea36a26aed569a6 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 2 Sep 2020 11:59:32 -0500 Subject: [PATCH 09/20] refactor again --- homeassistant/components/notify/__init__.py | 22 ++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/homeassistant/components/notify/__init__.py b/homeassistant/components/notify/__init__.py index f032659081e5a4..a846b9c4d44f90 100644 --- a/homeassistant/components/notify/__init__.py +++ b/homeassistant/components/notify/__init__.py @@ -86,6 +86,17 @@ async def async_reset_platform(hass, integration_name): del hass.data[NOTIFY_SERVICES][integration_name] +def _async_integration_has_notify_services(hass, integration_name): + """Determine if an integration has notify services registered.""" + if ( + NOTIFY_SERVICES not in hass.data + or integration_name not in hass.data[NOTIFY_SERVICES] + ): + return False + + return True + + class BaseNotificationService: """An abstract class for notification services.""" @@ -209,17 +220,6 @@ class NotifyServiceData: targets: Dict -def _async_integration_has_notify_services(hass, integration_name): - """Call a method on all notify services for an integration.""" - if ( - NOTIFY_SERVICES not in hass.data - or integration_name not in hass.data[NOTIFY_SERVICES] - ): - return False - - return True - - async def async_setup(hass, config): """Set up the notify services.""" hass.data.setdefault(NOTIFY_SERVICES, {}) From c80b0976d651e5fb158944a208a5402b41bd5400 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 2 Sep 2020 12:00:20 -0500 Subject: [PATCH 10/20] tweaks --- homeassistant/components/notify/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/notify/__init__.py b/homeassistant/components/notify/__init__.py index a846b9c4d44f90..71d39eafbd1326 100644 --- a/homeassistant/components/notify/__init__.py +++ b/homeassistant/components/notify/__init__.py @@ -211,7 +211,7 @@ class NotifyServiceData: target_service_name_prefix The prefix used to create new service for tagets. targets - A dict of targets index by service names. + A dict of targets indexed by service names. """ service: BaseNotificationService From 4397a8c7f8ebc025ebececb0f648137b3023ac04 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 2 Sep 2020 12:02:08 -0500 Subject: [PATCH 11/20] tweaks --- homeassistant/components/notify/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/notify/__init__.py b/homeassistant/components/notify/__init__.py index 71d39eafbd1326..bc36c208d84835 100644 --- a/homeassistant/components/notify/__init__.py +++ b/homeassistant/components/notify/__init__.py @@ -205,7 +205,7 @@ class NotifyServiceData: """Class for storing notify service data. service - The NotifyService + The NotificationService service_name The name of the notify service. target_service_name_prefix From 946d38eb89429e5b1a7de8a45c87ed23a06488e4 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 2 Sep 2020 12:13:44 -0500 Subject: [PATCH 12/20] typing --- homeassistant/components/notify/__init__.py | 31 +++++++++++++-------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/homeassistant/components/notify/__init__.py b/homeassistant/components/notify/__init__.py index bc36c208d84835..af362f42ee2442 100644 --- a/homeassistant/components/notify/__init__.py +++ b/homeassistant/components/notify/__init__.py @@ -3,11 +3,12 @@ from dataclasses import dataclass from functools import partial import logging -from typing import Dict, Optional +from typing import Any, Dict, Optional import voluptuous as vol from homeassistant.const import CONF_NAME, CONF_PLATFORM +from homeassistant.core import ServiceCall from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers import config_per_platform, discovery import homeassistant.helpers.config_validation as cv @@ -55,7 +56,7 @@ @bind_hass -async def async_reload(hass, integration_name): +async def async_reload(hass: HomeAssistantType, integration_name: str) -> None: """Register notify services for an integration.""" if not _async_integration_has_notify_services(hass, integration_name): return @@ -71,7 +72,7 @@ async def async_reload(hass, integration_name): @bind_hass -async def async_reset_platform(hass, integration_name): +async def async_reset_platform(hass: HomeAssistantType, integration_name: str) -> None: """Unregister notify services for an integration.""" if not _async_integration_has_notify_services(hass, integration_name): return @@ -86,7 +87,9 @@ async def async_reset_platform(hass, integration_name): del hass.data[NOTIFY_SERVICES][integration_name] -def _async_integration_has_notify_services(hass, integration_name): +def _async_integration_has_notify_services( + hass: HomeAssistantType, integration_name: str +) -> bool: """Determine if an integration has notify services registered.""" if ( NOTIFY_SERVICES not in hass.data @@ -109,14 +112,16 @@ def send_message(self, message, **kwargs): """ raise NotImplementedError() - async def async_send_message(self, message, **kwargs): + async def async_send_message(self, message: Any, **kwargs: Any) -> None: """Send a message. kwargs can contain ATTR_TITLE to specify a title. """ - await self.hass.async_add_job(partial(self.send_message, message, **kwargs)) + await self.hass.async_add_job(partial(self.send_message, message, **kwargs)) # type: ignore - async def _async_notify_message_service(self, service, targets): + async def _async_notify_message_service( + self, service: ServiceCall, targets: Dict + ) -> None: """Handle sending notification message service calls.""" kwargs = {} message = service.data[ATTR_MESSAGE] @@ -138,9 +143,10 @@ async def _async_notify_message_service(self, service, targets): await self.async_send_message(**kwargs) async def async_register_services( - self, service_name, target_service_name_prefix, targets - ): + self, service_name: str, target_service_name_prefix: str, targets: Dict + ) -> None: """Create or remove the notify services.""" + assert self.hass async def _async_notify_message(service): """Handle sending notification message service calls.""" @@ -149,7 +155,8 @@ async def _async_notify_message(service): if hasattr(self, "targets"): stale_targets = set(targets) - for name, target in self.targets.items(): # pylint: disable=no-member + # pylint: disable=no-member + for name, target in self.targets.items(): # type: ignore target_name = slugify(f"{target_service_name_prefix}_{name}") if target_name in stale_targets: stale_targets.remove(target_name) @@ -180,8 +187,10 @@ async def _async_notify_message(service): schema=NOTIFY_SERVICE_SCHEMA, ) - async def async_unregister_services(self, service_name, targets): + async def async_unregister_services(self, service_name: str, targets: Dict) -> None: """Unregister the notify services.""" + assert self.hass + if targets: remove_targets = set(targets) for remove_target_name in remove_targets: From c6d7e0c32c4b3798d836df9134ab218ec0905f5f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 2 Sep 2020 12:15:46 -0500 Subject: [PATCH 13/20] fix refactoring error --- homeassistant/components/notify/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/notify/__init__.py b/homeassistant/components/notify/__init__.py index af362f42ee2442..4d137f6895c920 100644 --- a/homeassistant/components/notify/__init__.py +++ b/homeassistant/components/notify/__init__.py @@ -282,11 +282,11 @@ async def async_setup_platform( conf_name = p_config.get(CONF_NAME) or discovery_info.get(CONF_NAME) target_service_name_prefix = conf_name or integration_name - service_name = conf_name or SERVICE_NOTIFY + service_name = slugify(conf_name or SERVICE_NOTIFY) targets = {} data = NotifyServiceData( - notify_service, slugify(service_name), target_service_name_prefix, targets + notify_service, service_name, target_service_name_prefix, targets ) hass.data[NOTIFY_SERVICES].setdefault(integration_name, []).append(data) From 732ec8e8fc01ee3c1fbde2faa522c5c3a3810087 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 2 Sep 2020 12:29:28 -0500 Subject: [PATCH 14/20] remove the dataclass --- homeassistant/components/notify/__init__.py | 96 ++++++++------------- 1 file changed, 36 insertions(+), 60 deletions(-) diff --git a/homeassistant/components/notify/__init__.py b/homeassistant/components/notify/__init__.py index 4d137f6895c920..cfe7fcba0e2d5e 100644 --- a/homeassistant/components/notify/__init__.py +++ b/homeassistant/components/notify/__init__.py @@ -1,6 +1,5 @@ """Provides functionality to notify people.""" import asyncio -from dataclasses import dataclass from functools import partial import logging from typing import Any, Dict, Optional @@ -62,10 +61,8 @@ async def async_reload(hass: HomeAssistantType, integration_name: str) -> None: return tasks = [ - data.service.async_register_services( - data.service_name, data.target_service_name_prefix, data.targets - ) - for data in hass.data[NOTIFY_SERVICES][integration_name] + notify_service.async_register_services() + for notify_service in hass.data[NOTIFY_SERVICES][integration_name] ] await asyncio.gather(*tasks) @@ -78,8 +75,8 @@ async def async_reset_platform(hass: HomeAssistantType, integration_name: str) - return tasks = [ - data.service.async_unregister_services(data.service_name, data.targets) - for data in hass.data[NOTIFY_SERVICES][integration_name] + notify_service.async_unregister_services() + for notify_service in hass.data[NOTIFY_SERVICES][integration_name] ] await asyncio.gather(*tasks) @@ -119,9 +116,7 @@ async def async_send_message(self, message: Any, **kwargs: Any) -> None: """ await self.hass.async_add_job(partial(self.send_message, message, **kwargs)) # type: ignore - async def _async_notify_message_service( - self, service: ServiceCall, targets: Dict - ) -> None: + async def _async_notify_message_service(self, service: ServiceCall) -> None: """Handle sending notification message service calls.""" kwargs = {} message = service.data[ATTR_MESSAGE] @@ -131,8 +126,8 @@ async def _async_notify_message_service( title.hass = self.hass kwargs[ATTR_TITLE] = title.async_render() - if targets.get(service.service) is not None: - kwargs[ATTR_TARGET] = [targets[service.service]] + if self._targets.get(service.service) is not None: + kwargs[ATTR_TARGET] = [self._targets[service.service]] elif service.data.get(ATTR_TARGET) is not None: kwargs[ATTR_TARGET] = service.data.get(ATTR_TARGET) @@ -142,93 +137,76 @@ async def _async_notify_message_service( await self.async_send_message(**kwargs) - async def async_register_services( - self, service_name: str, target_service_name_prefix: str, targets: Dict + async def async_setup( + self, service_name: str, target_service_name_prefix: str ) -> None: + """Store the data for the notify service.""" + # pylint: disable=attribute-defined-outside-init + self._service_name = service_name + self._target_service_name_prefix = target_service_name_prefix + self._targets: Dict = {} + + async def async_register_services(self) -> None: """Create or remove the notify services.""" assert self.hass - async def _async_notify_message(service): - """Handle sending notification message service calls.""" - await self._async_notify_message_service(service, targets) - if hasattr(self, "targets"): - stale_targets = set(targets) + stale_targets = set(self._targets) # pylint: disable=no-member for name, target in self.targets.items(): # type: ignore - target_name = slugify(f"{target_service_name_prefix}_{name}") + target_name = slugify(f"{self._target_service_name_prefix}_{name}") if target_name in stale_targets: stale_targets.remove(target_name) - if target_name in targets: + if target_name in self._targets: continue - targets[target_name] = target + self._targets[target_name] = target self.hass.services.async_register( DOMAIN, target_name, - _async_notify_message, + self._async_notify_message_service, schema=NOTIFY_SERVICE_SCHEMA, ) for stale_target_name in stale_targets: - del targets[stale_target_name] + del self._targets[stale_target_name] self.hass.services.async_remove( DOMAIN, stale_target_name, ) - if self.hass.services.has_service(DOMAIN, service_name): + if self.hass.services.has_service(DOMAIN, self._service_name): return self.hass.services.async_register( DOMAIN, - service_name, - _async_notify_message, + self._service_name, + self._async_notify_message_service, schema=NOTIFY_SERVICE_SCHEMA, ) - async def async_unregister_services(self, service_name: str, targets: Dict) -> None: + async def async_unregister_services(self) -> None: """Unregister the notify services.""" assert self.hass - if targets: - remove_targets = set(targets) + if self._targets: + remove_targets = set(self._targets) for remove_target_name in remove_targets: - del targets[remove_target_name] + del self._targets[remove_target_name] self.hass.services.async_remove( DOMAIN, remove_target_name, ) - if not self.hass.services.has_service(DOMAIN, service_name): + if not self.hass.services.has_service(DOMAIN, self._service_name): return self.hass.services.async_remove( DOMAIN, - service_name, + self._service_name, ) -@dataclass -class NotifyServiceData: - """Class for storing notify service data. - - service - The NotificationService - service_name - The name of the notify service. - target_service_name_prefix - The prefix used to create new service for tagets. - targets - A dict of targets indexed by service names. - """ - - service: BaseNotificationService - service_name: str - target_service_name_prefix: str - targets: Dict - - async def async_setup(hass, config): """Set up the notify services.""" hass.data.setdefault(NOTIFY_SERVICES, {}) @@ -283,17 +261,15 @@ async def async_setup_platform( conf_name = p_config.get(CONF_NAME) or discovery_info.get(CONF_NAME) target_service_name_prefix = conf_name or integration_name service_name = slugify(conf_name or SERVICE_NOTIFY) - targets = {} - data = NotifyServiceData( - notify_service, service_name, target_service_name_prefix, targets - ) - hass.data[NOTIFY_SERVICES].setdefault(integration_name, []).append(data) + await notify_service.async_setup(service_name, target_service_name_prefix) - await notify_service.async_register_services( - service_name, target_service_name_prefix, targets + hass.data[NOTIFY_SERVICES].setdefault(integration_name, []).append( + notify_service ) + await notify_service.async_register_services() + hass.config.components.add(f"{DOMAIN}.{integration_name}") return True From c99137ab1be78c630211582f5405108d060a34f2 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 2 Sep 2020 12:33:09 -0500 Subject: [PATCH 15/20] naming --- homeassistant/components/notify/__init__.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/homeassistant/components/notify/__init__.py b/homeassistant/components/notify/__init__.py index cfe7fcba0e2d5e..48bf6ff79f2a3b 100644 --- a/homeassistant/components/notify/__init__.py +++ b/homeassistant/components/notify/__init__.py @@ -126,8 +126,8 @@ async def _async_notify_message_service(self, service: ServiceCall) -> None: title.hass = self.hass kwargs[ATTR_TITLE] = title.async_render() - if self._targets.get(service.service) is not None: - kwargs[ATTR_TARGET] = [self._targets[service.service]] + if self._registered_targets.get(service.service) is not None: + kwargs[ATTR_TARGET] = [self._registered_targets[service.service]] elif service.data.get(ATTR_TARGET) is not None: kwargs[ATTR_TARGET] = service.data.get(ATTR_TARGET) @@ -144,23 +144,23 @@ async def async_setup( # pylint: disable=attribute-defined-outside-init self._service_name = service_name self._target_service_name_prefix = target_service_name_prefix - self._targets: Dict = {} + self._registered_targets: Dict = {} async def async_register_services(self) -> None: """Create or remove the notify services.""" assert self.hass if hasattr(self, "targets"): - stale_targets = set(self._targets) + stale_targets = set(self._registered_targets) # pylint: disable=no-member for name, target in self.targets.items(): # type: ignore target_name = slugify(f"{self._target_service_name_prefix}_{name}") if target_name in stale_targets: stale_targets.remove(target_name) - if target_name in self._targets: + if target_name in self._registered_targets: continue - self._targets[target_name] = target + self._registered_targets[target_name] = target self.hass.services.async_register( DOMAIN, target_name, @@ -169,7 +169,7 @@ async def async_register_services(self) -> None: ) for stale_target_name in stale_targets: - del self._targets[stale_target_name] + del self._registered_targets[stale_target_name] self.hass.services.async_remove( DOMAIN, stale_target_name, @@ -189,10 +189,10 @@ async def async_unregister_services(self) -> None: """Unregister the notify services.""" assert self.hass - if self._targets: - remove_targets = set(self._targets) + if self._registered_targets: + remove_targets = set(self._registered_targets) for remove_target_name in remove_targets: - del self._targets[remove_target_name] + del self._registered_targets[remove_target_name] self.hass.services.async_remove( DOMAIN, remove_target_name, From a4088a5f1194fd0f9f7c116c13422090ca2b48d4 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 2 Sep 2020 12:33:48 -0500 Subject: [PATCH 16/20] naming --- homeassistant/components/notify/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/notify/__init__.py b/homeassistant/components/notify/__init__.py index 48bf6ff79f2a3b..af67e632cb11b1 100644 --- a/homeassistant/components/notify/__init__.py +++ b/homeassistant/components/notify/__init__.py @@ -147,7 +147,7 @@ async def async_setup( self._registered_targets: Dict = {} async def async_register_services(self) -> None: - """Create or remove the notify services.""" + """Create or update the notify services.""" assert self.hass if hasattr(self, "targets"): From ccb3f22df2b5b33ad19bd25c8ed1308990dea1bf Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 2 Sep 2020 13:38:53 -0500 Subject: [PATCH 17/20] workaround integrations not calling super().__init__ --- homeassistant/components/notify/__init__.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/notify/__init__.py b/homeassistant/components/notify/__init__.py index af67e632cb11b1..ca78737e11e746 100644 --- a/homeassistant/components/notify/__init__.py +++ b/homeassistant/components/notify/__init__.py @@ -102,6 +102,12 @@ class BaseNotificationService: hass: Optional[HomeAssistantType] = None + def __init__(self): + """Init the notification service.""" + self._service_name = None + self._target_service_name_prefix = None + self._registered_targets: Dict = {} + def send_message(self, message, **kwargs): """Send a message. @@ -141,10 +147,9 @@ async def async_setup( self, service_name: str, target_service_name_prefix: str ) -> None: """Store the data for the notify service.""" - # pylint: disable=attribute-defined-outside-init self._service_name = service_name self._target_service_name_prefix = target_service_name_prefix - self._registered_targets: Dict = {} + self._registered_targets = {} async def async_register_services(self) -> None: """Create or update the notify services.""" From f21d1649b574783c26ea9c0bf1024ef7a6a2001a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 2 Sep 2020 13:40:18 -0500 Subject: [PATCH 18/20] reorder --- homeassistant/components/notify/__init__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/homeassistant/components/notify/__init__.py b/homeassistant/components/notify/__init__.py index ca78737e11e746..9c19a8ecbab81a 100644 --- a/homeassistant/components/notify/__init__.py +++ b/homeassistant/components/notify/__init__.py @@ -268,13 +268,11 @@ async def async_setup_platform( service_name = slugify(conf_name or SERVICE_NOTIFY) await notify_service.async_setup(service_name, target_service_name_prefix) + await notify_service.async_register_services() hass.data[NOTIFY_SERVICES].setdefault(integration_name, []).append( notify_service ) - - await notify_service.async_register_services() - hass.config.components.add(f"{DOMAIN}.{integration_name}") return True From bb0817dc72e146f2d485737b0932f81f65a6f2a3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 2 Sep 2020 13:47:49 -0500 Subject: [PATCH 19/20] clenaup --- homeassistant/components/notify/__init__.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/homeassistant/components/notify/__init__.py b/homeassistant/components/notify/__init__.py index 9c19a8ecbab81a..9aca091ff44317 100644 --- a/homeassistant/components/notify/__init__.py +++ b/homeassistant/components/notify/__init__.py @@ -144,9 +144,13 @@ async def _async_notify_message_service(self, service: ServiceCall) -> None: await self.async_send_message(**kwargs) async def async_setup( - self, service_name: str, target_service_name_prefix: str + self, + hass: HomeAssistantType, + service_name: str, + target_service_name_prefix: str, ) -> None: """Store the data for the notify service.""" + self.hass = hass self._service_name = service_name self._target_service_name_prefix = target_service_name_prefix self._registered_targets = {} @@ -258,8 +262,6 @@ async def async_setup_platform( _LOGGER.exception("Error setting up platform %s", integration_name) return - notify_service.hass = hass - if discovery_info is None: discovery_info = {} @@ -267,7 +269,7 @@ async def async_setup_platform( target_service_name_prefix = conf_name or integration_name service_name = slugify(conf_name or SERVICE_NOTIFY) - await notify_service.async_setup(service_name, target_service_name_prefix) + await notify_service.async_setup(hass, service_name, target_service_name_prefix) await notify_service.async_register_services() hass.data[NOTIFY_SERVICES].setdefault(integration_name, []).append( From 58ffc8a07787bbc3e04c7f161a420dcb9b960087 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 2 Sep 2020 14:09:44 -0500 Subject: [PATCH 20/20] Remove workaround --- homeassistant/components/notify/__init__.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/homeassistant/components/notify/__init__.py b/homeassistant/components/notify/__init__.py index 9aca091ff44317..9a4ec681aabfad 100644 --- a/homeassistant/components/notify/__init__.py +++ b/homeassistant/components/notify/__init__.py @@ -102,12 +102,6 @@ class BaseNotificationService: hass: Optional[HomeAssistantType] = None - def __init__(self): - """Init the notification service.""" - self._service_name = None - self._target_service_name_prefix = None - self._registered_targets: Dict = {} - def send_message(self, message, **kwargs): """Send a message. @@ -150,10 +144,11 @@ async def async_setup( target_service_name_prefix: str, ) -> None: """Store the data for the notify service.""" + # pylint: disable=attribute-defined-outside-init self.hass = hass self._service_name = service_name self._target_service_name_prefix = target_service_name_prefix - self._registered_targets = {} + self._registered_targets: Dict = {} async def async_register_services(self) -> None: """Create or update the notify services."""