diff --git a/homeassistant/bootstrap.py b/homeassistant/bootstrap.py index 923237d4e63..59061f40754 100644 --- a/homeassistant/bootstrap.py +++ b/homeassistant/bootstrap.py @@ -4,7 +4,7 @@ import logging import logging.handlers import os import sys -from collections import defaultdict +from collections import OrderedDict from types import ModuleType from typing import Any, Optional, Dict @@ -57,7 +57,7 @@ def async_setup_component(hass: core.HomeAssistant, domain: str, yield from hass.loop.run_in_executor(None, loader.prepare, hass) if config is None: - config = defaultdict(dict) + config = {} components = loader.load_order_component(domain) @@ -345,7 +345,7 @@ def from_config_dict(config: Dict[str, Any], # run task future = asyncio.Future(loop=hass.loop) - hass.loop.create_task(_async_init_from_config_dict(future)) + hass.async_add_job(_async_init_from_config_dict(future)) hass.loop.run_until_complete(future) return future.result() @@ -365,6 +365,12 @@ def async_from_config_dict(config: Dict[str, Any], Dynamically loads required components and its dependencies. This method is a coroutine. """ + setup_lock = hass.data.get('setup_lock') + if setup_lock is None: + setup_lock = hass.data['setup_lock'] = asyncio.Lock(loop=hass.loop) + + yield from setup_lock.acquire() + core_config = config.get(core.DOMAIN, {}) try: @@ -388,10 +394,12 @@ def async_from_config_dict(config: Dict[str, Any], yield from hass.loop.run_in_executor(None, loader.prepare, hass) # Make a copy because we are mutating it. - # Convert it to defaultdict so components can always have config dict + # Use OrderedDict in case original one was one. # Convert values to dictionaries if they are None - config = defaultdict( - dict, {key: value or {} for key, value in config.items()}) + new_config = OrderedDict() + for key, value in config.items(): + new_config[key] = value or {} + config = new_config # Filter out the repeating and common config section [homeassistant] components = set(key.split(' ')[0] for key in config.keys() @@ -417,6 +425,8 @@ def async_from_config_dict(config: Dict[str, Any], for domain in loader.load_order_components(components): yield from _async_setup_component(hass, domain, config) + setup_lock.release() + return hass diff --git a/homeassistant/helpers/discovery.py b/homeassistant/helpers/discovery.py index 783fc8354eb..76ff8efdf45 100644 --- a/homeassistant/helpers/discovery.py +++ b/homeassistant/helpers/discovery.py @@ -1,11 +1,16 @@ -"""Helper methods to help with platform discovery.""" +"""Helper methods to help with platform discovery. + +There are two different types of discoveries that can be fired/listened for. + - listen/discover is for services. These are targetted at a component. + - listen_platform/discover_platform is for platforms. These are used by + components to allow discovery of their platofrms. +""" import asyncio from homeassistant import bootstrap, core from homeassistant.const import ( ATTR_DISCOVERED, ATTR_SERVICE, EVENT_PLATFORM_DISCOVERED) -from homeassistant.util.async import ( - run_callback_threadsafe, fire_coroutine_threadsafe) +from homeassistant.util.async import run_callback_threadsafe EVENT_LOAD_PLATFORM = 'load_platform.{}' ATTR_PLATFORM = 'platform' @@ -43,8 +48,29 @@ def async_listen(hass, service, callback): def discover(hass, service, discovered=None, component=None, hass_config=None): """Fire discovery event. Can ensure a component is loaded.""" - if component is not None: - bootstrap.setup_component(hass, component, hass_config) + hass.async_add_job( + async_discover(hass, service, discovered, component, hass_config)) + + +@asyncio.coroutine +def async_discover(hass, service, discovered=None, component=None, + hass_config=None): + """Fire discovery event. Can ensure a component is loaded.""" + if component is not None and component not in hass.config.components: + did_lock = False + setup_lock = hass.data.get('setup_lock') + if setup_lock and setup_lock.locked(): + did_lock = True + yield from setup_lock.acquire() + + try: + # Could have been loaded while waiting for lock. + if component not in hass.config.components: + yield from bootstrap.async_setup_component(hass, component, + hass_config) + finally: + if did_lock: + setup_lock.release() data = { ATTR_SERVICE: service @@ -53,7 +79,7 @@ def discover(hass, service, discovered=None, component=None, hass_config=None): if discovered is not None: data[ATTR_DISCOVERED] = discovered - hass.bus.fire(EVENT_PLATFORM_DISCOVERED, data) + hass.bus.async_fire(EVENT_PLATFORM_DISCOVERED, data) def listen_platform(hass, component, callback): @@ -101,9 +127,9 @@ def load_platform(hass, component, platform, discovered=None, Use `listen_platform` to register a callback for these events. """ - fire_coroutine_threadsafe( - async_load_platform(hass, component, platform, - discovered, hass_config), hass.loop) + hass.async_add_job( + async_load_platform(hass, component, platform, discovered, + hass_config)) @asyncio.coroutine @@ -120,25 +146,30 @@ def async_load_platform(hass, component, platform, discovered=None, Use `listen_platform` to register a callback for these events. Warning: Do not yield from this inside a setup method to avoid a dead lock. - Use `hass.loop.create_task(async_load_platform(..))` instead. + Use `hass.loop.async_add_job(async_load_platform(..))` instead. This method is a coroutine. """ did_lock = False - setup_lock = hass.data.get('setup_lock') - if setup_lock and setup_lock.locked(): - did_lock = True - yield from setup_lock.acquire() + if component not in hass.config.components: + setup_lock = hass.data.get('setup_lock') + if setup_lock and setup_lock.locked(): + did_lock = True + yield from setup_lock.acquire() + + setup_success = True try: - # No need to fire event if we could not setup component - res = yield from bootstrap.async_setup_component( - hass, component, hass_config) + # Could have been loaded while waiting for lock. + if component not in hass.config.components: + setup_success = yield from bootstrap.async_setup_component( + hass, component, hass_config) finally: if did_lock: setup_lock.release() - if not res: + # No need to fire event if we could not setup component + if not setup_success: return data = { diff --git a/tests/helpers/test_discovery.py b/tests/helpers/test_discovery.py index a81db2074fb..c062cb7b566 100644 --- a/tests/helpers/test_discovery.py +++ b/tests/helpers/test_discovery.py @@ -1,7 +1,9 @@ """Test discovery helpers.""" +from collections import OrderedDict from unittest.mock import patch from homeassistant import loader, bootstrap +from homeassistant.core import callback from homeassistant.helpers import discovery from homeassistant.util.async import run_coroutine_threadsafe @@ -20,16 +22,18 @@ class TestHelpersDiscovery: """Stop everything that was started.""" self.hass.stop() - @patch('homeassistant.bootstrap.setup_component') + @patch('homeassistant.bootstrap.async_setup_component') def test_listen(self, mock_setup_component): """Test discovery listen/discover combo.""" calls_single = [] calls_multi = [] + @callback def callback_single(service, info): """Service discovered callback.""" calls_single.append((service, info)) + @callback def callback_multi(service, info): """Service discovered callback.""" calls_multi.append((service, info)) @@ -42,16 +46,16 @@ class TestHelpersDiscovery: 'test_component') self.hass.block_till_done() - discovery.discover(self.hass, 'another service', 'discovery info', - 'test_component') - self.hass.block_till_done() - assert mock_setup_component.called assert mock_setup_component.call_args[0] == \ (self.hass, 'test_component', None) assert len(calls_single) == 1 assert calls_single[0] == ('test service', 'discovery info') + discovery.discover(self.hass, 'another service', 'discovery info', + 'test_component') + self.hass.block_till_done() + assert len(calls_single) == 1 assert len(calls_multi) == 2 assert ['test service', 'another service'] == [info[0] for info @@ -63,6 +67,7 @@ class TestHelpersDiscovery: """Test discover platform method.""" calls = [] + @callback def platform_callback(platform, info): """Platform callback method.""" calls.append((platform, info)) @@ -149,3 +154,44 @@ class TestHelpersDiscovery: assert len(platform_calls) == 2 assert 'test_component' in self.hass.config.components assert 'switch' in self.hass.config.components + + def test_1st_discovers_2nd_component(self): + """Test that we don't break if one component discovers the other. + + If the first component fires a discovery event to setup the + second component while the second component is about to be setup, + it should not setup the second component twice. + """ + component_calls = [] + + def component1_setup(hass, config): + """Setup mock component.""" + discovery.discover(hass, 'test_component2') + return True + + def component2_setup(hass, config): + """Setup mock component.""" + component_calls.append(1) + return True + + loader.set_component( + 'test_component1', + MockModule('test_component1', setup=component1_setup)) + + loader.set_component( + 'test_component2', + MockModule('test_component2', setup=component2_setup)) + + config = OrderedDict() + config['test_component1'] = {} + config['test_component2'] = {} + + self.hass.loop.run_until_complete = \ + lambda _: self.hass.block_till_done() + + bootstrap.from_config_dict(config, self.hass) + + self.hass.block_till_done() + + # test_component will only be setup once + assert len(component_calls) == 1