From 3289db6056e94044cd19e2892b6ab8c51fc12461 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Sat, 8 Feb 2020 22:33:57 -0800 Subject: [PATCH 1/3] Clean up frontend services and events --- homeassistant/components/frontend/__init__.py | 54 ++++++--------- homeassistant/helpers/functools.py | 25 +++++++ homeassistant/helpers/service.py | 8 ++- tests/helpers/test_functools.py | 69 +++++++++++++++++++ 4 files changed, 121 insertions(+), 35 deletions(-) create mode 100644 homeassistant/helpers/functools.py create mode 100644 tests/helpers/test_functools.py diff --git a/homeassistant/components/frontend/__init__.py b/homeassistant/components/frontend/__init__.py index fdea21fe91e861..b26d7a4e1689aa 100644 --- a/homeassistant/components/frontend/__init__.py +++ b/homeassistant/components/frontend/__init__.py @@ -16,6 +16,7 @@ from homeassistant.config import async_hass_config_yaml from homeassistant.const import CONF_NAME, EVENT_THEMES_UPDATED from homeassistant.core import callback +from homeassistant.helpers import service import homeassistant.helpers.config_validation as cv from homeassistant.helpers.translation import async_get_translations from homeassistant.loader import bind_hass @@ -103,19 +104,6 @@ SERVICE_SET_THEME = "set_theme" SERVICE_RELOAD_THEMES = "reload_themes" -SERVICE_SET_THEME_SCHEMA = vol.Schema({vol.Required(CONF_NAME): cv.string}) -WS_TYPE_GET_PANELS = "get_panels" -SCHEMA_GET_PANELS = websocket_api.BASE_COMMAND_MESSAGE_SCHEMA.extend( - {vol.Required("type"): WS_TYPE_GET_PANELS} -) -WS_TYPE_GET_THEMES = "frontend/get_themes" -SCHEMA_GET_THEMES = websocket_api.BASE_COMMAND_MESSAGE_SCHEMA.extend( - {vol.Required("type"): WS_TYPE_GET_THEMES} -) -WS_TYPE_GET_TRANSLATIONS = "frontend/get_translations" -SCHEMA_GET_TRANSLATIONS = websocket_api.BASE_COMMAND_MESSAGE_SCHEMA.extend( - {vol.Required("type"): WS_TYPE_GET_TRANSLATIONS, vol.Required("language"): str} -) class Panel: @@ -251,15 +239,9 @@ def _frontend_root(dev_repo_path): async def async_setup(hass, config): """Set up the serving of the frontend.""" await async_setup_frontend_storage(hass) - hass.components.websocket_api.async_register_command( - WS_TYPE_GET_PANELS, websocket_get_panels, SCHEMA_GET_PANELS - ) - hass.components.websocket_api.async_register_command( - WS_TYPE_GET_THEMES, websocket_get_themes, SCHEMA_GET_THEMES - ) - hass.components.websocket_api.async_register_command( - WS_TYPE_GET_TRANSLATIONS, websocket_get_translations, SCHEMA_GET_TRANSLATIONS - ) + hass.components.websocket_api.async_register_command(websocket_get_panels) + hass.components.websocket_api.async_register_command(websocket_get_themes) + hass.components.websocket_api.async_register_command(websocket_get_translations) hass.http.register_view(ManifestJSONView) conf = config.get(DOMAIN, {}) @@ -331,11 +313,7 @@ async def async_setup(hass, config): def _async_setup_themes(hass, themes): """Set up themes data and services.""" hass.data[DATA_DEFAULT_THEME] = DEFAULT_THEME - if themes is None: - hass.data[DATA_THEMES] = {} - return - - hass.data[DATA_THEMES] = themes + hass.data[DATA_THEMES] = themes or {} @callback def update_theme_and_fire_event(): @@ -348,9 +326,7 @@ def update_theme_and_fire_event(): "app-header-background-color", themes[name].get(PRIMARY_COLOR, DEFAULT_THEME_COLOR), ) - hass.bus.async_fire( - EVENT_THEMES_UPDATED, {"themes": themes, "default_theme": name} - ) + hass.bus.async_fire(EVENT_THEMES_UPDATED) @callback def set_theme(call): @@ -373,10 +349,17 @@ async def reload_themes(_): hass.data[DATA_DEFAULT_THEME] = DEFAULT_THEME update_theme_and_fire_event() - hass.services.async_register( - DOMAIN, SERVICE_SET_THEME, set_theme, schema=SERVICE_SET_THEME_SCHEMA + service.async_register_admin_service( + hass, + DOMAIN, + SERVICE_SET_THEME, + set_theme, + vol.Schema({vol.Required(CONF_NAME): cv.string}), + ) + + service.async_register_admin_service( + hass, DOMAIN, SERVICE_RELOAD_THEMES, reload_themes ) - hass.services.async_register(DOMAIN, SERVICE_RELOAD_THEMES, reload_themes) class IndexView(web_urldispatcher.AbstractResource): @@ -498,6 +481,7 @@ def get(self, request): # pylint: disable=no-self-use @callback +@websocket_api.websocket_command({"type": "get_panels"}) def websocket_get_panels(hass, connection, msg): """Handle get panels command. @@ -514,6 +498,7 @@ def websocket_get_panels(hass, connection, msg): @callback +@websocket_api.websocket_command({"type": "frontend/get_themes"}) def websocket_get_themes(hass, connection, msg): """Handle get themes command. @@ -530,6 +515,9 @@ def websocket_get_themes(hass, connection, msg): ) +@websocket_api.websocket_command( + {"type": "frontend/get_translations", vol.Required("language"): str} +) @websocket_api.async_response async def websocket_get_translations(hass, connection, msg): """Handle get translations command. diff --git a/homeassistant/helpers/functools.py b/homeassistant/helpers/functools.py new file mode 100644 index 00000000000000..d544a1cbcceb16 --- /dev/null +++ b/homeassistant/helpers/functools.py @@ -0,0 +1,25 @@ +"""Functools that are HA aware.""" +import functools +from typing import Callable + +from homeassistant.core import is_callback + + +def wraps(wrapped_func: Callable) -> Callable: + """Decorate function to mimic wrapped function. + + Wraps copies all values set on a function. This can cause the callback decorator + to be accidentally added to non-callback functions. + """ + + def wrap(to_wrap_func: Callable) -> Callable: + """Wrap a function.""" + remove_callback = is_callback(wrapped_func) and not is_callback(to_wrap_func) + wrapped = functools.wraps(wrapped_func)(to_wrap_func) + + if remove_callback: + setattr(wrapped, "_hass_callback", False) + + return wrapped + + return wrap diff --git a/homeassistant/helpers/service.py b/homeassistant/helpers/service.py index 46ebc467c0b232..fd2f820a47fad5 100644 --- a/homeassistant/helpers/service.py +++ b/homeassistant/helpers/service.py @@ -1,6 +1,6 @@ """Service calling related helpers.""" import asyncio -from functools import partial, wraps +from functools import partial import logging from typing import Callable @@ -27,6 +27,8 @@ from homeassistant.util.yaml import load_yaml from homeassistant.util.yaml.loader import JSON_TYPE +from .functools import wraps + # mypy: allow-untyped-defs, no-check-untyped-defs CONF_SERVICE = "service" @@ -461,7 +463,9 @@ async def admin_handler(call): if not user.is_admin: raise Unauthorized(context=call.context) - await hass.async_add_job(service_func, call) + result = hass.async_add_job(service_func, call) + if result is not None: + await result hass.services.async_register(domain, service, admin_handler, schema) diff --git a/tests/helpers/test_functools.py b/tests/helpers/test_functools.py new file mode 100644 index 00000000000000..35421b33f6fb3c --- /dev/null +++ b/tests/helpers/test_functools.py @@ -0,0 +1,69 @@ +"""Tests for HA functools.""" + +from homeassistant.core import callback, is_callback +from homeassistant.helpers import functools + + +def test_wraps_wrap_not_callback(): + """Test wraps function.""" + + def to_wrap(): + pass + + to_wrap.hello = True + + @functools.wraps(to_wrap) + def wrapped(): + pass + + assert wrapped.hello is True + assert not is_callback(wrapped) + + @callback + @functools.wraps(to_wrap) + def wrapped(): + pass + + assert wrapped.hello is True + assert is_callback(wrapped) + + @functools.wraps(to_wrap) + @callback + def wrapped(): + pass + + assert wrapped.hello is True + assert is_callback(wrapped) + + +def test_wraps_callback(): + """Test wraps function.""" + + @callback + def to_wrap(): + pass + + to_wrap.hello = True + + @functools.wraps(to_wrap) + def wrapped(): + pass + + assert wrapped.hello is True + assert not is_callback(wrapped) + + @callback + @functools.wraps(to_wrap) + def wrapped(): + pass + + assert wrapped.hello is True + assert is_callback(wrapped) + + @functools.wraps(to_wrap) + @callback + def wrapped(): + pass + + assert wrapped.hello is True + assert is_callback(wrapped) From 1ebe0a22a87d99fdc4fb8e43d641b4433c6fd161 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Sat, 8 Feb 2020 22:51:49 -0800 Subject: [PATCH 2/3] Fix bug in core instead --- homeassistant/core.py | 16 ++++--- homeassistant/helpers/functools.py | 25 ----------- homeassistant/helpers/service.py | 4 +- tests/helpers/test_functools.py | 69 ------------------------------ 4 files changed, 11 insertions(+), 103 deletions(-) delete mode 100644 homeassistant/helpers/functools.py delete mode 100644 tests/helpers/test_functools.py diff --git a/homeassistant/core.py b/homeassistant/core.py index 3f561cdfab8206..e819a32b7c7a09 100644 --- a/homeassistant/core.py +++ b/homeassistant/core.py @@ -298,10 +298,10 @@ def async_add_job( if asyncio.iscoroutine(check_target): task = self.loop.create_task(target) # type: ignore - elif is_callback(check_target): - self.loop.call_soon(target, *args) elif asyncio.iscoroutinefunction(check_target): task = self.loop.create_task(target(*args)) + elif is_callback(check_target): + self.loop.call_soon(target, *args) else: task = self.loop.run_in_executor( # type: ignore None, target, *args @@ -360,7 +360,11 @@ def async_run_job(self, target: Callable[..., None], *args: Any) -> None: target: target to call. args: parameters for method to call. """ - if not asyncio.iscoroutine(target) and is_callback(target): + if ( + not asyncio.iscoroutine(target) + and not asyncio.iscoroutinefunction(target) + and is_callback(target) + ): target(*args) else: self.async_add_job(target, *args) @@ -1245,10 +1249,10 @@ async def _execute_service( self, handler: Service, service_call: ServiceCall ) -> None: """Execute a service.""" - if handler.is_callback: - handler.func(service_call) - elif handler.is_coroutinefunction: + if handler.is_coroutinefunction: await handler.func(service_call) + elif handler.is_callback: + handler.func(service_call) else: await self._hass.async_add_executor_job(handler.func, service_call) diff --git a/homeassistant/helpers/functools.py b/homeassistant/helpers/functools.py deleted file mode 100644 index d544a1cbcceb16..00000000000000 --- a/homeassistant/helpers/functools.py +++ /dev/null @@ -1,25 +0,0 @@ -"""Functools that are HA aware.""" -import functools -from typing import Callable - -from homeassistant.core import is_callback - - -def wraps(wrapped_func: Callable) -> Callable: - """Decorate function to mimic wrapped function. - - Wraps copies all values set on a function. This can cause the callback decorator - to be accidentally added to non-callback functions. - """ - - def wrap(to_wrap_func: Callable) -> Callable: - """Wrap a function.""" - remove_callback = is_callback(wrapped_func) and not is_callback(to_wrap_func) - wrapped = functools.wraps(wrapped_func)(to_wrap_func) - - if remove_callback: - setattr(wrapped, "_hass_callback", False) - - return wrapped - - return wrap diff --git a/homeassistant/helpers/service.py b/homeassistant/helpers/service.py index fd2f820a47fad5..9085c929651dc1 100644 --- a/homeassistant/helpers/service.py +++ b/homeassistant/helpers/service.py @@ -1,6 +1,6 @@ """Service calling related helpers.""" import asyncio -from functools import partial +from functools import partial, wraps import logging from typing import Callable @@ -27,8 +27,6 @@ from homeassistant.util.yaml import load_yaml from homeassistant.util.yaml.loader import JSON_TYPE -from .functools import wraps - # mypy: allow-untyped-defs, no-check-untyped-defs CONF_SERVICE = "service" diff --git a/tests/helpers/test_functools.py b/tests/helpers/test_functools.py deleted file mode 100644 index 35421b33f6fb3c..00000000000000 --- a/tests/helpers/test_functools.py +++ /dev/null @@ -1,69 +0,0 @@ -"""Tests for HA functools.""" - -from homeassistant.core import callback, is_callback -from homeassistant.helpers import functools - - -def test_wraps_wrap_not_callback(): - """Test wraps function.""" - - def to_wrap(): - pass - - to_wrap.hello = True - - @functools.wraps(to_wrap) - def wrapped(): - pass - - assert wrapped.hello is True - assert not is_callback(wrapped) - - @callback - @functools.wraps(to_wrap) - def wrapped(): - pass - - assert wrapped.hello is True - assert is_callback(wrapped) - - @functools.wraps(to_wrap) - @callback - def wrapped(): - pass - - assert wrapped.hello is True - assert is_callback(wrapped) - - -def test_wraps_callback(): - """Test wraps function.""" - - @callback - def to_wrap(): - pass - - to_wrap.hello = True - - @functools.wraps(to_wrap) - def wrapped(): - pass - - assert wrapped.hello is True - assert not is_callback(wrapped) - - @callback - @functools.wraps(to_wrap) - def wrapped(): - pass - - assert wrapped.hello is True - assert is_callback(wrapped) - - @functools.wraps(to_wrap) - @callback - def wrapped(): - pass - - assert wrapped.hello is True - assert is_callback(wrapped) From 3507f2afb7b944d0d190c5151ca164cb2c8a20b9 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Sat, 8 Feb 2020 22:59:29 -0800 Subject: [PATCH 3/3] Add test that core works correctly with callback marked async funcs --- tests/test_core.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/test_core.py b/tests/test_core.py index aa0c615ec04594..657bbeda7c6669 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -1180,3 +1180,28 @@ def test_context(): assert c.user_id == 23 assert c.parent_id == 100 assert c.id is not None + + +async def test_async_functions_with_callback(hass): + """Test we deal with async functions accidentally marked as callback.""" + runs = [] + + @ha.callback + async def test(): + runs.append(True) + + await hass.async_add_job(test) + assert len(runs) == 1 + + hass.async_run_job(test) + await hass.async_block_till_done() + assert len(runs) == 2 + + @ha.callback + async def service_handler(call): + runs.append(True) + + hass.services.async_register("test_domain", "test_service", service_handler) + + await hass.services.async_call("test_domain", "test_service", blocking=True) + assert len(runs) == 3