Test for circular dependencies using manifests (#22908)

* Integration dependencies

* Lint

* Lint

* Fix one test

* Lint

* Fix load custom component integration

Fix async issue
Add circular dependency detection in manifest validation

* Fix test

* Address review comment

* Apply suggestions from code review

Co-Authored-By: balloob <paulus@home-assistant.io>

homeassistant/bootstrap.py

@@ -128,16 +128,17 @@ async def async_from_config_dict(config: Dict[str, Any],
     hass.config_entries = config_entries.ConfigEntries(hass, config)
     await hass.config_entries.async_initialize()
 
-    components = _get_components(hass, config)
+    domains = _get_domains(hass, config)
 
     # Resolve all dependencies of all components.
-    for component in list(components):
-        try:
-            components.update(loader.component_dependencies(hass, component))
-        except loader.LoaderError:
-            # Ignore it, or we'll break startup
-            # It will be properly handled during setup.
-            pass
+    for dep_domains in await asyncio.gather(*[
+            loader.async_component_dependencies(hass, domain)
+            for domain in domains
+    ], return_exceptions=True):
+        # Result is either a set or an exception. We ignore exceptions
+        # It will be properly handled during setup of the domain.
+        if isinstance(dep_domains, set):
+            domains.update(dep_domains)
 
     # setup components
     res = await core_component.async_setup(hass, config)
@@ -151,10 +152,10 @@ async def async_from_config_dict(config: Dict[str, Any],
     _LOGGER.info("Home Assistant core initialized")
 
     # stage 0, load logging components
-    for component in components:
-        if component in LOGGING_COMPONENT:
+    for domain in domains:
+        if domain in LOGGING_COMPONENT:
             hass.async_create_task(
-                async_setup_component(hass, component, config))
+                async_setup_component(hass, domain, config))
 
     await hass.async_block_till_done()
 
@@ -165,18 +166,18 @@ async def async_from_config_dict(config: Dict[str, Any],
         hass.helpers.area_registry.async_get_registry())
 
     # stage 1
-    for component in components:
-        if component in FIRST_INIT_COMPONENT:
+    for domain in domains:
+        if domain in FIRST_INIT_COMPONENT:
             hass.async_create_task(
-                async_setup_component(hass, component, config))
+                async_setup_component(hass, domain, config))
 
     await hass.async_block_till_done()
 
     # stage 2
-    for component in components:
-        if component in FIRST_INIT_COMPONENT or component in LOGGING_COMPONENT:
+    for domain in domains:
+        if domain in FIRST_INIT_COMPONENT or domain in LOGGING_COMPONENT:
             continue
-        hass.async_create_task(async_setup_component(hass, component, config))
+        hass.async_create_task(async_setup_component(hass, domain, config))
 
     await hass.async_block_till_done()
 
@@ -398,18 +399,17 @@ async def async_mount_local_lib_path(config_dir: str) -> str:
 
 
 @core.callback
-def _get_components(hass: core.HomeAssistant,
-                    config: Dict[str, Any]) -> Set[str]:
-    """Get components to set up."""
+def _get_domains(hass: core.HomeAssistant, config: Dict[str, Any]) -> Set[str]:
+    """Get domains of components to set up."""
     # Filter out the repeating and common config section [homeassistant]
-    components = set(key.split(' ')[0] for key in config.keys()
-                     if key != core.DOMAIN)
+    domains = set(key.split(' ')[0] for key in config.keys()
+                  if key != core.DOMAIN)
 
     # Add config entry domains
-    components.update(hass.config_entries.async_domains())  # type: ignore
+    domains.update(hass.config_entries.async_domains())  # type: ignore
 
     # Make sure the Hass.io component is loaded
     if 'HASSIO' in os.environ:
-        components.add('hassio')
+        domains.add('hassio')
 
-    return components
+    return domains

homeassistant/loader.py

@@ -12,17 +12,28 @@ available it will check the built-in components and platforms.
 """
 import functools as ft
 import importlib
+import json
 import logging
+import pathlib
 import sys
 from types import ModuleType
-from typing import Optional, Set, TYPE_CHECKING, Callable, Any, TypeVar, List  # noqa pylint: disable=unused-import
+from typing import (
+    Optional,
+    Set,
+    TYPE_CHECKING,
+    Callable,
+    Any,
+    TypeVar,
+    List,
+    Dict
+)
 
 from homeassistant.const import PLATFORM_FORMAT
 
 # Typing imports that create a circular dependency
 # pylint: disable=using-constant-test,unused-import
 if TYPE_CHECKING:
-    from homeassistant.core import HomeAssistant  # NOQA
+    from homeassistant.core import HomeAssistant  # noqa
 
 CALLABLE_T = TypeVar('CALLABLE_T', bound=Callable)  # noqa pylint: disable=invalid-name
 
@@ -34,17 +45,146 @@ _LOGGER = logging.getLogger(__name__)
 
 
 DATA_KEY = 'components'
+DATA_INTEGRATIONS = 'integrations'
 PACKAGE_CUSTOM_COMPONENTS = 'custom_components'
 PACKAGE_BUILTIN = 'homeassistant.components'
 LOOKUP_PATHS = [PACKAGE_CUSTOM_COMPONENTS, PACKAGE_BUILTIN]
 COMPONENTS_WITH_BAD_PLATFORMS = ['automation', 'mqtt', 'telegram_bot']
+_UNDEF = object()
+
+
+def manifest_from_legacy_module(module: Any) -> Dict:
+    """Generate a manifest from a legacy module."""
+    return {
+        'domain': module.DOMAIN,
+        'name': module.DOMAIN,
+        'documentation': None,
+        'requirements': getattr(module, 'REQUIREMENTS', []),
+        'dependencies': getattr(module, 'DEPENDENCIES', []),
+        'codeowners': [],
+    }
+
+
+class Integration:
+    """An integration in Home Assistant."""
+
+    @classmethod
+    def resolve_from_root(cls, hass: 'HomeAssistant', root_module: Any,
+                          domain: str) -> 'Optional[Integration]':
+        """Resolve an integration from a root module."""
+        for base in root_module.__path__:
+            manifest_path = (
+                pathlib.Path(base) / domain / 'manifest.json'
+            )
+
+            if not manifest_path.is_file():
+                continue
+
+            try:
+                manifest = json.loads(manifest_path.read_text())
+            except ValueError as err:
+                _LOGGER.error("Error parsing manifest.json file at %s: %s",
+                              manifest_path, err)
+                continue
+
+            return cls(
+                hass, "{}.{}".format(root_module.__name__, domain), manifest
+            )
+
+        return None
+
+    @classmethod
+    def resolve_legacy(cls, hass: 'HomeAssistant', domain: str) \
+            -> 'Optional[Integration]':
+        """Resolve legacy component.
+
+        Will create a stub manifest.
+        """
+        comp = get_component(hass, domain)
+
+        if comp is None:
+            return None
+
+        return cls(
+            hass, comp.__name__, manifest_from_legacy_module(comp)
+        )
+
+    def __init__(self, hass: 'HomeAssistant', pkg_path: str, manifest: Dict):
+        """Initialize an integration."""
+        self.hass = hass
+        self.pkg_path = pkg_path
+        self.name = manifest['name']  # type: str
+        self.domain = manifest['domain']  # type: str
+        self.dependencies = manifest['dependencies']  # type: List[str]
+        self.requirements = manifest['requirements']  # type: List[str]
+
+    def get_component(self) -> Any:
+        """Return the component."""
+        return importlib.import_module(self.pkg_path)
+
+    def get_platform(self, platform_name: str) -> Any:
+        """Return a platform for an integration."""
+        return importlib.import_module(
+            "{}.{}".format(self.pkg_path, platform_name)
+        )
+
+
+async def async_get_integration(hass: 'HomeAssistant', domain: str)\
+         -> Integration:
+    """Get an integration."""
+    cache = hass.data.get(DATA_INTEGRATIONS)
+    if cache is None:
+        if not _async_mount_config_dir(hass):
+            raise IntegrationNotFound(domain)
+        cache = hass.data[DATA_INTEGRATIONS] = {}
+
+    integration = cache.get(domain, _UNDEF)  # type: Optional[Integration]
+
+    if integration is _UNDEF:
+        pass
+    elif integration is None:
+        raise IntegrationNotFound(domain)
+    else:
+        return integration
+
+    try:
+        import custom_components
+        integration = await hass.async_add_executor_job(
+            Integration.resolve_from_root, hass, custom_components, domain
+        )
+        if integration is not None:
+            cache[domain] = integration
+            return integration
+
+    except ImportError:
+        pass
+
+    from homeassistant import components
+
+    integration = await hass.async_add_executor_job(
+        Integration.resolve_from_root, hass, components, domain
+    )
+
+    if integration is not None:
+        cache[domain] = integration
+        return integration
+
+    integration = await hass.async_add_executor_job(
+        Integration.resolve_legacy, hass, domain
+    )
+    cache[domain] = integration
+
+    if not integration:
+        raise IntegrationNotFound(domain)
+
+    return integration
 
 
 class LoaderError(Exception):
     """Loader base error."""
 
 
-class ComponentNotFound(LoaderError):
+class IntegrationNotFound(LoaderError):
     """Raised when a component is not found."""
 
     def __init__(self, domain: str) -> None:
@@ -169,12 +309,8 @@ def _load_file(hass,  # type: HomeAssistant
 
     cache = hass.data.get(DATA_KEY)
     if cache is None:
-        if hass.config.config_dir is None:
-            _LOGGER.error("Can't load components - config dir is not set")
+        if not _async_mount_config_dir(hass):
             return 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] = {}
 
     for path in ('{}.{}'.format(base, comp_or_platform)
@@ -294,46 +430,58 @@ def bind_hass(func: CALLABLE_T) -> CALLABLE_T:
     return func
 
 
-def component_dependencies(hass,  # type: HomeAssistant
-                           comp_name: str) -> Set[str]:
+async def async_component_dependencies(hass,  # type: HomeAssistant
+                                       domain: str) -> Set[str]:
     """Return all dependencies and subdependencies of components.
 
     Raises CircularDependency if a circular dependency is found.
-
-    Async friendly.
     """
-    return _component_dependencies(hass, comp_name, set(), set())
+    return await _async_component_dependencies(hass, domain, set(), set())
 
 
-def _component_dependencies(hass,  # type: HomeAssistant
-                            comp_name: str, loaded: Set[str],
-                            loading: Set) -> Set[str]:
+async def _async_component_dependencies(hass,  # type: HomeAssistant
+                                        domain: str, loaded: Set[str],
+                                        loading: Set) -> Set[str]:
     """Recursive function to get component dependencies.
 
     Async friendly.
     """
-    component = get_component(hass, comp_name)
+    integration = await async_get_integration(hass, domain)
 
-    if component is None:
-        raise ComponentNotFound(comp_name)
+    if integration is None:
+        raise IntegrationNotFound(domain)
 
-    loading.add(comp_name)
+    loading.add(domain)
 
-    for dependency in getattr(component, 'DEPENDENCIES', []):
+    for dependency_domain in integration.dependencies:
         # Check not already loaded
-        if dependency in loaded:
+        if dependency_domain in loaded:
             continue
 
         # If we are already loading it, we have a circular dependency.
-        if dependency in loading:
-            raise CircularDependency(comp_name, dependency)
+        if dependency_domain in loading:
+            raise CircularDependency(domain, dependency_domain)
 
-        dep_loaded = _component_dependencies(
-            hass, dependency, loaded, loading)
+        dep_loaded = await _async_component_dependencies(
+            hass, dependency_domain, loaded, loading)
 
         loaded.update(dep_loaded)
 
-    loaded.add(comp_name)
-    loading.remove(comp_name)
+    loaded.add(domain)
+    loading.remove(domain)
 
     return loaded
+
+
+def _async_mount_config_dir(hass,  # type: HomeAssistant
+                            ) -> bool:
+    """Mount config dir in order to load custom_component.
+
+    Async friendly but not a coroutine.
+    """
+    if hass.config.config_dir is None:
+        _LOGGER.error("Can't load components - config dir is not set")
+        return False
+    if hass.config.config_dir not in sys.path:
+        sys.path.insert(0, hass.config.config_dir)
+    return True

homeassistant/setup.py

@@ -108,8 +108,8 @@ async def _async_setup_component(hass: core.HomeAssistant,
 
     # Validate all dependencies exist and there are no circular dependencies
     try:
-        loader.component_dependencies(hass, domain)
-    except loader.ComponentNotFound as err:
+        await loader.async_component_dependencies(hass, domain)
+    except loader.IntegrationNotFound as err:
         _LOGGER.error(
             "Not setting up %s because we are unable to resolve "
             "(sub)dependency %s", domain, err.domain)

script/manifest/validate.py

@@ -18,10 +18,16 @@ MANIFEST_SCHEMA = vol.Schema({
 })
 
 
-components_path = pathlib.Path('homeassistant/components')
+COMPONENTS_PATH = pathlib.Path('homeassistant/components')
 
 
-def validate_integration(path):
+def validate_dependency(path, dependency, loaded, loading):
+    """Validate dependency is exist and no circular dependency."""
+    dep_path = path.parent / dependency
+    return validate_integration(dep_path, loaded, loading)
+
+
+def validate_integration(path, loaded, loading):
     """Validate that an integrations has a valid manifest."""
     errors = []
     path = pathlib.Path(path)
@@ -29,7 +35,7 @@ def validate_integration(path):
     manifest_path = path / 'manifest.json'
 
     if not manifest_path.is_file():
-        errors.append('File manifest.json not found')
+        errors.append('Manifest file {} not found'.format(manifest_path))
         return errors  # Fatal error
 
     try:
@@ -47,10 +53,18 @@ def validate_integration(path):
         errors.append('Domain does not match dir name')
 
     for dep in manifest['dependencies']:
-        dep_manifest = path.parent / dep / 'manifest.json'
-        if not dep_manifest.is_file():
-            errors.append("Unable to find dependency {}".format(dep))
+        if dep in loaded:
+            continue
+        if dep in loading:
+            errors.append("Found circular dependency {} in {}".format(
+                dep, path
+            ))
+            continue
+        loading.add(dep)
 
+        errors.extend(validate_dependency(path, dep, loaded, loading))
+
+    loaded.add(path.name)
     return errors
 
 
@@ -58,11 +72,11 @@ def validate_all():
     """Validate all integrations."""
     invalid = []
 
-    for fil in components_path.iterdir():
+    for fil in COMPONENTS_PATH.iterdir():
         if fil.is_file() or fil.name == '__pycache__':
             continue
 
-        errors = validate_integration(fil)
+        errors = validate_integration(fil, set(), set())
 
         if errors:
             invalid.append((fil, errors))

tests/common.py

@@ -16,7 +16,7 @@ from unittest.mock import MagicMock, Mock, patch
 import homeassistant.util.dt as date_util
 import homeassistant.util.yaml as yaml
 
-from homeassistant import auth, config_entries, core as ha
+from homeassistant import auth, config_entries, core as ha, loader
 from homeassistant.auth import (
     models as auth_models, auth_store, providers as auth_providers,
     permissions as auth_permissions)
@@ -35,6 +35,7 @@ from homeassistant.util.unit_system import METRIC_SYSTEM
 from homeassistant.util.async_ import (
     run_callback_threadsafe, run_coroutine_threadsafe)
 
+
 _TEST_INSTANCE_PORT = SERVER_PORT
 _LOGGER = logging.getLogger(__name__)
 INSTANCES = []
@@ -894,3 +895,18 @@ async def flush_store(store):
 async def get_system_health_info(hass, domain):
     """Get system health info."""
     return await hass.data['system_health']['info'][domain](hass)
+
+
+def mock_integration(hass, module):
+    """Mock an integration."""
+    integration = loader.Integration(
+        hass, 'homeassisant.components.{}'.format(module.DOMAIN),
+        loader.manifest_from_legacy_module(module))
+    integration.get_component = lambda: module
+
+    # Backwards compat
+    loader.set_component(hass, module.DOMAIN, module)
+
+    hass.data.setdefault(
+        loader.DATA_INTEGRATIONS, {}
+    )[module.DOMAIN] = integration

tests/components/mobile_app/__init__.py

@@ -55,7 +55,7 @@ async def webhook_client(hass, aiohttp_client, hass_storage, hass_admin_user):
     }
 
     await async_setup_component(hass, DOMAIN, {DOMAIN: {}})
-
+    await hass.async_block_till_done()
     return await aiohttp_client(hass.http.app)
 
 
@@ -63,6 +63,7 @@ async def webhook_client(hass, aiohttp_client, hass_storage, hass_admin_user):
 async def authed_api_client(hass, hass_client):
     """Provide an authenticated client for mobile_app to use."""
     await async_setup_component(hass, DOMAIN, {DOMAIN: {}})
+    await hass.async_block_till_done()
     return await hass_client()
 
 
@@ -70,3 +71,4 @@ async def authed_api_client(hass, hass_client):
 async def setup_ws(hass):
     """Configure the websocket_api component."""
     assert await async_setup_component(hass, 'websocket_api', {})
+    await hass.async_block_till_done()

tests/test_bootstrap.py

@@ -108,7 +108,7 @@ async def test_async_from_config_file_not_mount_deps_folder(loop):
 async def test_load_hassio(hass):
     """Test that we load Hass.io component."""
     with patch.dict(os.environ, {}, clear=True):
-        assert bootstrap._get_components(hass, {}) == set()
+        assert bootstrap._get_domains(hass, {}) == set()
 
     with patch.dict(os.environ, {'HASSIO': '1'}):
-        assert bootstrap._get_components(hass, {}) == {'hassio'}
+        assert bootstrap._get_domains(hass, {}) == {'hassio'}

tests/test_loader.py

@@ -4,9 +4,10 @@ import asyncio
 import pytest
 
 import homeassistant.loader as loader
-import homeassistant.components.http as http
+from homeassistant.components import http, hue
+from homeassistant.components.hue import light as hue_light
 
-from tests.common import MockModule, async_mock_service
+from tests.common import MockModule, async_mock_service, mock_integration
 
 
 def test_set_component(hass):
@@ -22,31 +23,30 @@ def test_get_component(hass):
     assert http == loader.get_component(hass, 'http')
 
 
-def test_component_dependencies(hass):
+async def test_component_dependencies(hass):
     """Test if we can get the proper load order of components."""
-    loader.set_component(hass, 'mod1', MockModule('mod1'))
-    loader.set_component(hass, 'mod2', MockModule('mod2', ['mod1']))
-    loader.set_component(hass, 'mod3', MockModule('mod3', ['mod2']))
+    mock_integration(hass, MockModule('mod1'))
+    mock_integration(hass, MockModule('mod2', ['mod1']))
+    mock_integration(hass, MockModule('mod3', ['mod2']))
 
     assert {'mod1', 'mod2', 'mod3'} == \
-        loader.component_dependencies(hass, 'mod3')
+        await loader.async_component_dependencies(hass, 'mod3')
 
     # Create circular dependency
-    loader.set_component(hass, 'mod1', MockModule('mod1', ['mod3']))
+    mock_integration(hass, MockModule('mod1', ['mod3']))
 
     with pytest.raises(loader.CircularDependency):
-        print(loader.component_dependencies(hass, 'mod3'))
+        print(await loader.async_component_dependencies(hass, 'mod3'))
 
     # Depend on non-existing component
-    loader.set_component(hass, 'mod1',
-                         MockModule('mod1', ['nonexisting']))
+    mock_integration(hass, MockModule('mod1', ['nonexisting']))
 
-    with pytest.raises(loader.ComponentNotFound):
-        print(loader.component_dependencies(hass, 'mod1'))
+    with pytest.raises(loader.IntegrationNotFound):
+        print(await loader.async_component_dependencies(hass, 'mod1'))
 
     # Try to get dependencies for non-existing component
-    with pytest.raises(loader.ComponentNotFound):
-        print(loader.component_dependencies(hass, 'nonexisting'))
+    with pytest.raises(loader.IntegrationNotFound):
+        print(await loader.async_component_dependencies(hass, 'nonexisting'))
 
 
 def test_component_loader(hass):
@@ -142,3 +142,39 @@ async def test_get_platform_enforces_component_path(hass, caplog):
     assert loader.get_platform(hass, 'comp_path_test', 'hue') is None
     assert ('Search path was limited to path of component: '
             'homeassistant.components') in caplog.text
+
+
+async def test_get_integration(hass):
+    """Test resolving integration."""
+    integration = await loader.async_get_integration(hass, 'hue')
+    assert hue == integration.get_component()
+    assert hue_light == integration.get_platform('light')
+
+
+async def test_get_integration_legacy(hass):
+    """Test resolving integration."""
+    integration = await loader.async_get_integration(hass, 'test_embedded')
+    assert integration.get_component().DOMAIN == 'test_embedded'
+    assert integration.get_platform('switch') is not None
+
+
+async def test_get_integration_custom_component(hass):
+    """Test resolving integration."""
+    integration = await loader.async_get_integration(hass, 'test_package')
+    print(integration)
+    assert integration.get_component().DOMAIN == 'test_package'
+    assert integration.name == 'Test Package'
+
+
+def test_integration_properties(hass):
+    """Test integration properties."""
+    integration = loader.Integration(hass, 'homeassistant.components.hue', {
+        'name': 'Philips Hue',
+        'domain': 'hue',
+        'dependencies': ['test-dep'],
+        'requirements': ['test-req==1.0.0'],
+    })
+    assert integration.name == "Philips Hue"
+    assert integration.domain == 'hue'
+    assert integration.dependencies == ['test-dep']
+    assert integration.requirements == ['test-req==1.0.0']

tests/test_setup.py

@@ -20,7 +20,7 @@ from homeassistant.helpers import discovery
 
 from tests.common import \
     get_test_home_assistant, MockModule, MockPlatform, \
-    assert_setup_component, get_test_config_dir
+    assert_setup_component, get_test_config_dir, mock_integration
 
 ORIG_TIMEZONE = dt_util.DEFAULT_TIME_ZONE
 VERSION_PATH = os.path.join(get_test_config_dir(), config_util.VERSION_FILE)
@@ -378,18 +378,18 @@ class TestSetup:
 
     def test_component_not_setup_missing_dependencies(self):
         """Test we do not set up a component if not all dependencies loaded."""
-        deps = ['non_existing']
-        loader.set_component(
-            self.hass, 'comp', MockModule('comp', dependencies=deps))
+        deps = ['maybe_existing']
+        mock_integration(self.hass, MockModule('comp', dependencies=deps))
 
         assert not setup.setup_component(self.hass, 'comp', {})
         assert 'comp' not in self.hass.config.components
 
         self.hass.data.pop(setup.DATA_SETUP)
 
-        loader.set_component(
-            self.hass, 'non_existing', MockModule('non_existing'))
-        assert setup.setup_component(self.hass, 'comp', {})
+        mock_integration(self.hass, MockModule('comp2', dependencies=deps))
+        mock_integration(self.hass, MockModule('maybe_existing'))
+
+        assert setup.setup_component(self.hass, 'comp2', {})
 
     def test_component_failing_setup(self):
         """Test component that fails setup."""

tests/testing_config/custom_components/test_embedded/__init__.py

@@ -1,4 +1,5 @@
 """Component with embedded platforms."""
+DOMAIN = 'test_embedded'
 
 
 async def async_setup(hass, config):

tests/testing_config/custom_components/test_package/manifest.json

@@ -0,0 +1,8 @@
+{
+  "domain": "test_package",
+  "name": "Test Package",
+  "documentation": "http://test-package.io",
+  "requirements": [],
+  "dependencies": [],
+  "codeowners": []
+}