Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2023.7.1 #96006

Merged
merged 18 commits into from
Jul 6, 2023
Merged

2023.7.1 #96006

Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Handle integrations with empty services or failing to load during ser…
…vice description enumeration (#95911)

* wip

* tweaks

* tweaks

* add coverage

* complain loudly as we never execpt this to happen

* ensure not None

* comment it
bdraco authored and balloob committed Jul 6, 2023

Verified

This commit was signed with the committer’s verified signature.
commit 3e19fba7d359136bf21d3712ebda55df0593c70a
61 changes: 33 additions & 28 deletions homeassistant/helpers/service.py
Original file line number Diff line number Diff line change
@@ -566,67 +566,70 @@ async def async_get_all_descriptions(
hass: HomeAssistant,
) -> dict[str, dict[str, Any]]:
"""Return descriptions (i.e. user documentation) for all service calls."""
descriptions_cache = hass.data.setdefault(SERVICE_DESCRIPTION_CACHE, {})
descriptions_cache: dict[
tuple[str, str], dict[str, Any] | None
] = hass.data.setdefault(SERVICE_DESCRIPTION_CACHE, {})
services = hass.services.async_services()

# See if there are new services not seen before.
# Any service that we saw before already has an entry in description_cache.
missing = set()
all_services = []
for domain in services:
for service in services[domain]:
cache_key = (domain, service)
for service_name in services[domain]:
cache_key = (domain, service_name)
all_services.append(cache_key)
if cache_key not in descriptions_cache:
missing.add(domain)

# If we have a complete cache, check if it is still valid
if ALL_SERVICE_DESCRIPTIONS_CACHE in hass.data:
previous_all_services, previous_descriptions_cache = hass.data[
ALL_SERVICE_DESCRIPTIONS_CACHE
]
if all_cache := hass.data.get(ALL_SERVICE_DESCRIPTIONS_CACHE):
previous_all_services, previous_descriptions_cache = all_cache
# If the services are the same, we can return the cache
if previous_all_services == all_services:
return cast(dict[str, dict[str, Any]], previous_descriptions_cache)

# Files we loaded for missing descriptions
loaded = {}
loaded: dict[str, JSON_TYPE] = {}

if missing:
ints_or_excs = await async_get_integrations(hass, missing)
integrations = [
int_or_exc
for int_or_exc in ints_or_excs.values()
if isinstance(int_or_exc, Integration)
]

integrations: list[Integration] = []
for domain, int_or_exc in ints_or_excs.items():
if type(int_or_exc) is Integration: # pylint: disable=unidiomatic-typecheck
integrations.append(int_or_exc)
continue
if TYPE_CHECKING:
assert isinstance(int_or_exc, Exception)
_LOGGER.error("Failed to load integration: %s", domain, exc_info=int_or_exc)
contents = await hass.async_add_executor_job(
_load_services_files, hass, integrations
)

for domain, content in zip(missing, contents):
loaded[domain] = content
loaded = dict(zip(missing, contents))

# Build response
descriptions: dict[str, dict[str, Any]] = {}
for domain in services:
for domain, services_map in services.items():
descriptions[domain] = {}
domain_descriptions = descriptions[domain]

for service in services[domain]:
cache_key = (domain, service)
for service_name in services_map:
cache_key = (domain, service_name)
description = descriptions_cache.get(cache_key)

# Cache missing descriptions
if description is None:
domain_yaml = loaded[domain]
domain_yaml = loaded.get(domain) or {}
# The YAML may be empty for dynamically defined
# services (ie shell_command) that never call
# service.async_set_service_schema for the dynamic
# service

yaml_description = domain_yaml.get( # type: ignore[union-attr]
service, {}
service_name, {}
)

# Don't warn for missing services, because it triggers false
# positives for things like scripts, that register as a service

description = {
"name": yaml_description.get("name", ""),
"description": yaml_description.get("description", ""),
@@ -637,15 +640,15 @@ async def async_get_all_descriptions(
description["target"] = yaml_description["target"]

if (
response := hass.services.supports_response(domain, service)
response := hass.services.supports_response(domain, service_name)
) != SupportsResponse.NONE:
description["response"] = {
"optional": response == SupportsResponse.OPTIONAL,
}

descriptions_cache[cache_key] = description

descriptions[domain][service] = description
domain_descriptions[service_name] = description

hass.data[ALL_SERVICE_DESCRIPTIONS_CACHE] = (all_services, descriptions)
return descriptions
@@ -667,7 +670,9 @@ def async_set_service_schema(
hass: HomeAssistant, domain: str, service: str, schema: dict[str, Any]
) -> None:
"""Register a description for a service."""
hass.data.setdefault(SERVICE_DESCRIPTION_CACHE, {})
descriptions_cache: dict[
tuple[str, str], dict[str, Any] | None
] = hass.data.setdefault(SERVICE_DESCRIPTION_CACHE, {})

description = {
"name": schema.get("name", ""),
@@ -679,7 +684,7 @@ def async_set_service_schema(
description["target"] = schema["target"]

hass.data.pop(ALL_SERVICE_DESCRIPTIONS_CACHE, None)
hass.data[SERVICE_DESCRIPTION_CACHE][(domain, service)] = description
descriptions_cache[(domain, service)] = description


@bind_hass
102 changes: 102 additions & 0 deletions tests/helpers/test_service.py
Original file line number Diff line number Diff line change
@@ -605,6 +605,108 @@ async def test_async_get_all_descriptions(hass: HomeAssistant) -> None:
assert await service.async_get_all_descriptions(hass) is descriptions


async def test_async_get_all_descriptions_failing_integration(
hass: HomeAssistant, caplog: pytest.LogCaptureFixture
) -> None:
"""Test async_get_all_descriptions when async_get_integrations returns an exception."""
group = hass.components.group
group_config = {group.DOMAIN: {}}
await async_setup_component(hass, group.DOMAIN, group_config)
descriptions = await service.async_get_all_descriptions(hass)

assert len(descriptions) == 1

assert "description" in descriptions["group"]["reload"]
assert "fields" in descriptions["group"]["reload"]

logger = hass.components.logger
logger_config = {logger.DOMAIN: {}}
await async_setup_component(hass, logger.DOMAIN, logger_config)
with patch(
"homeassistant.helpers.service.async_get_integrations",
return_value={"logger": ImportError},
):
descriptions = await service.async_get_all_descriptions(hass)

assert len(descriptions) == 2
assert "Failed to load integration: logger" in caplog.text

# Services are empty defaults if the load fails but should
# not raise
assert descriptions[logger.DOMAIN]["set_level"] == {
"description": "",
"fields": {},
"name": "",
}

hass.services.async_register(logger.DOMAIN, "new_service", lambda x: None, None)
service.async_set_service_schema(
hass, logger.DOMAIN, "new_service", {"description": "new service"}
)
descriptions = await service.async_get_all_descriptions(hass)
assert "description" in descriptions[logger.DOMAIN]["new_service"]
assert descriptions[logger.DOMAIN]["new_service"]["description"] == "new service"

hass.services.async_register(
logger.DOMAIN, "another_new_service", lambda x: None, None
)
hass.services.async_register(
logger.DOMAIN,
"service_with_optional_response",
lambda x: None,
None,
SupportsResponse.OPTIONAL,
)
hass.services.async_register(
logger.DOMAIN,
"service_with_only_response",
lambda x: None,
None,
SupportsResponse.ONLY,
)

descriptions = await service.async_get_all_descriptions(hass)
assert "another_new_service" in descriptions[logger.DOMAIN]
assert "service_with_optional_response" in descriptions[logger.DOMAIN]
assert descriptions[logger.DOMAIN]["service_with_optional_response"][
"response"
] == {"optional": True}
assert "service_with_only_response" in descriptions[logger.DOMAIN]
assert descriptions[logger.DOMAIN]["service_with_only_response"]["response"] == {
"optional": False
}

# Verify the cache returns the same object
assert await service.async_get_all_descriptions(hass) is descriptions


async def test_async_get_all_descriptions_dynamically_created_services(
hass: HomeAssistant, caplog: pytest.LogCaptureFixture
) -> None:
"""Test async_get_all_descriptions when async_get_integrations when services are dynamic."""
group = hass.components.group
group_config = {group.DOMAIN: {}}
await async_setup_component(hass, group.DOMAIN, group_config)
descriptions = await service.async_get_all_descriptions(hass)

assert len(descriptions) == 1

assert "description" in descriptions["group"]["reload"]
assert "fields" in descriptions["group"]["reload"]

shell_command = hass.components.shell_command
shell_command_config = {shell_command.DOMAIN: {"test_service": "ls /bin"}}
await async_setup_component(hass, shell_command.DOMAIN, shell_command_config)
descriptions = await service.async_get_all_descriptions(hass)

assert len(descriptions) == 2
assert descriptions[shell_command.DOMAIN]["test_service"] == {
"description": "",
"fields": {},
"name": "",
}


async def test_call_with_required_features(hass: HomeAssistant, mock_entities) -> None:
"""Test service calls invoked only if entity has required features."""
test_service_mock = AsyncMock(return_value=None)