From f7c4900d5c22da8aba35258a66a89e4e68cc7c60 Mon Sep 17 00:00:00 2001
From: Phil Bruckner <pnbruckner@gmail.com>
Date: Sun, 5 Jul 2020 09:25:15 -0500
Subject: [PATCH] Enhance automation integration to use new features in script
 helper (#37479)

---
 .../components/automation/__init__.py         | 21 ++++--
 homeassistant/components/automation/config.py | 67 +++++++++++++----
 homeassistant/components/script/__init__.py   | 73 +++++++------------
 homeassistant/const.py                        |  1 +
 homeassistant/helpers/script.py               | 41 ++++++++++-
 tests/components/automation/test_init.py      | 32 ++++++++
 6 files changed, 161 insertions(+), 74 deletions(-)

diff --git a/homeassistant/components/automation/__init__.py b/homeassistant/components/automation/__init__.py
index e5f2f611cdb..433742da584 100644
--- a/homeassistant/components/automation/__init__.py
+++ b/homeassistant/components/automation/__init__.py
@@ -9,10 +9,13 @@ import voluptuous as vol
 from homeassistant.const import (
     ATTR_ENTITY_ID,
     ATTR_NAME,
+    CONF_ALIAS,
     CONF_DEVICE_ID,
     CONF_ENTITY_ID,
     CONF_ID,
+    CONF_MODE,
     CONF_PLATFORM,
+    CONF_QUEUE_SIZE,
     CONF_ZONE,
     EVENT_HOMEASSISTANT_STARTED,
     SERVICE_RELOAD,
@@ -23,11 +26,12 @@ from homeassistant.const import (
 )
 from homeassistant.core import Context, CoreState, HomeAssistant, callback
 from homeassistant.exceptions import HomeAssistantError
-from homeassistant.helpers import condition, extract_domain_configs, script
+from homeassistant.helpers import condition, extract_domain_configs
 import homeassistant.helpers.config_validation as cv
 from homeassistant.helpers.entity import ToggleEntity
 from homeassistant.helpers.entity_component import EntityComponent
 from homeassistant.helpers.restore_state import RestoreEntity
+from homeassistant.helpers.script import SCRIPT_BASE_SCHEMA, Script, validate_queue_size
 from homeassistant.helpers.service import async_register_admin_service
 from homeassistant.helpers.typing import TemplateVarsType
 from homeassistant.loader import bind_hass
@@ -41,7 +45,6 @@ ENTITY_ID_FORMAT = DOMAIN + ".{}"
 
 GROUP_NAME_ALL_AUTOMATIONS = "all automations"
 
-CONF_ALIAS = "alias"
 CONF_DESCRIPTION = "description"
 CONF_HIDE_ENTITY = "hide_entity"
 
@@ -96,7 +99,7 @@ _CONDITION_SCHEMA = vol.All(cv.ensure_list, [cv.CONDITION_SCHEMA])
 
 PLATFORM_SCHEMA = vol.All(
     cv.deprecated(CONF_HIDE_ENTITY, invalidation_version="0.110"),
-    vol.Schema(
+    SCRIPT_BASE_SCHEMA.extend(
         {
             # str on purpose
             CONF_ID: str,
@@ -109,6 +112,7 @@ PLATFORM_SCHEMA = vol.All(
             vol.Required(CONF_ACTION): cv.SCRIPT_SCHEMA,
         }
     ),
+    validate_queue_size,
 )
 
 
@@ -389,7 +393,7 @@ class AutomationEntity(ToggleEntity, RestoreEntity):
         try:
             await self.action_script.async_run(variables, trigger_context)
         except Exception:  # pylint: disable=broad-except
-            pass
+            _LOGGER.exception("While executing automation %s", self.entity_id)
 
     async def async_will_remove_from_hass(self):
         """Remove listeners when removing automation from Home Assistant."""
@@ -498,8 +502,13 @@ async def _async_process_config(hass, config, component):
 
             initial_state = config_block.get(CONF_INITIAL_STATE)
 
-            action_script = script.Script(
-                hass, config_block.get(CONF_ACTION, {}), name, logger=_LOGGER
+            action_script = Script(
+                hass,
+                config_block[CONF_ACTION],
+                name,
+                script_mode=config_block[CONF_MODE],
+                queue_size=config_block.get(CONF_QUEUE_SIZE, 0),
+                logger=_LOGGER,
             )
 
             if CONF_CONDITION in config_block:
diff --git a/homeassistant/components/automation/config.py b/homeassistant/components/automation/config.py
index c2cd00fd683..4fa913fc3ce 100644
--- a/homeassistant/components/automation/config.py
+++ b/homeassistant/components/automation/config.py
@@ -1,6 +1,7 @@
 """Config validation helper for the automation integration."""
 import asyncio
 import importlib
+import logging
 
 import voluptuous as vol
 
@@ -8,13 +9,20 @@ from homeassistant.components.device_automation.exceptions import (
     InvalidDeviceAutomationConfig,
 )
 from homeassistant.config import async_log_exception, config_without_domain
-from homeassistant.const import CONF_PLATFORM
+from homeassistant.const import CONF_ALIAS, CONF_ID, CONF_MODE, CONF_PLATFORM
 from homeassistant.exceptions import HomeAssistantError
-from homeassistant.helpers import condition, config_per_platform, script
+from homeassistant.helpers import condition, config_per_platform
+from homeassistant.helpers.script import (
+    SCRIPT_MODE_LEGACY,
+    async_validate_action_config,
+    warn_deprecated_legacy,
+)
 from homeassistant.loader import IntegrationNotFound
 
 from . import CONF_ACTION, CONF_CONDITION, CONF_TRIGGER, DOMAIN, PLATFORM_SCHEMA
 
+_LOGGER = logging.getLogger(__name__)
+
 # mypy: allow-untyped-calls, allow-untyped-defs
 # mypy: no-check-untyped-defs, no-warn-return-any
 
@@ -44,10 +52,7 @@ async def async_validate_config_item(hass, config, full_config=None):
         )
 
     config[CONF_ACTION] = await asyncio.gather(
-        *[
-            script.async_validate_action_config(hass, action)
-            for action in config[CONF_ACTION]
-        ]
+        *[async_validate_action_config(hass, action) for action in config[CONF_ACTION]]
     )
 
     return config
@@ -69,24 +74,54 @@ async def _try_async_validate_config_item(hass, config, full_config=None):
     return config
 
 
+def _deprecated_legacy_mode(config):
+    legacy_names = []
+    legacy_unnamed_found = False
+
+    for cfg in config[DOMAIN]:
+        mode = cfg.get(CONF_MODE)
+        if mode is None:
+            cfg[CONF_MODE] = SCRIPT_MODE_LEGACY
+            name = cfg.get(CONF_ID) or cfg.get(CONF_ALIAS)
+            if name:
+                legacy_names.append(name)
+            else:
+                legacy_unnamed_found = True
+
+    if legacy_names or legacy_unnamed_found:
+        msgs = []
+        if legacy_unnamed_found:
+            msgs.append("unnamed automations")
+        if legacy_names:
+            if len(legacy_names) == 1:
+                base_msg = "this automation"
+            else:
+                base_msg = "these automations"
+            msgs.append(f"{base_msg}: {', '.join(legacy_names)}")
+        warn_deprecated_legacy(_LOGGER, " and ".join(msgs))
+
+    return config
+
+
 async def async_validate_config(hass, config):
     """Validate config."""
-    validated_automations = await asyncio.gather(
-        *(
-            _try_async_validate_config_item(hass, p_config, config)
-            for _, p_config in config_per_platform(config, DOMAIN)
+    automations = list(
+        filter(
+            lambda x: x is not None,
+            await asyncio.gather(
+                *(
+                    _try_async_validate_config_item(hass, p_config, config)
+                    for _, p_config in config_per_platform(config, DOMAIN)
+                )
+            ),
         )
     )
 
-    automations = [
-        validated_automation
-        for validated_automation in validated_automations
-        if validated_automation is not None
-    ]
-
     # Create a copy of the configuration with all config for current
     # component removed and add validated config back in.
     config = config_without_domain(config, DOMAIN)
     config[DOMAIN] = automations
 
+    _deprecated_legacy_mode(config)
+
     return config
diff --git a/homeassistant/components/script/__init__.py b/homeassistant/components/script/__init__.py
index 740a5a21a5f..b97cc17f6a8 100644
--- a/homeassistant/components/script/__init__.py
+++ b/homeassistant/components/script/__init__.py
@@ -11,6 +11,7 @@ from homeassistant.const import (
     CONF_ALIAS,
     CONF_ICON,
     CONF_MODE,
+    CONF_QUEUE_SIZE,
     SERVICE_RELOAD,
     SERVICE_TOGGLE,
     SERVICE_TURN_OFF,
@@ -23,11 +24,11 @@ from homeassistant.helpers.config_validation import make_entity_service_schema
 from homeassistant.helpers.entity import ToggleEntity
 from homeassistant.helpers.entity_component import EntityComponent
 from homeassistant.helpers.script import (
-    DEFAULT_QUEUE_MAX,
-    SCRIPT_MODE_CHOICES,
+    SCRIPT_BASE_SCHEMA,
     SCRIPT_MODE_LEGACY,
-    SCRIPT_MODE_QUEUE,
     Script,
+    validate_queue_size,
+    warn_deprecated_legacy,
 )
 from homeassistant.helpers.service import async_set_service_schema
 from homeassistant.loader import bind_hass
@@ -44,7 +45,6 @@ CONF_DESCRIPTION = "description"
 CONF_EXAMPLE = "example"
 CONF_FIELDS = "fields"
 CONF_SEQUENCE = "sequence"
-CONF_QUEUE_MAX = "queue_size"
 
 ENTITY_ID_FORMAT = DOMAIN + ".{}"
 
@@ -59,55 +59,32 @@ def _deprecated_legacy_mode(config):
             legacy_scripts.append(object_id)
             cfg[CONF_MODE] = SCRIPT_MODE_LEGACY
     if legacy_scripts:
-        _LOGGER.warning(
-            "Script behavior has changed. "
-            "To continue using previous behavior, which is now deprecated, "
-            "add '%s: %s' to script(s): %s.",
-            CONF_MODE,
-            SCRIPT_MODE_LEGACY,
-            ", ".join(legacy_scripts),
-        )
-    return config
-
-
-def _queue_max(config):
-    for object_id, cfg in config.items():
-        mode = cfg[CONF_MODE]
-        queue_max = cfg.get(CONF_QUEUE_MAX)
-        if mode == SCRIPT_MODE_QUEUE:
-            if queue_max is None:
-                cfg[CONF_QUEUE_MAX] = DEFAULT_QUEUE_MAX
-        elif queue_max is not None:
-            raise vol.Invalid(
-                f"{CONF_QUEUE_MAX} not valid with {mode} {CONF_MODE} "
-                f"for script '{object_id}'"
-            )
+        warn_deprecated_legacy(_LOGGER, f"script(s): {', '.join(legacy_scripts)}")
     return config
 
 
-SCRIPT_ENTRY_SCHEMA = vol.Schema(
-    {
-        vol.Optional(CONF_ALIAS): cv.string,
-        vol.Optional(CONF_ICON): cv.icon,
-        vol.Required(CONF_SEQUENCE): cv.SCRIPT_SCHEMA,
-        vol.Optional(CONF_DESCRIPTION, default=""): cv.string,
-        vol.Optional(CONF_FIELDS, default={}): {
-            cv.string: {
-                vol.Optional(CONF_DESCRIPTION): cv.string,
-                vol.Optional(CONF_EXAMPLE): cv.string,
-            }
-        },
-        vol.Optional(CONF_MODE): vol.In(SCRIPT_MODE_CHOICES),
-        vol.Optional(CONF_QUEUE_MAX): vol.All(vol.Coerce(int), vol.Range(min=2)),
-    }
+SCRIPT_ENTRY_SCHEMA = vol.All(
+    SCRIPT_BASE_SCHEMA.extend(
+        {
+            vol.Optional(CONF_ALIAS): cv.string,
+            vol.Optional(CONF_ICON): cv.icon,
+            vol.Required(CONF_SEQUENCE): cv.SCRIPT_SCHEMA,
+            vol.Optional(CONF_DESCRIPTION, default=""): cv.string,
+            vol.Optional(CONF_FIELDS, default={}): {
+                cv.string: {
+                    vol.Optional(CONF_DESCRIPTION): cv.string,
+                    vol.Optional(CONF_EXAMPLE): cv.string,
+                }
+            },
+        }
+    ),
+    validate_queue_size,
 )
 
 CONFIG_SCHEMA = vol.Schema(
     {
         DOMAIN: vol.All(
-            cv.schema_with_slug_keys(SCRIPT_ENTRY_SCHEMA),
-            _deprecated_legacy_mode,
-            _queue_max,
+            cv.schema_with_slug_keys(SCRIPT_ENTRY_SCHEMA), _deprecated_legacy_mode
         )
     },
     extra=vol.ALLOW_EXTRA,
@@ -271,7 +248,7 @@ async def _async_process_config(hass, config, component):
                 cfg.get(CONF_ICON),
                 cfg[CONF_SEQUENCE],
                 cfg[CONF_MODE],
-                cfg.get(CONF_QUEUE_MAX, 0),
+                cfg.get(CONF_QUEUE_SIZE, 0),
             )
         )
 
@@ -303,7 +280,7 @@ class ScriptEntity(ToggleEntity):
 
     icon = None
 
-    def __init__(self, hass, object_id, name, icon, sequence, mode, queue_max):
+    def __init__(self, hass, object_id, name, icon, sequence, mode, queue_size):
         """Initialize the script."""
         self.object_id = object_id
         self.icon = icon
@@ -314,7 +291,7 @@ class ScriptEntity(ToggleEntity):
             name,
             self.async_change_listener,
             mode,
-            queue_max,
+            queue_size,
             logging.getLogger(f"{__name__}.{object_id}"),
         )
         self._changed = asyncio.Event()
diff --git a/homeassistant/const.py b/homeassistant/const.py
index 3472e46dd69..658b3081d02 100644
--- a/homeassistant/const.py
+++ b/homeassistant/const.py
@@ -134,6 +134,7 @@ CONF_PREFIX = "prefix"
 CONF_PROFILE_NAME = "profile_name"
 CONF_PROTOCOL = "protocol"
 CONF_PROXY_SSL = "proxy_ssl"
+CONF_QUEUE_SIZE = "queue_size"
 CONF_QUOTE = "quote"
 CONF_RADIUS = "radius"
 CONF_RECIPIENT = "recipient"
diff --git a/homeassistant/helpers/script.py b/homeassistant/helpers/script.py
index 107cd9e2106..c235a65dc81 100644
--- a/homeassistant/helpers/script.py
+++ b/homeassistant/helpers/script.py
@@ -24,6 +24,8 @@ from homeassistant.const import (
     CONF_EVENT,
     CONF_EVENT_DATA,
     CONF_EVENT_DATA_TEMPLATE,
+    CONF_MODE,
+    CONF_QUEUE_SIZE,
     CONF_SCENE,
     CONF_TIMEOUT,
     CONF_WAIT_TEMPLATE,
@@ -72,11 +74,42 @@ SCRIPT_MODE_CHOICES = [
 ]
 DEFAULT_SCRIPT_MODE = SCRIPT_MODE_LEGACY
 
-DEFAULT_QUEUE_MAX = 10
+DEFAULT_QUEUE_SIZE = 10
 
 _LOG_EXCEPTION = logging.ERROR + 1
 _TIMEOUT_MSG = "Timeout reached, abort script."
 
+SCRIPT_BASE_SCHEMA = vol.Schema(
+    {
+        vol.Optional(CONF_MODE): vol.In(SCRIPT_MODE_CHOICES),
+        vol.Optional(CONF_QUEUE_SIZE): vol.All(vol.Coerce(int), vol.Range(min=2)),
+    }
+)
+
+
+def warn_deprecated_legacy(logger, msg):
+    """Warn about deprecated legacy mode."""
+    logger.warning(
+        "Script behavior has changed. "
+        "To continue using previous behavior, which is now deprecated, "
+        "add '%s: %s' to %s.",
+        CONF_MODE,
+        SCRIPT_MODE_LEGACY,
+        msg,
+    )
+
+
+def validate_queue_size(config):
+    """Validate queue_size option."""
+    mode = config.get(CONF_MODE, DEFAULT_SCRIPT_MODE)
+    queue_size = config.get(CONF_QUEUE_SIZE)
+    if mode == SCRIPT_MODE_QUEUE:
+        if queue_size is None:
+            config[CONF_QUEUE_SIZE] = DEFAULT_QUEUE_SIZE
+    elif queue_size is not None:
+        raise vol.Invalid(f"{CONF_QUEUE_SIZE} not valid with {mode} {CONF_MODE}")
+    return config
+
 
 async def async_validate_action_config(
     hass: HomeAssistant, config: ConfigType
@@ -673,7 +706,7 @@ class Script:
         name: Optional[str] = None,
         change_listener: Optional[Callable[..., Any]] = None,
         script_mode: str = DEFAULT_SCRIPT_MODE,
-        queue_max: int = DEFAULT_QUEUE_MAX,
+        queue_size: int = DEFAULT_QUEUE_SIZE,
         logger: Optional[logging.Logger] = None,
         log_exceptions: bool = True,
     ) -> None:
@@ -702,7 +735,7 @@ class Script:
 
         self._runs: List[_ScriptRunBase] = []
         if script_mode == SCRIPT_MODE_QUEUE:
-            self._queue_max = queue_max
+            self._queue_size = queue_size
             self._queue_len = 0
             self._queue_lck = asyncio.Lock()
         self._config_cache: Dict[Set[Tuple], Callable[..., bool]] = {}
@@ -806,7 +839,7 @@ class Script:
                     self._queue_len,
                     "s" if self._queue_len > 1 else "",
                 )
-                if self._queue_len >= self._queue_max:
+                if self._queue_len >= self._queue_size:
                     raise QueueFull
 
         if self.is_legacy:
diff --git a/tests/components/automation/test_init.py b/tests/components/automation/test_init.py
index 9af8a6591d9..70feb2a7796 100644
--- a/tests/components/automation/test_init.py
+++ b/tests/components/automation/test_init.py
@@ -1070,3 +1070,35 @@ async def test_logbook_humanify_automation_triggered_event(hass):
     assert event2["domain"] == "automation"
     assert event2["message"] == "has been triggered"
     assert event2["entity_id"] == "automation.bye"
+
+
+async def test_invalid_config(hass):
+    """Test invalid config."""
+    with assert_setup_component(0, automation.DOMAIN):
+        assert await async_setup_component(
+            hass,
+            automation.DOMAIN,
+            {
+                automation.DOMAIN: {
+                    "mode": "parallel",
+                    "queue_size": 5,
+                    "trigger": {"platform": "event", "event_type": "test_event"},
+                    "action": [],
+                }
+            },
+        )
+
+
+async def test_config_legacy(hass, caplog):
+    """Test config defaulting to legacy mode."""
+    assert await async_setup_component(
+        hass,
+        automation.DOMAIN,
+        {
+            automation.DOMAIN: {
+                "trigger": {"platform": "event", "event_type": "test_event"},
+                "action": [],
+            }
+        },
+    )
+    assert "To continue using previous behavior, which is now deprecated" in caplog.text
-- 
GitLab