From e7e48cd9f68f8523fe1630ff48078a820b5fbbf6 Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Sat, 28 May 2022 20:28:22 -0700 Subject: [PATCH] Defer google calendar integration reload to a task to avoid races of reload during setup (#72608) --- homeassistant/components/google/__init__.py | 29 ++++------ homeassistant/components/google/api.py | 12 +++- tests/components/google/test_config_flow.py | 31 +++++----- tests/components/google/test_init.py | 64 +++++++++++++++++++++ 4 files changed, 101 insertions(+), 35 deletions(-) diff --git a/homeassistant/components/google/__init__.py b/homeassistant/components/google/__init__.py index 1336e9991e3..2a40bfe7043 100644 --- a/homeassistant/components/google/__init__.py +++ b/homeassistant/components/google/__init__.py @@ -217,7 +217,6 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Set up Google from a config entry.""" hass.data.setdefault(DOMAIN, {}) - async_upgrade_entry(hass, entry) implementation = ( await config_entry_oauth2_flow.async_get_config_entry_implementation( hass, entry @@ -240,10 +239,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: except aiohttp.ClientError as err: raise ConfigEntryNotReady from err - access = FeatureAccess[entry.options[CONF_CALENDAR_ACCESS]] - token_scopes = session.token.get("scope", []) - if access.scope not in token_scopes: - _LOGGER.debug("Scope '%s' not in scopes '%s'", access.scope, token_scopes) + if not async_entry_has_scopes(hass, entry): raise ConfigEntryAuthFailed( "Required scopes are not available, reauth required" ) @@ -254,27 +250,21 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: await async_setup_services(hass, calendar_service) # Only expose the add event service if we have the correct permissions - if access is FeatureAccess.read_write: + if get_feature_access(hass, entry) is FeatureAccess.read_write: await async_setup_add_event_service(hass, calendar_service) hass.config_entries.async_setup_platforms(entry, PLATFORMS) - # Reload entry when options are updated entry.async_on_unload(entry.add_update_listener(async_reload_entry)) return True -def async_upgrade_entry(hass: HomeAssistant, entry: ConfigEntry) -> None: - """Upgrade the config entry if needed.""" - if entry.options: - return - hass.config_entries.async_update_entry( - entry, - options={ - CONF_CALENDAR_ACCESS: get_feature_access(hass).name, - }, - ) +def async_entry_has_scopes(hass: HomeAssistant, entry: ConfigEntry) -> bool: + """Verify that the config entry desired scope is present in the oauth token.""" + access = get_feature_access(hass, entry) + token_scopes = entry.data.get("token", {}).get("scope", []) + return access.scope in token_scopes async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: @@ -283,8 +273,9 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: async def async_reload_entry(hass: HomeAssistant, entry: ConfigEntry) -> None: - """Reload the config entry when it changed.""" - await hass.config_entries.async_reload(entry.entry_id) + """Reload config entry if the access options change.""" + if not async_entry_has_scopes(hass, entry): + await hass.config_entries.async_reload(entry.entry_id) async def async_setup_services( diff --git a/homeassistant/components/google/api.py b/homeassistant/components/google/api.py index eeac854a2ae..4bb9de5d581 100644 --- a/homeassistant/components/google/api.py +++ b/homeassistant/components/google/api.py @@ -19,6 +19,7 @@ from oauth2client.client import ( ) from homeassistant.components.application_credentials import AuthImplementation +from homeassistant.config_entries import ConfigEntry from homeassistant.core import CALLBACK_TYPE, HomeAssistant from homeassistant.helpers import config_entry_oauth2_flow from homeassistant.helpers.event import async_track_time_interval @@ -127,8 +128,17 @@ class DeviceFlow: ) -def get_feature_access(hass: HomeAssistant) -> FeatureAccess: +def get_feature_access( + hass: HomeAssistant, config_entry: ConfigEntry | None = None +) -> FeatureAccess: """Return the desired calendar feature access.""" + if ( + config_entry + and config_entry.options + and CONF_CALENDAR_ACCESS in config_entry.options + ): + return FeatureAccess[config_entry.options[CONF_CALENDAR_ACCESS]] + # This may be called during config entry setup without integration setup running when there # is no google entry in configuration.yaml return cast( diff --git a/tests/components/google/test_config_flow.py b/tests/components/google/test_config_flow.py index 77ebe1e56cd..8ac017fcba4 100644 --- a/tests/components/google/test_config_flow.py +++ b/tests/components/google/test_config_flow.py @@ -540,9 +540,15 @@ async def test_options_flow_triggers_reauth( ) -> None: """Test load and unload of a ConfigEntry.""" config_entry.add_to_hass(hass) - await component_setup() + + with patch( + "homeassistant.components.google.async_setup_entry", return_value=True + ) as mock_setup: + await component_setup() + mock_setup.assert_called_once() + assert config_entry.state is ConfigEntryState.LOADED - assert config_entry.options == {"calendar_access": "read_write"} + assert config_entry.options == {} # Default is read_write result = await hass.config_entries.options.async_init(config_entry.entry_id) assert result["type"] == "form" @@ -557,14 +563,7 @@ async def test_options_flow_triggers_reauth( }, ) assert result["type"] == "create_entry" - - await hass.async_block_till_done() assert config_entry.options == {"calendar_access": "read_only"} - # Re-auth flow was initiated because access level changed - assert config_entry.state is ConfigEntryState.SETUP_ERROR - flows = hass.config_entries.flow.async_progress() - assert len(flows) == 1 - assert flows[0]["step_id"] == "reauth_confirm" async def test_options_flow_no_changes( @@ -574,9 +573,15 @@ async def test_options_flow_no_changes( ) -> None: """Test load and unload of a ConfigEntry.""" config_entry.add_to_hass(hass) - await component_setup() + + with patch( + "homeassistant.components.google.async_setup_entry", return_value=True + ) as mock_setup: + await component_setup() + mock_setup.assert_called_once() + assert config_entry.state is ConfigEntryState.LOADED - assert config_entry.options == {"calendar_access": "read_write"} + assert config_entry.options == {} # Default is read_write result = await hass.config_entries.options.async_init(config_entry.entry_id) assert result["type"] == "form" @@ -589,8 +594,4 @@ async def test_options_flow_no_changes( }, ) assert result["type"] == "create_entry" - - await hass.async_block_till_done() assert config_entry.options == {"calendar_access": "read_write"} - # Re-auth flow was initiated because access level changed - assert config_entry.state is ConfigEntryState.LOADED diff --git a/tests/components/google/test_init.py b/tests/components/google/test_init.py index b6f7a6b4cbc..f2cf067f7bb 100644 --- a/tests/components/google/test_init.py +++ b/tests/components/google/test_init.py @@ -102,6 +102,24 @@ async def test_existing_token_missing_scope( assert flows[0]["step_id"] == "reauth_confirm" +@pytest.mark.parametrize("config_entry_options", [{CONF_CALENDAR_ACCESS: "read_only"}]) +async def test_config_entry_scope_reauth( + hass: HomeAssistant, + token_scopes: list[str], + component_setup: ComponentSetup, + config_entry: MockConfigEntry, +) -> None: + """Test setup where the config entry options requires reauth to match the scope.""" + config_entry.add_to_hass(hass) + assert await component_setup() + + assert config_entry.state is ConfigEntryState.SETUP_ERROR + + flows = hass.config_entries.flow.async_progress() + assert len(flows) == 1 + assert flows[0]["step_id"] == "reauth_confirm" + + @pytest.mark.parametrize("calendars_config", [[{"cal_id": "invalid-schema"}]]) async def test_calendar_yaml_missing_required_fields( hass: HomeAssistant, @@ -629,3 +647,49 @@ async def test_calendar_yaml_update( # No yaml config loaded that overwrites the entity name assert not hass.states.get(TEST_YAML_ENTITY) + + +async def test_update_will_reload( + hass: HomeAssistant, + component_setup: ComponentSetup, + setup_config_entry: Any, + mock_calendars_list: ApiResult, + test_api_calendar: dict[str, Any], + mock_events_list: ApiResult, + config_entry: MockConfigEntry, +) -> None: + """Test updating config entry options will trigger a reload.""" + mock_calendars_list({"items": [test_api_calendar]}) + mock_events_list({}) + await component_setup() + assert config_entry.state is ConfigEntryState.LOADED + assert config_entry.options == {} # read_write is default + + with patch( + "homeassistant.config_entries.ConfigEntries.async_reload", + return_value=None, + ) as mock_reload: + # No-op does not reload + hass.config_entries.async_update_entry( + config_entry, options={CONF_CALENDAR_ACCESS: "read_write"} + ) + await hass.async_block_till_done() + mock_reload.assert_not_called() + + # Data change does not trigger reload + hass.config_entries.async_update_entry( + config_entry, + data={ + **config_entry.data, + "example": "field", + }, + ) + await hass.async_block_till_done() + mock_reload.assert_not_called() + + # Reload when options changed + hass.config_entries.async_update_entry( + config_entry, options={CONF_CALENDAR_ACCESS: "read_only"} + ) + await hass.async_block_till_done() + mock_reload.assert_called_once()