Add voluptuous config validation to scenes (home-assistant#6830)

* Add platform schema to scene component and homeassistant platform.
* Clean up code and add constants.
* Add unit test and clean up tests.

homeassistant/components/scene/__init__.py

@@ -6,28 +6,55 @@
 """
 import asyncio
 import logging
-from collections import namedtuple
 
 import voluptuous as vol
 
 from homeassistant.const import (
-    ATTR_ENTITY_ID, SERVICE_TURN_ON, CONF_PLATFORM)
-from homeassistant.helpers import extract_domain_configs
+    ATTR_ENTITY_ID, CONF_PLATFORM, SERVICE_TURN_ON)
 import homeassistant.helpers.config_validation as cv
 from homeassistant.helpers.entity import Entity
 from homeassistant.helpers.entity_component import EntityComponent
+from homeassistant.helpers.state import HASS_DOMAIN
+from homeassistant.loader import get_platform
 
 DOMAIN = 'scene'
 STATE = 'scening'
+STATES = 'states'
 
-CONF_ENTITIES = "entities"
+
+def _hass_domain_validator(config):
+    """Validate platform in config for homeassistant domain."""
+    if CONF_PLATFORM not in config:
+        config = {
+            CONF_PLATFORM: HASS_DOMAIN, STATES: config}
+
+    return config
+
+
+def _platform_validator(config):
+    """Validate it is a valid  platform."""
+    p_name = config[CONF_PLATFORM]
+    platform = get_platform(DOMAIN, p_name)
+
+    if not hasattr(platform, 'PLATFORM_SCHEMA'):
+        return config
+
+    return getattr(platform, 'PLATFORM_SCHEMA')(config)
+
+
+PLATFORM_SCHEMA = vol.Schema(
+    vol.All(
+        _hass_domain_validator,
+        vol.Schema({
+            vol.Required(CONF_PLATFORM): cv.platform_validator(DOMAIN)
+        }, extra=vol.ALLOW_EXTRA),
+        _platform_validator
+    ), extra=vol.ALLOW_EXTRA)
 
 SCENE_SERVICE_SCHEMA = vol.Schema({
     vol.Optional(ATTR_ENTITY_ID): cv.entity_ids,
 })
 
-SceneConfig = namedtuple('SceneConfig', ['name', 'states'])
-
 
 def activate(hass, entity_id=None):
     """Activate a scene."""
@@ -43,21 +70,6 @@ def activate(hass, entity_id=None):
 def async_setup(hass, config):
     """Setup scenes."""
     logger = logging.getLogger(__name__)
-
-    # You are not allowed to mutate the original config so make a copy
-    config = dict(config)
-
-    for config_key in extract_domain_configs(config, DOMAIN):
-        platform_config = config[config_key]
-        if not isinstance(platform_config, list):
-            platform_config = [platform_config]
-
-        if not any(CONF_PLATFORM in entry for entry in platform_config):
-            platform_config = [{'platform': 'homeassistant', 'states': entry}
-                               for entry in platform_config]
-
-        config[config_key] = platform_config
-
     component = EntityComponent(logger, DOMAIN, hass)
 
     yield from component.async_setup(config)

homeassistant/components/scene/homeassistant.py

@@ -7,26 +7,38 @@
 import asyncio
 from collections import namedtuple
 
-from homeassistant.components.scene import Scene
+import voluptuous as vol
+
+import homeassistant.helpers.config_validation as cv
+from homeassistant.components.scene import Scene, STATES
 from homeassistant.const import (
-    ATTR_ENTITY_ID, STATE_OFF, STATE_ON)
+    ATTR_ENTITY_ID, ATTR_STATE, CONF_ENTITIES, CONF_NAME, CONF_PLATFORM,
+    STATE_OFF, STATE_ON)
 from homeassistant.core import State
-from homeassistant.helpers.state import async_reproduce_state
-
-STATE = 'scening'
-
-CONF_ENTITIES = "entities"
-
-SceneConfig = namedtuple('SceneConfig', ['name', 'states'])
+from homeassistant.helpers.state import async_reproduce_state, HASS_DOMAIN
+
+PLATFORM_SCHEMA = vol.Schema({
+    vol.Required(CONF_PLATFORM): HASS_DOMAIN,
+    vol.Required(STATES): vol.All(
+        cv.ensure_list,
+        [
+            {
+                vol.Required(CONF_NAME): cv.string,
+                vol.Required(CONF_ENTITIES): {
+                    cv.entity_id: vol.Any(str, bool, dict)
+                },
+            }
+        ]
+    ),
+}, extra=vol.ALLOW_EXTRA)
+
+SCENECONFIG = namedtuple('SceneConfig', [CONF_NAME, STATES])
 
 
 @asyncio.coroutine
 def async_setup_platform(hass, config, async_add_devices, discovery_info=None):
     """Setup home assistant scene entries."""
-    scene_config = config.get("states")
-
-    if not isinstance(scene_config, list):
-        scene_config = [scene_config]
+    scene_config = config.get(STATES)
 
     async_add_devices(HomeAssistantScene(
         hass, _process_config(scene)) for scene in scene_config)
@@ -38,15 +50,15 @@ def _process_config(scene_config):
 
     Async friendly.
     """
-    name = scene_config.get('name')
+    name = scene_config.get(CONF_NAME)
 
     states = {}
     c_entities = dict(scene_config.get(CONF_ENTITIES, {}))
 
     for entity_id in c_entities:
         if isinstance(c_entities[entity_id], dict):
             entity_attrs = c_entities[entity_id].copy()
-            state = entity_attrs.pop('state', None)
+            state = entity_attrs.pop(ATTR_STATE, None)
             attributes = entity_attrs
         else:
             state = c_entities[entity_id]
@@ -61,7 +73,7 @@ def _process_config(scene_config):
 
         states[entity_id.lower()] = State(entity_id, state, attributes)
 
-    return SceneConfig(name, states)
+    return SCENECONFIG(name, states)
 
 
 class HomeAssistantScene(Scene):

tests/components/scene/test_init.py

@@ -1,9 +1,11 @@
 """The tests for the Scene component."""
+import io
 import unittest
 
 from homeassistant.setup import setup_component
 from homeassistant import loader
 from homeassistant.components import light, scene
+from homeassistant.util import yaml
 
 from tests.common import get_test_home_assistant
 
@@ -14,6 +16,22 @@ class TestScene(unittest.TestCase):
     def setUp(self):  # pylint: disable=invalid-name
         """Setup things to be run when tests are started."""
         self.hass = get_test_home_assistant()
+        test_light = loader.get_component('light.test')
+        test_light.init()
+
+        self.assertTrue(setup_component(self.hass, light.DOMAIN, {
+            light.DOMAIN: {'platform': 'test'}
+        }))
+
+        self.light_1, self.light_2 = test_light.DEVICES[0:2]
+
+        light.turn_off(
+            self.hass, [self.light_1.entity_id, self.light_2.entity_id])
+
+        self.hass.block_till_done()
+
+        self.assertFalse(self.light_1.is_on)
+        self.assertFalse(self.light_2.is_on)
 
     def tearDown(self):  # pylint: disable=invalid-name
         """Stop everything that was started."""
@@ -36,19 +54,6 @@ def test_config_yaml_alias_anchor(self):
         reference to the original dictionary, instead of creating a copy, so
         care needs to be taken to not modify the original.
         """
-        test_light = loader.get_component('light.test')
-        test_light.init()
-
-        self.assertTrue(setup_component(self.hass, light.DOMAIN, {
-            light.DOMAIN: {'platform': 'test'}
-        }))
-
-        light_1, light_2 = test_light.DEVICES[0:2]
-
-        light.turn_off(self.hass, [light_1.entity_id, light_2.entity_id])
-
-        self.hass.block_till_done()
-
         entity_state = {
             'state': 'on',
             'brightness': 100,
@@ -57,43 +62,54 @@ def test_config_yaml_alias_anchor(self):
             'scene': [{
                 'name': 'test',
                 'entities': {
-                    light_1.entity_id: entity_state,
-                    light_2.entity_id: entity_state,
+                    self.light_1.entity_id: entity_state,
+                    self.light_2.entity_id: entity_state,
                 }
             }]
         }))
 
         scene.activate(self.hass, 'scene.test')
         self.hass.block_till_done()
 
-        self.assertTrue(light_1.is_on)
-        self.assertTrue(light_2.is_on)
-        self.assertEqual(100,
-                         light_1.last_call('turn_on')[1].get('brightness'))
-        self.assertEqual(100,
-                         light_2.last_call('turn_on')[1].get('brightness'))
+        self.assertTrue(self.light_1.is_on)
+        self.assertTrue(self.light_2.is_on)
+        self.assertEqual(
+            100, self.light_1.last_call('turn_on')[1].get('brightness'))
+        self.assertEqual(
+            100, self.light_2.last_call('turn_on')[1].get('brightness'))
+
+    def test_config_yaml_bool(self):
+        """Test parsing of booleans in yaml config."""
+        config = (
+            'scene:\n'
+            '  - name: test\n'
+            '    entities:\n'
+            '      {0}: on\n'
+            '      {1}:\n'
+            '        state: on\n'
+            '        brightness: 100\n').format(
+                self.light_1.entity_id, self.light_2.entity_id)
+
+        with io.StringIO(config) as file:
+            doc = yaml.yaml.safe_load(file)
+
+        self.assertTrue(setup_component(self.hass, scene.DOMAIN, doc))
+        scene.activate(self.hass, 'scene.test')
+        self.hass.block_till_done()
+
+        self.assertTrue(self.light_1.is_on)
+        self.assertTrue(self.light_2.is_on)
+        self.assertEqual(
+            100, self.light_2.last_call('turn_on')[1].get('brightness'))
 
     def test_activate_scene(self):
         """Test active scene."""
-        test_light = loader.get_component('light.test')
-        test_light.init()
-
-        self.assertTrue(setup_component(self.hass, light.DOMAIN, {
-            light.DOMAIN: {'platform': 'test'}
-        }))
-
-        light_1, light_2 = test_light.DEVICES[0:2]
-
-        light.turn_off(self.hass, [light_1.entity_id, light_2.entity_id])
-
-        self.hass.block_till_done()
-
         self.assertTrue(setup_component(self.hass, scene.DOMAIN, {
             'scene': [{
                 'name': 'test',
                 'entities': {
-                    light_1.entity_id: 'on',
-                    light_2.entity_id: {
+                    self.light_1.entity_id: 'on',
+                    self.light_2.entity_id: {
                         'state': 'on',
                         'brightness': 100,
                     }
@@ -104,7 +120,7 @@ def test_activate_scene(self):
         scene.activate(self.hass, 'scene.test')
         self.hass.block_till_done()
 
-        self.assertTrue(light_1.is_on)
-        self.assertTrue(light_2.is_on)
-        self.assertEqual(100,
-                         light_2.last_call('turn_on')[1].get('brightness'))
+        self.assertTrue(self.light_1.is_on)
+        self.assertTrue(self.light_2.is_on)
+        self.assertEqual(
+            100, self.light_2.last_call('turn_on')[1].get('brightness'))