From 812afc1bd0df5b6f798d46b28db91e901424e0ae Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Wed, 28 Feb 2024 18:45:51 -0800 Subject: [PATCH] Fix calendar trigger to survive config entry reloads (#111334) * Fix calendar trigger to survive config entry reloads * Apply suggestions from code review --------- Co-authored-by: Martin Hjelmare --- homeassistant/components/calendar/trigger.py | 31 ++++++--- tests/components/calendar/conftest.py | 40 ++++++----- tests/components/calendar/test_init.py | 7 +- tests/components/calendar/test_recorder.py | 9 ++- tests/components/calendar/test_trigger.py | 71 ++++++++++++++++++-- 5 files changed, 119 insertions(+), 39 deletions(-) diff --git a/homeassistant/components/calendar/trigger.py b/homeassistant/components/calendar/trigger.py index 073c41fc0df..e4fe5d22efd 100644 --- a/homeassistant/components/calendar/trigger.py +++ b/homeassistant/components/calendar/trigger.py @@ -91,11 +91,24 @@ EventFetcher = Callable[[Timespan], Awaitable[list[CalendarEvent]]] QueuedEventFetcher = Callable[[Timespan], Awaitable[list[QueuedCalendarEvent]]] -def event_fetcher(hass: HomeAssistant, entity: CalendarEntity) -> EventFetcher: +def get_entity(hass: HomeAssistant, entity_id: str) -> CalendarEntity: + """Get the calendar entity for the provided entity_id.""" + component: EntityComponent[CalendarEntity] = hass.data[DOMAIN] + if not (entity := component.get_entity(entity_id)) or not isinstance( + entity, CalendarEntity + ): + raise HomeAssistantError( + f"Entity does not exist {entity_id} or is not a calendar entity" + ) + return entity + + +def event_fetcher(hass: HomeAssistant, entity_id: str) -> EventFetcher: """Build an async_get_events wrapper to fetch events during a time span.""" async def async_get_events(timespan: Timespan) -> list[CalendarEvent]: """Return events active in the specified time span.""" + entity = get_entity(hass, entity_id) # Expand by one second to make the end time exclusive end_time = timespan.end + datetime.timedelta(seconds=1) return await entity.async_get_events(hass, timespan.start, end_time) @@ -237,7 +250,10 @@ class CalendarEventListener: self._dispatch_events(now) self._clear_event_listener() self._timespan = self._timespan.next_upcoming(now, UPDATE_INTERVAL) - self._events.extend(await self._fetcher(self._timespan)) + try: + self._events.extend(await self._fetcher(self._timespan)) + except HomeAssistantError as ex: + _LOGGER.error("Calendar trigger failed to fetch events: %s", ex) self._listen_next_calendar_event() @@ -252,13 +268,8 @@ async def async_attach_trigger( event_type = config[CONF_EVENT] offset = config[CONF_OFFSET] - component: EntityComponent[CalendarEntity] = hass.data[DOMAIN] - if not (entity := component.get_entity(entity_id)) or not isinstance( - entity, CalendarEntity - ): - raise HomeAssistantError( - f"Entity does not exist {entity_id} or is not a calendar entity" - ) + # Validate the entity id is valid + get_entity(hass, entity_id) trigger_data = { **trigger_info["trigger_data"], @@ -270,7 +281,7 @@ async def async_attach_trigger( hass, HassJob(action), trigger_data, - queued_event_fetcher(event_fetcher(hass, entity), event_type, offset), + queued_event_fetcher(event_fetcher(hass, entity_id), event_type, offset), ) await listener.async_attach() return listener.async_detach diff --git a/tests/components/calendar/conftest.py b/tests/components/calendar/conftest.py index f42cc6fd508..29d4bb9f5ff 100644 --- a/tests/components/calendar/conftest.py +++ b/tests/components/calendar/conftest.py @@ -99,8 +99,20 @@ def config_flow_fixture(hass: HomeAssistant) -> Generator[None, None, None]: yield +@pytest.fixture(name="config_entry") +async def mock_config_entry(hass: HomeAssistant) -> MockConfigEntry: + """Create a mock config entry.""" + config_entry = MockConfigEntry(domain=TEST_DOMAIN) + config_entry.add_to_hass(hass) + return config_entry + + @pytest.fixture -def mock_setup_integration(hass: HomeAssistant, config_flow_fixture: None) -> None: +def mock_setup_integration( + hass: HomeAssistant, + config_flow_fixture: None, + test_entities: list[CalendarEntity], +) -> None: """Fixture to set up a mock integration.""" async def async_setup_entry_init( @@ -129,20 +141,16 @@ def mock_setup_integration(hass: HomeAssistant, config_flow_fixture: None) -> No ), ) - -async def create_mock_platform( - hass: HomeAssistant, - entities: list[CalendarEntity], -) -> MockConfigEntry: - """Create a calendar platform with the specified entities.""" - async def async_setup_entry_platform( hass: HomeAssistant, config_entry: ConfigEntry, async_add_entities: AddEntitiesCallback, ) -> None: """Set up test event platform via config entry.""" - async_add_entities(entities) + new_entities = create_test_entities() + test_entities.clear() + test_entities.extend(new_entities) + async_add_entities(test_entities) mock_platform( hass, @@ -150,17 +158,15 @@ async def create_mock_platform( MockPlatform(async_setup_entry=async_setup_entry_platform), ) - config_entry = MockConfigEntry(domain=TEST_DOMAIN) - config_entry.add_to_hass(hass) - assert await hass.config_entries.async_setup(config_entry.entry_id) - await hass.async_block_till_done() - - return config_entry - @pytest.fixture(name="test_entities") def mock_test_entities() -> list[MockCalendarEntity]: - """Fixture to create fake entities used in the test.""" + """Fixture that holdes the fake entities created during the test.""" + return [] + + +def create_test_entities() -> list[MockCalendarEntity]: + """Create test entities used during the test.""" half_hour_from_now = dt_util.now() + datetime.timedelta(minutes=30) entity1 = MockCalendarEntity( "Calendar 1", diff --git a/tests/components/calendar/test_init.py b/tests/components/calendar/test_init.py index 52d5855271d..d786ce8d8ad 100644 --- a/tests/components/calendar/test_init.py +++ b/tests/components/calendar/test_init.py @@ -21,7 +21,7 @@ from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.issue_registry import IssueRegistry import homeassistant.util.dt as dt_util -from .conftest import TEST_DOMAIN, MockCalendarEntity, create_mock_platform +from .conftest import TEST_DOMAIN, MockCalendarEntity, MockConfigEntry from tests.typing import ClientSessionGenerator, WebSocketGenerator @@ -51,10 +51,11 @@ async def mock_setup_platform( set_time_zone: Any, frozen_time: Any, mock_setup_integration: Any, - test_entities: list[MockCalendarEntity], + config_entry: MockConfigEntry, ) -> None: """Fixture to setup platforms used in the test and fixtures are set up in the right order.""" - await create_mock_platform(hass, test_entities) + assert await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() async def test_events_http_api( diff --git a/tests/components/calendar/test_recorder.py b/tests/components/calendar/test_recorder.py index 441d757aa4e..ef6c7658a89 100644 --- a/tests/components/calendar/test_recorder.py +++ b/tests/components/calendar/test_recorder.py @@ -10,9 +10,7 @@ from homeassistant.const import ATTR_FRIENDLY_NAME from homeassistant.core import HomeAssistant from homeassistant.util import dt as dt_util -from .conftest import MockCalendarEntity, create_mock_platform - -from tests.common import async_fire_time_changed +from tests.common import MockConfigEntry, async_fire_time_changed from tests.components.recorder.common import async_wait_recording_done @@ -22,10 +20,11 @@ async def mock_setup_dependencies( hass: HomeAssistant, set_time_zone: Any, mock_setup_integration: None, - test_entities: list[MockCalendarEntity], + config_entry: MockConfigEntry, ) -> None: """Fixture that ensures the recorder is setup in the right order.""" - await create_mock_platform(hass, test_entities) + assert await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() async def test_exclude_attributes(hass: HomeAssistant) -> None: diff --git a/tests/components/calendar/test_trigger.py b/tests/components/calendar/test_trigger.py index 120d2e8bfca..0111f11c27b 100644 --- a/tests/components/calendar/test_trigger.py +++ b/tests/components/calendar/test_trigger.py @@ -27,9 +27,9 @@ from homeassistant.core import HomeAssistant from homeassistant.setup import async_setup_component import homeassistant.util.dt as dt_util -from .conftest import MockCalendarEntity, create_mock_platform +from .conftest import MockCalendarEntity -from tests.common import async_fire_time_changed, async_mock_service +from tests.common import MockConfigEntry, async_fire_time_changed, async_mock_service _LOGGER = logging.getLogger(__name__) @@ -105,10 +105,11 @@ def mock_test_entity(test_entities: list[MockCalendarEntity]) -> MockCalendarEnt async def mock_setup_platform( hass: HomeAssistant, mock_setup_integration: Any, - test_entities: list[MockCalendarEntity], + config_entry: MockConfigEntry, ) -> None: """Fixture to setup platforms used in the test.""" - await create_mock_platform(hass, test_entities) + assert await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() @asynccontextmanager @@ -745,3 +746,65 @@ async def test_event_start_trigger_dst( "calendar_event": event3_data, }, ] + + +async def test_config_entry_reload( + hass: HomeAssistant, + calls: Callable[[], list[dict[str, Any]]], + fake_schedule: FakeSchedule, + test_entities: list[MockCalendarEntity], + setup_platform: None, + config_entry: MockConfigEntry, +) -> None: + """Test the a calendar trigger after a config entry reload. + + This sets ups a config entry, sets up an automation for an entity in that + config entry, then reloads the config entry. This reproduces a bug where + the automation kept a reference to the specific entity which would be + invalid after a config entry was reloaded. + """ + async with create_automation(hass, EVENT_START): + assert len(calls()) == 0 + + assert await hass.config_entries.async_reload(config_entry.entry_id) + + # Ensure the reloaded entity has events upcoming. + test_entity = test_entities[1] + event_data = test_entity.create_event( + start=datetime.datetime.fromisoformat("2022-04-19 11:00:00+00:00"), + end=datetime.datetime.fromisoformat("2022-04-19 11:30:00+00:00"), + ) + + await fake_schedule.fire_until( + datetime.datetime.fromisoformat("2022-04-19 11:15:00+00:00"), + ) + + assert calls() == [ + { + "platform": "calendar", + "event": EVENT_START, + "calendar_event": event_data, + } + ] + + +async def test_config_entry_unload( + hass: HomeAssistant, + calls: Callable[[], list[dict[str, Any]]], + fake_schedule: FakeSchedule, + test_entities: list[MockCalendarEntity], + setup_platform: None, + config_entry: MockConfigEntry, + caplog: pytest.LogCaptureFixture, +) -> None: + """Test an automation that references a calendar entity that is unloaded.""" + async with create_automation(hass, EVENT_START): + assert len(calls()) == 0 + + assert await hass.config_entries.async_unload(config_entry.entry_id) + + await fake_schedule.fire_until( + datetime.datetime.fromisoformat("2022-04-19 11:15:00+00:00"), + ) + + assert "Entity does not exist calendar.calendar_2" in caplog.text