From 8adcd10f55f5df77382029ca7ff94260612fbd91 Mon Sep 17 00:00:00 2001 From: Allen Porter <allen@thebends.org> Date: Tue, 8 Mar 2022 12:56:49 -0800 Subject: [PATCH] Make google calendar loading API centric, not loading from yaml (#67722) * Make google calendar loading API centric, not loading from yaml Update the behavior for google calendar to focus on loading calendars based on the API and using the yaml configuration to override behavior. The old behavior was to first load from yaml, then also load from the API, which is atypical. This is pulled out from a larger change to rewrite calendar using async and config flows. Tests needed to be updated to reflect the new API centric behavior, and changing the API call ordering required changing tests that exercise failures. * Update to use async_fire_time_changed to invoke updates --- homeassistant/components/google/__init__.py | 41 ++++++---- tests/components/google/conftest.py | 2 +- tests/components/google/test_calendar.py | 85 +++++++++++++++++--- tests/components/google/test_init.py | 86 ++++++++++++++++++--- 4 files changed, 177 insertions(+), 37 deletions(-) diff --git a/homeassistant/components/google/__init__.py b/homeassistant/components/google/__init__.py index be973ac9b45..87d76b9f4c8 100644 --- a/homeassistant/components/google/__init__.py +++ b/homeassistant/components/google/__init__.py @@ -74,7 +74,6 @@ SERVICE_SCAN_CALENDARS = "scan_for_calendars" SERVICE_FOUND_CALENDARS = "found_calendar" SERVICE_ADD_EVENT = "add_event" -DATA_CALENDARS = "calendars" DATA_SERVICE = "service" YAML_DEVICES = f"{DOMAIN}_calendars.yaml" @@ -257,7 +256,6 @@ def setup(hass: HomeAssistant, config: ConfigType) -> bool: storage = Storage(hass.config.path(TOKEN_FILE)) hass.data[DOMAIN] = { - DATA_CALENDARS: {}, DATA_SERVICE: GoogleCalendarService(hass, storage), } creds = storage.get() @@ -281,14 +279,26 @@ def setup_services( ) -> None: """Set up the service listeners.""" + created_calendars = set() + calendars = load_config(hass.config.path(YAML_DEVICES)) + def _found_calendar(call: ServiceCall) -> None: """Check if we know about a calendar and generate PLATFORM_DISCOVER.""" calendar = get_calendar_info(hass, call.data) calendar_id = calendar[CONF_CAL_ID] - if calendar_id in hass.data[DOMAIN][DATA_CALENDARS]: + + if calendar_id in created_calendars: return - hass.data[DOMAIN][DATA_CALENDARS][calendar_id] = calendar - update_config(hass.config.path(YAML_DEVICES), calendar) + created_calendars.add(calendar_id) + + # Populate the yaml file with all discovered calendars + if calendar_id not in calendars: + calendars[calendar_id] = calendar + update_config(hass.config.path(YAML_DEVICES), calendar) + else: + # Prefer entity/name information from yaml, overriding api + calendar = calendars[calendar_id] + discovery.load_platform( hass, Platform.CALENDAR, @@ -363,17 +373,10 @@ def setup_services( def do_setup(hass: HomeAssistant, hass_config: ConfigType, config: ConfigType) -> None: """Run the setup after we have everything configured.""" - # Load calendars the user has configured - calendars = load_config(hass.config.path(YAML_DEVICES)) - hass.data[DOMAIN][DATA_CALENDARS] = calendars - calendar_service = hass.data[DOMAIN][DATA_SERVICE] setup_services(hass, hass_config, config, calendar_service) - for calendar in calendars.values(): - discovery.load_platform(hass, Platform.CALENDAR, DOMAIN, calendar, hass_config) - - # Look for any new calendars + # Fetch calendars from the API hass.services.call(DOMAIN, SERVICE_SCAN_CALENDARS, None) @@ -410,7 +413,8 @@ def load_config(path: str) -> dict[str, Any]: except VoluptuousError as exception: # keep going _LOGGER.warning("Calendar Invalid Data: %s", exception) - except FileNotFoundError: + except FileNotFoundError as err: + _LOGGER.debug("Error reading calendar configuration: %s", err) # When YAML file could not be loaded/did not contain a dict return {} @@ -419,6 +423,9 @@ def load_config(path: str) -> dict[str, Any]: def update_config(path: str, calendar: dict[str, Any]) -> None: """Write the google_calendar_devices.yaml.""" - with open(path, "a", encoding="utf8") as out: - out.write("\n") - yaml.dump([calendar], out, default_flow_style=False) + try: + with open(path, "a", encoding="utf8") as out: + out.write("\n") + yaml.dump([calendar], out, default_flow_style=False) + except FileNotFoundError as err: + _LOGGER.debug("Error persisting calendar configuration: %s", err) diff --git a/tests/components/google/conftest.py b/tests/components/google/conftest.py index 5e1411c76c8..f3d8f9a28b9 100644 --- a/tests/components/google/conftest.py +++ b/tests/components/google/conftest.py @@ -99,7 +99,7 @@ def calendars_config(calendars_config_entity: dict[str, Any]) -> list[dict[str, ] -@pytest.fixture +@pytest.fixture(autouse=True) async def mock_calendars_yaml( hass: HomeAssistant, calendars_config: list[dict[str, Any]], diff --git a/tests/components/google/test_calendar.py b/tests/components/google/test_calendar.py index 76ab20ff41b..34227ae02b1 100644 --- a/tests/components/google/test_calendar.py +++ b/tests/components/google/test_calendar.py @@ -5,7 +5,7 @@ from __future__ import annotations import datetime from http import HTTPStatus from typing import Any -from unittest.mock import Mock +from unittest.mock import patch import httplib2 import pytest @@ -16,6 +16,8 @@ import homeassistant.util.dt as dt_util from .conftest import TEST_YAML_ENTITY, TEST_YAML_ENTITY_NAME +from tests.common import async_fire_time_changed + TEST_ENTITY = TEST_YAML_ENTITY TEST_ENTITY_NAME = TEST_YAML_ENTITY_NAME @@ -50,8 +52,11 @@ TEST_EVENT = { @pytest.fixture(autouse=True) -def mock_test_setup(mock_calendars_yaml, mock_token_read): +def mock_test_setup( + mock_calendars_yaml, test_api_calendar, mock_calendars_list, mock_token_read +): """Fixture that pulls in the default fixtures for tests in this file.""" + mock_calendars_list({"items": [test_api_calendar]}) return @@ -252,13 +257,72 @@ async def test_all_day_offset_event(hass, mock_events_list_items, component_setu } -async def test_update_error(hass, calendar_resource, component_setup): - """Test that the calendar handles a server error.""" - calendar_resource.return_value.get = Mock( - side_effect=httplib2.ServerNotFoundError("unit test") - ) - assert await component_setup() +async def test_update_error( + hass, calendar_resource, component_setup, test_api_calendar +): + """Test that the calendar update handles a server error.""" + now = dt_util.now() + with patch("homeassistant.components.google.api.google_discovery.build") as mock: + mock.return_value.calendarList.return_value.list.return_value.execute.return_value = { + "items": [test_api_calendar] + } + mock.return_value.events.return_value.list.return_value.execute.return_value = { + "items": [ + { + **TEST_EVENT, + "start": { + "dateTime": (now + datetime.timedelta(minutes=-30)).isoformat() + }, + "end": { + "dateTime": (now + datetime.timedelta(minutes=30)).isoformat() + }, + } + ] + } + assert await component_setup() + + state = hass.states.get(TEST_ENTITY) + assert state.name == TEST_ENTITY_NAME + assert state.state == "on" + + # Advance time to avoid throttling + now += datetime.timedelta(minutes=30) + with patch( + "homeassistant.components.google.api.google_discovery.build", + side_effect=httplib2.ServerNotFoundError("unit test"), + ), patch("homeassistant.util.utcnow", return_value=now): + async_fire_time_changed(hass, now) + await hass.async_block_till_done() + + # No change + state = hass.states.get(TEST_ENTITY) + assert state.name == TEST_ENTITY_NAME + assert state.state == "on" + + # Advance time to avoid throttling + now += datetime.timedelta(minutes=30) + with patch( + "homeassistant.components.google.api.google_discovery.build" + ) as mock, patch("homeassistant.util.utcnow", return_value=now): + + mock.return_value.events.return_value.list.return_value.execute.return_value = { + "items": [ + { + **TEST_EVENT, + "start": { + "dateTime": (now + datetime.timedelta(minutes=30)).isoformat() + }, + "end": { + "dateTime": (now + datetime.timedelta(minutes=60)).isoformat() + }, + } + ] + } + async_fire_time_changed(hass, now) + await hass.async_block_till_done() + + # State updated state = hass.states.get(TEST_ENTITY) assert state.name == TEST_ENTITY_NAME assert state.state == "off" @@ -284,11 +348,12 @@ async def test_http_event_api_failure( hass, hass_client, calendar_resource, component_setup ): """Test the Rest API response during a calendar failure.""" - calendar_resource.side_effect = httplib2.ServerNotFoundError("unit test") - assert await component_setup() client = await hass_client() + + calendar_resource.side_effect = httplib2.ServerNotFoundError("unit test") + response = await client.get(upcoming_event_url()) assert response.status == HTTPStatus.OK # A failure to talk to the server results in an empty list of events diff --git a/tests/components/google/test_init.py b/tests/components/google/test_init.py index a0766c8256e..0f02e20c735 100644 --- a/tests/components/google/test_init.py +++ b/tests/components/google/test_init.py @@ -13,7 +13,11 @@ from oauth2client.client import ( ) import pytest -from homeassistant.components.google import DOMAIN, SERVICE_ADD_EVENT +from homeassistant.components.google import ( + DOMAIN, + SERVICE_ADD_EVENT, + SERVICE_SCAN_CALENDARS, +) from homeassistant.const import STATE_OFF from homeassistant.core import HomeAssistant, State from homeassistant.util.dt import utcnow @@ -109,10 +113,13 @@ async def test_init_success( mock_code_flow: Mock, mock_exchange: Mock, mock_notification: Mock, + mock_calendars_list: ApiResult, + test_api_calendar: dict[str, Any], mock_calendars_yaml: None, component_setup: ComponentSetup, ) -> None: """Test successful creds setup.""" + mock_calendars_list({"items": [test_api_calendar]}) assert await component_setup() # Run one tick to invoke the credential exchange check @@ -199,9 +206,12 @@ async def test_existing_token( mock_token_read: None, component_setup: ComponentSetup, mock_calendars_yaml: None, + mock_calendars_list: ApiResult, + test_api_calendar: dict[str, Any], mock_notification: Mock, ) -> None: """Test setup with an existing token file.""" + mock_calendars_list({"items": [test_api_calendar]}) assert await component_setup() state = hass.states.get(TEST_YAML_ENTITY) @@ -221,11 +231,14 @@ async def test_existing_token_missing_scope( mock_token_read: None, component_setup: ComponentSetup, mock_calendars_yaml: None, + mock_calendars_list: ApiResult, + test_api_calendar: dict[str, Any], mock_notification: Mock, mock_code_flow: Mock, mock_exchange: Mock, ) -> None: """Test setup where existing token does not have sufficient scopes.""" + mock_calendars_list({"items": [test_api_calendar]}) assert await component_setup() # Run one tick to invoke the credential exchange check @@ -279,6 +292,25 @@ async def test_invalid_calendar_yaml( mock_notification.assert_not_called() +async def test_calendar_yaml_error( + hass: HomeAssistant, + mock_token_read: None, + component_setup: ComponentSetup, + mock_calendars_list: ApiResult, + test_api_calendar: dict[str, Any], + mock_notification: Mock, +) -> None: + """Test setup with yaml file not found.""" + + mock_calendars_list({"items": [test_api_calendar]}) + + with patch("homeassistant.components.google.open", side_effect=FileNotFoundError()): + assert await component_setup() + + assert not hass.states.get(TEST_YAML_ENTITY) + assert hass.states.get(TEST_API_ENTITY) + + @pytest.mark.parametrize( "google_config_track_new,calendars_config,expected_state", [ @@ -324,7 +356,6 @@ async def test_track_new( mock_calendars_list({"items": [test_api_calendar]}) assert await component_setup() - # The calendar does not state = hass.states.get(TEST_API_ENTITY) assert_state(state, expected_state) @@ -343,7 +374,6 @@ async def test_found_calendar_from_api( mock_calendars_list({"items": [test_api_calendar]}) assert await component_setup() - # The calendar does not state = hass.states.get(TEST_API_ENTITY) assert state assert state.name == TEST_API_ENTITY_NAME @@ -387,12 +417,6 @@ async def test_calendar_config_track_new( state = hass.states.get(TEST_YAML_ENTITY) assert_state(state, expected_state) - if calendars_config_track: - assert state - assert state.name == TEST_YAML_ENTITY_NAME - assert state.state == STATE_OFF - else: - assert not state async def test_add_event( @@ -573,3 +597,47 @@ async def test_add_event_date_time( }, }, ) + + +async def test_scan_calendars( + hass: HomeAssistant, + mock_token_read: None, + component_setup: ComponentSetup, + mock_calendars_list: ApiResult, + test_api_calendar: dict[str, Any], +) -> None: + """Test finding a calendar from the API.""" + + assert await component_setup() + + calendar_1 = { + "id": "calendar-id-1", + "summary": "Calendar 1", + } + calendar_2 = { + "id": "calendar-id-2", + "summary": "Calendar 2", + } + + mock_calendars_list({"items": [calendar_1]}) + await hass.services.async_call(DOMAIN, SERVICE_SCAN_CALENDARS, {}, blocking=True) + await hass.async_block_till_done() + + state = hass.states.get("calendar.calendar_1") + assert state + assert state.name == "Calendar 1" + assert state.state == STATE_OFF + assert not hass.states.get("calendar.calendar_2") + + mock_calendars_list({"items": [calendar_1, calendar_2]}) + await hass.services.async_call(DOMAIN, SERVICE_SCAN_CALENDARS, {}, blocking=True) + await hass.async_block_till_done() + + state = hass.states.get("calendar.calendar_1") + assert state + assert state.name == "Calendar 1" + assert state.state == STATE_OFF + state = hass.states.get("calendar.calendar_2") + assert state + assert state.name == "Calendar 2" + assert state.state == STATE_OFF