From 7b374a6427a07291daaf37592391843f54e51a56 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Sun, 6 May 2018 23:26:16 -0400 Subject: [PATCH 1/4] Revert custom component loading logic --- homeassistant/loader.py | 100 ++++++------------ tests/test_loader.py | 4 + .../image_processing/test.py | 6 +- .../custom_components/light/test.py | 5 +- .../custom_components/switch/test.py | 5 +- .../test_package/__init__.py | 3 + .../custom_components/test_package/const.py | 1 + 7 files changed, 53 insertions(+), 71 deletions(-) create mode 100644 tests/testing_config/custom_components/test_package/const.py diff --git a/homeassistant/loader.py b/homeassistant/loader.py index b6dabb1d88397b..23baae9cce40a5 100644 --- a/homeassistant/loader.py +++ b/homeassistant/loader.py @@ -31,12 +31,6 @@ DEPENDENCY_BLACKLIST = set(('config',)) -# List of available components -AVAILABLE_COMPONENTS = [] # type: List[str] - -# Dict of loaded components mapped name => module -_COMPONENT_CACHE = {} # type: Dict[str, ModuleType] - _LOGGER = logging.getLogger(__name__) @@ -64,83 +58,59 @@ def get_platform(hass, domain: str, platform: str) -> Optional[ModuleType]: return get_component(hass, PLATFORM_FORMAT.format(domain, platform)) -def get_component(hass, comp_or_platform): - """Load a module from either custom component or built-in.""" +def get_component(hass, comp_or_platform) -> Optional[ModuleType]: + """Try to load specified component. + + Looks in config dir first, then built-in components. + Only returns it if also found to be valid. + Async friendly. + """ try: return hass.data[DATA_KEY][comp_or_platform] except KeyError: pass - # Try custom component - module = _load_module(hass.config.path(PATH_CUSTOM_COMPONENTS), - PATH_CUSTOM_COMPONENTS, comp_or_platform) - - if module is None: - try: - module = importlib.import_module( - '{}.{}'.format(PACKAGE_COMPONENTS, comp_or_platform)) - _LOGGER.debug('Loaded %s (built-in)', comp_or_platform) - except ImportError: - _LOGGER.warning('Unable to find %s', comp_or_platform) - module = None - cache = hass.data.get(DATA_KEY) if cache is None: + sys.path.insert(0, hass.config.config_dir) cache = hass.data[DATA_KEY] = {} - cache[comp_or_platform] = module - - return module + # First check custom, then built-in + potential_paths = ['custom_components.{}'.format(comp_or_platform), + 'homeassistant.components.{}'.format(comp_or_platform)] -def _find_spec(path, name): - for finder in sys.meta_path: + for path in potential_paths: try: - spec = finder.find_spec(name, path=path) - if spec is not None: - return spec - except AttributeError: - # Not all finders have the find_spec method - pass - return None + module = importlib.import_module(path) + # In Python 3 you can import files from directories that do not + # contain the file __init__.py. A directory is a valid module if + # it contains a file with the .py extension. In this case Python + # will succeed in importing the directory as a module and call it + # a namespace. We do not care about namespaces. + # This prevents that when only + # custom_components/switch/some_platform.py exists, + # the import custom_components.switch would succeed. + if module.__spec__.origin == 'namespace': + continue -def _load_module(path, base_module, name): - """Load a module based on a folder and a name.""" - mod_name = "{}.{}".format(base_module, name) - spec = _find_spec([path], name) + _LOGGER.info("Loaded %s from %s", comp_or_platform, path) - # Special handling if loading platforms and the folder is a namespace - # (namespace is a folder without __init__.py) - if spec is None and '.' in name: - mod_parent_name = name.split('.')[0] - parent_spec = _find_spec([path], mod_parent_name) - if (parent_spec is None or - parent_spec.submodule_search_locations is None): - return None - spec = _find_spec(parent_spec.submodule_search_locations, mod_name) + cache[comp_or_platform] = module - # Not found - if spec is None: - return None + return module - # This is a namespace - if spec.loader is None: - return None + except ImportError as err: + # This error happens if for example custom_components/switch + # exists and we try to load switch.demo. + if str(err) != "No module named '{}'".format(path): + _LOGGER.exception( + ("Error loading %s. Make sure all " + "dependencies are installed"), path) - _LOGGER.debug('Loaded %s (%s)', name, base_module) + _LOGGER.error("Unable to find component %s", comp_or_platform) - module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(module) - # A hack, I know. Don't currently know how to work around it. - if not module.__name__.startswith(base_module): - module.__name__ = "{}.{}".format(base_module, name) - - if not module.__package__: - module.__package__ = base_module - elif not module.__package__.startswith(base_module): - module.__package__ = "{}.{}".format(base_module, name) - - return module + return None class Components: diff --git a/tests/test_loader.py b/tests/test_loader.py index e8a79c6501f33d..c97e94a7ce10f1 100644 --- a/tests/test_loader.py +++ b/tests/test_loader.py @@ -120,3 +120,7 @@ async def test_custom_component_name(hass): comp = loader.get_component(hass, 'light.test') assert comp.__name__ == 'custom_components.light.test' assert comp.__package__ == 'custom_components.light' + + # Test custom components is mounted + from custom_components.test_package import TEST + assert TEST == 5 diff --git a/tests/testing_config/custom_components/image_processing/test.py b/tests/testing_config/custom_components/image_processing/test.py index 29d362699f59f1..b50050ed68e5e7 100644 --- a/tests/testing_config/custom_components/image_processing/test.py +++ b/tests/testing_config/custom_components/image_processing/test.py @@ -3,9 +3,11 @@ from homeassistant.components.image_processing import ImageProcessingEntity -def setup_platform(hass, config, add_devices, discovery_info=None): +async def async_setup_platform(hass, config, async_add_devices_callback, + discovery_info=None): """Set up the test image_processing platform.""" - add_devices([TestImageProcessing('camera.demo_camera', "Test")]) + async_add_devices_callback([ + TestImageProcessing('camera.demo_camera', "Test")]) class TestImageProcessing(ImageProcessingEntity): diff --git a/tests/testing_config/custom_components/light/test.py b/tests/testing_config/custom_components/light/test.py index 71625dfdf933e8..fbf79f9e77054a 100644 --- a/tests/testing_config/custom_components/light/test.py +++ b/tests/testing_config/custom_components/light/test.py @@ -21,6 +21,7 @@ def init(empty=False): ] -def setup_platform(hass, config, add_devices_callback, discovery_info=None): +async def async_setup_platform(hass, config, async_add_devices_callback, + discovery_info=None): """Return mock devices.""" - add_devices_callback(DEVICES) + async_add_devices_callback(DEVICES) diff --git a/tests/testing_config/custom_components/switch/test.py b/tests/testing_config/custom_components/switch/test.py index 2819f2f2951a86..79126b7b52ae45 100644 --- a/tests/testing_config/custom_components/switch/test.py +++ b/tests/testing_config/custom_components/switch/test.py @@ -21,6 +21,7 @@ def init(empty=False): ] -def setup_platform(hass, config, add_devices_callback, discovery_info=None): +async def async_setup_platform(hass, config, async_add_devices_callback, + discovery_info=None): """Find and return test switches.""" - add_devices_callback(DEVICES) + async_add_devices_callback(DEVICES) diff --git a/tests/testing_config/custom_components/test_package/__init__.py b/tests/testing_config/custom_components/test_package/__init__.py index ee669c6c9b59fc..85e78a7f9d6d41 100644 --- a/tests/testing_config/custom_components/test_package/__init__.py +++ b/tests/testing_config/custom_components/test_package/__init__.py @@ -1,4 +1,7 @@ """Provide a mock package component.""" +from .const import TEST # noqa + + DOMAIN = 'test_package' diff --git a/tests/testing_config/custom_components/test_package/const.py b/tests/testing_config/custom_components/test_package/const.py new file mode 100644 index 00000000000000..416f1da0ce606e --- /dev/null +++ b/tests/testing_config/custom_components/test_package/const.py @@ -0,0 +1 @@ +TEST = 5 From e2d62e72af318aceb0bd4880db2ebcbbc5e47bb7 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Sun, 6 May 2018 23:40:21 -0400 Subject: [PATCH 2/4] Lint --- tests/testing_config/custom_components/test_package/const.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/testing_config/custom_components/test_package/const.py b/tests/testing_config/custom_components/test_package/const.py index 416f1da0ce606e..7e13e04cb473ff 100644 --- a/tests/testing_config/custom_components/test_package/const.py +++ b/tests/testing_config/custom_components/test_package/const.py @@ -1 +1,2 @@ +"""Constants for test_package custom component.""" TEST = 5 From 3991313cec6bf4fc0816e0112dcf9e89948c2c1b Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Mon, 7 May 2018 00:04:02 -0400 Subject: [PATCH 3/4] Fix tests --- tests/components/notify/test_file.py | 46 +++++++++++++--------------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/tests/components/notify/test_file.py b/tests/components/notify/test_file.py index 42b9eb9d82d256..c5064fca851bc3 100644 --- a/tests/components/notify/test_file.py +++ b/tests/components/notify/test_file.py @@ -35,28 +35,30 @@ def test_bad_config(self): assert setup_component(self.hass, notify.DOMAIN, config) assert not handle_config[notify.DOMAIN] - def _test_notify_file(self, timestamp, mock_utcnow, mock_stat): + def _test_notify_file(self, timestamp): """Test the notify file output.""" - mock_utcnow.return_value = dt_util.as_utc(dt_util.now()) - mock_stat.return_value.st_size = 0 + filename = 'mock_file' + message = 'one, two, testing, testing' + with assert_setup_component(1) as handle_config: + self.assertTrue(setup_component(self.hass, notify.DOMAIN, { + 'notify': { + 'name': 'test', + 'platform': 'file', + 'filename': filename, + 'timestamp': timestamp, + } + })) + assert handle_config[notify.DOMAIN] m_open = mock_open() with patch( 'homeassistant.components.notify.file.open', m_open, create=True - ): - filename = 'mock_file' - message = 'one, two, testing, testing' - with assert_setup_component(1) as handle_config: - self.assertTrue(setup_component(self.hass, notify.DOMAIN, { - 'notify': { - 'name': 'test', - 'platform': 'file', - 'filename': filename, - 'timestamp': timestamp, - } - })) - assert handle_config[notify.DOMAIN] + ), patch('homeassistant.components.notify.file.os.stat') as mock_st, \ + patch('homeassistant.util.dt.utcnow', + return_value=dt_util.utcnow()): + + mock_st.return_value.st_size = 0 title = '{} notifications (Log started: {})\n{}\n'.format( ATTR_TITLE_DEFAULT, dt_util.utcnow().isoformat(), @@ -82,14 +84,10 @@ def _test_notify_file(self, timestamp, mock_utcnow, mock_stat): dt_util.utcnow().isoformat(), message))] ) - @patch('homeassistant.components.notify.file.os.stat') - @patch('homeassistant.util.dt.utcnow') - def test_notify_file(self, mock_utcnow, mock_stat): + def test_notify_file(self): """Test the notify file output without timestamp.""" - self._test_notify_file(False, mock_utcnow, mock_stat) + self._test_notify_file(False) - @patch('homeassistant.components.notify.file.os.stat') - @patch('homeassistant.util.dt.utcnow') - def test_notify_file_timestamp(self, mock_utcnow, mock_stat): + def test_notify_file_timestamp(self): """Test the notify file output with timestamp.""" - self._test_notify_file(True, mock_utcnow, mock_stat) + self._test_notify_file(True) From 312767285984d33f18c17c66d23f196b849a6c2f Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Mon, 7 May 2018 00:07:33 -0400 Subject: [PATCH 4/4] Guard for infinite inserts into sys.path --- homeassistant/loader.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/homeassistant/loader.py b/homeassistant/loader.py index 23baae9cce40a5..e94fb2d6833023 100644 --- a/homeassistant/loader.py +++ b/homeassistant/loader.py @@ -72,7 +72,9 @@ def get_component(hass, comp_or_platform) -> Optional[ModuleType]: cache = hass.data.get(DATA_KEY) if cache is None: - sys.path.insert(0, hass.config.config_dir) + # Only insert if it's not there (happens during tests) + if sys.path[0] != hass.config.config_dir: + sys.path.insert(0, hass.config.config_dir) cache = hass.data[DATA_KEY] = {} # First check custom, then built-in