Fix circular dependancy detection (#100458)

* Fix _async_component_dependencies

Fix bug with circular dependency detection
Fix bug with circular after_dependency detection
Simplify interface and make the code more readable

* Implement review feedback

* Pass all conflicting deps to Exception

* Change inner docstring

Co-authored-by: Erik Montnemery <erik@montnemery.com>

---------

Co-authored-by: Erik Montnemery <erik@montnemery.com>
This commit is contained in:
Artur Pragacz 2023-09-28 08:05:00 +02:00 committed by GitHub
parent d41144ee88
commit e1771ae01e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 57 additions and 51 deletions

View File

@ -777,9 +777,7 @@ class Integration:
return self._all_dependencies_resolved
try:
dependencies = await _async_component_dependencies(
self.hass, self.domain, self, set(), set()
)
dependencies = await _async_component_dependencies(self.hass, self)
dependencies.discard(self.domain)
self._all_dependencies = dependencies
self._all_dependencies_resolved = True
@ -998,7 +996,7 @@ class IntegrationNotLoaded(LoaderError):
class CircularDependency(LoaderError):
"""Raised when a circular dependency is found when resolving components."""
def __init__(self, from_domain: str, to_domain: str) -> None:
def __init__(self, from_domain: str | set[str], to_domain: str) -> None:
"""Initialize circular dependency error."""
super().__init__(f"Circular dependency detected: {from_domain} -> {to_domain}.")
self.from_domain = from_domain
@ -1132,43 +1130,40 @@ def bind_hass(func: _CallableT) -> _CallableT:
async def _async_component_dependencies(
hass: HomeAssistant,
start_domain: str,
integration: Integration,
loaded: set[str],
loading: set[str],
) -> set[str]:
"""Recursive function to get component dependencies.
"""Get component dependencies."""
loading = set()
loaded = set()
Async friendly.
"""
async def component_dependencies_impl(integration: Integration) -> None:
"""Recursively get component dependencies."""
domain = integration.domain
loading.add(domain)
for dependency_domain in integration.dependencies:
# Check not already loaded
dep_integration = await async_get_integration(hass, dependency_domain)
# If we are already loading it, we have a circular dependency.
# We have to check it here to make sure that every integration that
# depends on us, does not appear in our own after_dependencies.
if conflict := loading.intersection(dep_integration.after_dependencies):
raise CircularDependency(conflict, dependency_domain)
# If we have already loaded it, no point doing it again.
if dependency_domain in loaded:
continue
# If we are already loading it, we have a circular dependency.
if dependency_domain in loading:
raise CircularDependency(domain, dependency_domain)
raise CircularDependency(dependency_domain, domain)
loaded.add(dependency_domain)
await component_dependencies_impl(dep_integration)
dep_integration = await async_get_integration(hass, dependency_domain)
if start_domain in dep_integration.after_dependencies:
raise CircularDependency(start_domain, dependency_domain)
if dep_integration.dependencies:
dep_loaded = await _async_component_dependencies(
hass, start_domain, dep_integration, loaded, loading
)
loaded.update(dep_loaded)
loaded.add(domain)
loading.remove(domain)
loaded.add(domain)
await component_dependencies_impl(integration)
return loaded

View File

@ -11,35 +11,46 @@ from homeassistant.core import HomeAssistant, callback
from .common import MockModule, async_get_persistent_notifications, mock_integration
async def test_component_dependencies(hass: HomeAssistant) -> None:
"""Test if we can get the proper load order of components."""
async def test_circular_component_dependencies(hass: HomeAssistant) -> None:
"""Test if we can detect circular dependencies of components."""
mock_integration(hass, MockModule("mod1"))
mock_integration(hass, MockModule("mod2", ["mod1"]))
mod_3 = mock_integration(hass, MockModule("mod3", ["mod2"]))
mock_integration(hass, MockModule("mod3", ["mod1"]))
mod_4 = mock_integration(hass, MockModule("mod4", ["mod2", "mod3"]))
assert {"mod1", "mod2", "mod3"} == await loader._async_component_dependencies(
hass, "mod_3", mod_3, set(), set()
)
deps = await loader._async_component_dependencies(hass, mod_4)
assert deps == {"mod1", "mod2", "mod3", "mod4"}
# Create circular dependency
# Create a circular dependency
mock_integration(hass, MockModule("mod1", ["mod4"]))
with pytest.raises(loader.CircularDependency):
await loader._async_component_dependencies(hass, mod_4)
# Create a different circular dependency
mock_integration(hass, MockModule("mod1", ["mod3"]))
with pytest.raises(loader.CircularDependency):
await loader._async_component_dependencies(hass, "mod_3", mod_3, set(), set())
await loader._async_component_dependencies(hass, mod_4)
# Depend on non-existing component
mod_1 = mock_integration(hass, MockModule("mod1", ["nonexisting"]))
with pytest.raises(loader.IntegrationNotFound):
await loader._async_component_dependencies(hass, "mod_1", mod_1, set(), set())
# Having an after dependency 2 deps down that is circular
mod_1 = mock_integration(
hass, MockModule("mod1", partial_manifest={"after_dependencies": ["mod_3"]})
# Create a circular after_dependency
mock_integration(
hass, MockModule("mod1", partial_manifest={"after_dependencies": ["mod4"]})
)
with pytest.raises(loader.CircularDependency):
await loader._async_component_dependencies(hass, "mod_3", mod_3, set(), set())
await loader._async_component_dependencies(hass, mod_4)
# Create a different circular after_dependency
mock_integration(
hass, MockModule("mod1", partial_manifest={"after_dependencies": ["mod3"]})
)
with pytest.raises(loader.CircularDependency):
await loader._async_component_dependencies(hass, mod_4)
async def test_nonexistent_component_dependencies(hass: HomeAssistant) -> None:
"""Test if we can detect nonexistent dependencies of components."""
mod_1 = mock_integration(hass, MockModule("mod1", ["nonexistent"]))
with pytest.raises(loader.IntegrationNotFound):
await loader._async_component_dependencies(hass, mod_1)
def test_component_loader(hass: HomeAssistant) -> None: