From 5c95c53c6c33e22eacc742e7b9761cc1b0ed7d03 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Mon, 7 May 2018 05:25:48 -0400 Subject: [PATCH] Revert custom component loading logic (#14327) * Revert custom component loading logic * Lint * Fix tests * Guard for infinite inserts into sys.path --- homeassistant/loader.py | 114 +++++++----------- tests/components/notify/test_file.py | 46 ++++--- 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 | 2 + 8 files changed, 84 insertions(+), 101 deletions(-) create mode 100644 tests/testing_config/custom_components/test_package/const.py diff --git a/homeassistant/loader.py b/homeassistant/loader.py index b6dabb1d883..e94fb2d6833 100644 --- a/homeassistant/loader.py +++ b/homeassistant/loader.py @@ -31,12 +31,6 @@ PREPARED = False 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,85 +58,63 @@ 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: + # 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] = {} - 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 + 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 + + _LOGGER.info("Loaded %s from %s", comp_or_platform, path) + + cache[comp_or_platform] = module + + return module + + 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.error("Unable to find component %s", comp_or_platform) + return None -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) - - # 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) - - # Not found - if spec is None: - return None - - # This is a namespace - if spec.loader is None: - return None - - _LOGGER.debug('Loaded %s (%s)', name, base_module) - - 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 - - class Components: """Helper to load components.""" diff --git a/tests/components/notify/test_file.py b/tests/components/notify/test_file.py index 42b9eb9d82d..c5064fca851 100644 --- a/tests/components/notify/test_file.py +++ b/tests/components/notify/test_file.py @@ -35,28 +35,30 @@ class TestNotifyFile(unittest.TestCase): 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 @@ class TestNotifyFile(unittest.TestCase): 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) diff --git a/tests/test_loader.py b/tests/test_loader.py index e8a79c6501f..c97e94a7ce1 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 29d362699f5..b50050ed68e 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 71625dfdf93..fbf79f9e770 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 2819f2f2951..79126b7b52a 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 ee669c6c9b5..85e78a7f9d6 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 00000000000..7e13e04cb47 --- /dev/null +++ b/tests/testing_config/custom_components/test_package/const.py @@ -0,0 +1,2 @@ +"""Constants for test_package custom component.""" +TEST = 5