mirror of
https://github.com/home-assistant/core.git
synced 2026-08-03 20:24:55 +02:00
Address review: preserve opt-outs when pruning, test read-only lookup
- Only prune stale legacy records whose domain can never be exposed by default, so an explicit `should_expose: False` opt-out on a default-exposed domain (e.g. a light) is not silently reversed if the entity returns. - Add a test that async_get_should_expose does not persist a record for either the registry-backed or legacy path, contrasting with async_should_expose. - Extend the prune test with an absent opted-out light that must be kept. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -65,6 +65,10 @@ DEFAULT_EXPOSED_SENSOR_DEVICE_CLASSES = {
|
||||
SensorDeviceClass.VOLATILE_ORGANIC_COMPOUNDS,
|
||||
}
|
||||
|
||||
# Domains that can be exposed by default via their device class, so a stored
|
||||
# "should_expose: False" for one may be a user opt-out rather than noise.
|
||||
DEVICE_CLASS_EXPOSED_DOMAINS = {"binary_sensor", "sensor"}
|
||||
|
||||
DEFAULT_EXPOSED_ASSISTANT = {
|
||||
"conversation": True,
|
||||
}
|
||||
@@ -138,8 +142,8 @@ class ExposedEntities:
|
||||
accumulate indefinitely.
|
||||
|
||||
Only drop records for entities that no longer exist (neither a current
|
||||
state nor in the entity registry) and that are not exposed to any
|
||||
assistant, so an explicit "expose this entity" setting is never lost.
|
||||
state nor in the entity registry) and that carry no user intent, so an
|
||||
explicit exposure choice is never lost.
|
||||
"""
|
||||
entity_registry = er.async_get(self._hass)
|
||||
stale_entity_ids = [
|
||||
@@ -147,10 +151,7 @@ class ExposedEntities:
|
||||
for entity_id, exposed_entity in self.entities.items()
|
||||
if self._hass.states.get(entity_id) is None
|
||||
and entity_id not in entity_registry.entities
|
||||
and not any(
|
||||
settings.get("should_expose")
|
||||
for settings in exposed_entity.assistants.values()
|
||||
)
|
||||
and not self._legacy_entity_has_user_intent(entity_id, exposed_entity)
|
||||
]
|
||||
if not stale_entity_ids:
|
||||
return
|
||||
@@ -163,6 +164,29 @@ class ExposedEntities:
|
||||
)
|
||||
self._async_schedule_save()
|
||||
|
||||
@callback
|
||||
def _legacy_entity_has_user_intent(
|
||||
self, entity_id: str, exposed_entity: ExposedEntity
|
||||
) -> bool:
|
||||
"""Return True if a legacy record holds an exposure choice worth keeping."""
|
||||
if any(
|
||||
settings.get("should_expose")
|
||||
for settings in exposed_entity.assistants.values()
|
||||
):
|
||||
# Explicitly exposed to at least one assistant
|
||||
return True
|
||||
|
||||
# A stored "should_expose: False" is indistinguishable from an
|
||||
# auto-created default. It is only meaningful as an opt-out if the
|
||||
# entity could be exposed by default, which depends on its domain (and,
|
||||
# for sensors, a device class we cannot know while it is absent). Keep
|
||||
# the record when the domain can be default-exposed to avoid silently
|
||||
# reversing an opt-out if the entity returns.
|
||||
domain = split_entity_id(entity_id)[0]
|
||||
return (
|
||||
domain in DEFAULT_EXPOSED_DOMAINS or domain in DEVICE_CLASS_EXPOSED_DOMAINS
|
||||
)
|
||||
|
||||
@callback
|
||||
def async_listen_entity_updates(
|
||||
self, assistant: str, listener: Callable[[], None]
|
||||
|
||||
@@ -11,6 +11,7 @@ from homeassistant.components.homeassistant.exposed_entities import (
|
||||
async_expose_entity,
|
||||
async_get_assistant_settings,
|
||||
async_get_entity_settings,
|
||||
async_get_should_expose,
|
||||
async_listen_entity_updates,
|
||||
async_should_expose,
|
||||
)
|
||||
@@ -147,15 +148,56 @@ async def test_prune_legacy_entities_on_start(hass: HomeAssistant) -> None:
|
||||
exposed_entities.entities["light.gone_but_exposed"] = ExposedEntity(
|
||||
assistants={"conversation": {"should_expose": True}}
|
||||
)
|
||||
# Explicit opt-out for a default-exposed domain that is currently absent;
|
||||
# keep it so the choice is not reversed if the entity returns.
|
||||
exposed_entities.entities["light.gone_opted_out"] = ExposedEntity(
|
||||
assistants={"conversation": {"should_expose": False}}
|
||||
)
|
||||
|
||||
exposed_entities._async_prune_legacy_entities(hass)
|
||||
|
||||
assert set(exposed_entities.entities) == {
|
||||
"geo_location.live_strike",
|
||||
"light.gone_but_exposed",
|
||||
"light.gone_opted_out",
|
||||
}
|
||||
|
||||
|
||||
async def test_get_should_expose_does_not_persist(
|
||||
hass: HomeAssistant,
|
||||
entity_registry: er.EntityRegistry,
|
||||
) -> None:
|
||||
"""Test async_get_should_expose evaluates exposure without persisting."""
|
||||
assert await async_setup_component(hass, DOMAIN, {})
|
||||
|
||||
exposed_entities: ExposedEntities = hass.data[DATA_EXPOSED_ENTITIES]
|
||||
|
||||
# Registry entity without a conversation option (light is exposed by default)
|
||||
registry_light = entity_registry.async_get_or_create("light", "demo", "1234")
|
||||
assert "conversation" not in registry_light.options
|
||||
|
||||
# Legacy entity (no registry entry) without a stored record
|
||||
legacy_entity_id = "light.legacy"
|
||||
|
||||
# The read-only lookup returns the default but writes nothing, for both the
|
||||
# registry-backed and legacy paths.
|
||||
assert async_get_should_expose(hass, "conversation", registry_light.entity_id)
|
||||
assert async_get_should_expose(hass, "conversation", legacy_entity_id)
|
||||
assert (
|
||||
"conversation"
|
||||
not in entity_registry.async_get(registry_light.entity_id).options
|
||||
)
|
||||
assert legacy_entity_id not in exposed_entities.entities
|
||||
|
||||
# The writing variant does persist, for contrast.
|
||||
assert async_should_expose(hass, "conversation", registry_light.entity_id)
|
||||
assert async_should_expose(hass, "conversation", legacy_entity_id)
|
||||
assert entity_registry.async_get(registry_light.entity_id).options[
|
||||
"conversation"
|
||||
] == {"should_expose": True}
|
||||
assert legacy_entity_id in exposed_entities.entities
|
||||
|
||||
|
||||
async def test_expose_entity(
|
||||
hass: HomeAssistant,
|
||||
entity_registry: er.EntityRegistry,
|
||||
|
||||
Reference in New Issue
Block a user