From 54591b8ca1b928c883beb2aa080361e236d6d704 Mon Sep 17 00:00:00 2001 From: rikroe <42204099+rikroe@users.noreply.github.com> Date: Wed, 22 Jun 2022 14:24:16 +0200 Subject: [PATCH] BMW Connected Drive: Handle HTTP 429 issues better (#73675) Co-authored-by: rikroe --- .../bmw_connected_drive/config_flow.py | 21 +++++++++++----- .../bmw_connected_drive/coordinator.py | 22 +++++++++++++---- .../bmw_connected_drive/test_config_flow.py | 24 +++++++++++++++---- 3 files changed, 52 insertions(+), 15 deletions(-) diff --git a/homeassistant/components/bmw_connected_drive/config_flow.py b/homeassistant/components/bmw_connected_drive/config_flow.py index c07be4c8849..3994b0732a8 100644 --- a/homeassistant/components/bmw_connected_drive/config_flow.py +++ b/homeassistant/components/bmw_connected_drive/config_flow.py @@ -3,7 +3,7 @@ from __future__ import annotations from typing import Any -from bimmer_connected.account import MyBMWAccount +from bimmer_connected.api.authentication import MyBMWAuthentication from bimmer_connected.api.regions import get_region_from_name from httpx import HTTPError import voluptuous as vol @@ -14,7 +14,7 @@ from homeassistant.core import callback from homeassistant.data_entry_flow import FlowResult from . import DOMAIN -from .const import CONF_ALLOWED_REGIONS, CONF_READ_ONLY +from .const import CONF_ALLOWED_REGIONS, CONF_READ_ONLY, CONF_REFRESH_TOKEN DATA_SCHEMA = vol.Schema( { @@ -32,19 +32,22 @@ async def validate_input( Data has the keys from DATA_SCHEMA with values provided by the user. """ - account = MyBMWAccount( + auth = MyBMWAuthentication( data[CONF_USERNAME], data[CONF_PASSWORD], get_region_from_name(data[CONF_REGION]), ) try: - await account.get_vehicles() + await auth.login() except HTTPError as ex: raise CannotConnect from ex # Return info that you want to store in the config entry. - return {"title": f"{data[CONF_USERNAME]}{data.get(CONF_SOURCE, '')}"} + retval = {"title": f"{data[CONF_USERNAME]}{data.get(CONF_SOURCE, '')}"} + if auth.refresh_token: + retval[CONF_REFRESH_TOKEN] = auth.refresh_token + return retval class BMWConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): @@ -70,7 +73,13 @@ class BMWConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): errors["base"] = "cannot_connect" if info: - return self.async_create_entry(title=info["title"], data=user_input) + return self.async_create_entry( + title=info["title"], + data={ + **user_input, + CONF_REFRESH_TOKEN: info.get(CONF_REFRESH_TOKEN), + }, + ) return self.async_show_form( step_id="user", data_schema=DATA_SCHEMA, errors=errors diff --git a/homeassistant/components/bmw_connected_drive/coordinator.py b/homeassistant/components/bmw_connected_drive/coordinator.py index 1443a3e1e29..e5a968b47fd 100644 --- a/homeassistant/components/bmw_connected_drive/coordinator.py +++ b/homeassistant/components/bmw_connected_drive/coordinator.py @@ -7,7 +7,7 @@ import logging from bimmer_connected.account import MyBMWAccount from bimmer_connected.api.regions import get_region_from_name from bimmer_connected.models import GPSPosition -from httpx import HTTPError, TimeoutException +from httpx import HTTPError, HTTPStatusError, TimeoutException from homeassistant.config_entries import ConfigEntry from homeassistant.const import CONF_PASSWORD, CONF_REGION, CONF_USERNAME @@ -16,7 +16,8 @@ from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, Upda from .const import CONF_READ_ONLY, CONF_REFRESH_TOKEN, DOMAIN -SCAN_INTERVAL = timedelta(seconds=300) +DEFAULT_SCAN_INTERVAL_SECONDS = 300 +SCAN_INTERVAL = timedelta(seconds=DEFAULT_SCAN_INTERVAL_SECONDS) _LOGGER = logging.getLogger(__name__) @@ -53,8 +54,18 @@ class BMWDataUpdateCoordinator(DataUpdateCoordinator): try: await self.account.get_vehicles() - except (HTTPError, TimeoutException) as err: - self._update_config_entry_refresh_token(None) + except (HTTPError, HTTPStatusError, TimeoutException) as err: + if isinstance(err, HTTPStatusError) and err.response.status_code == 429: + # Increase scan interval to not jump to not bring up the issue next time + self.update_interval = timedelta( + seconds=DEFAULT_SCAN_INTERVAL_SECONDS * 3 + ) + if isinstance(err, HTTPStatusError) and err.response.status_code in ( + 401, + 403, + ): + # Clear refresh token only on issues with authorization + self._update_config_entry_refresh_token(None) raise UpdateFailed(f"Error communicating with BMW API: {err}") from err if self.account.refresh_token != old_refresh_token: @@ -65,6 +76,9 @@ class BMWDataUpdateCoordinator(DataUpdateCoordinator): self.account.refresh_token, ) + # Reset scan interval after successful update + self.update_interval = timedelta(seconds=DEFAULT_SCAN_INTERVAL_SECONDS) + def _update_config_entry_refresh_token(self, refresh_token: str | None) -> None: """Update or delete the refresh_token in the Config Entry.""" data = { diff --git a/tests/components/bmw_connected_drive/test_config_flow.py b/tests/components/bmw_connected_drive/test_config_flow.py index d3c7c64dc99..10178c22de8 100644 --- a/tests/components/bmw_connected_drive/test_config_flow.py +++ b/tests/components/bmw_connected_drive/test_config_flow.py @@ -1,19 +1,32 @@ """Test the for the BMW Connected Drive config flow.""" from unittest.mock import patch +from bimmer_connected.api.authentication import MyBMWAuthentication from httpx import HTTPError from homeassistant import config_entries, data_entry_flow from homeassistant.components.bmw_connected_drive.config_flow import DOMAIN -from homeassistant.components.bmw_connected_drive.const import CONF_READ_ONLY +from homeassistant.components.bmw_connected_drive.const import ( + CONF_READ_ONLY, + CONF_REFRESH_TOKEN, +) from homeassistant.const import CONF_USERNAME from . import FIXTURE_CONFIG_ENTRY, FIXTURE_USER_INPUT from tests.common import MockConfigEntry -FIXTURE_COMPLETE_ENTRY = FIXTURE_USER_INPUT.copy() -FIXTURE_IMPORT_ENTRY = FIXTURE_USER_INPUT.copy() +FIXTURE_REFRESH_TOKEN = "SOME_REFRESH_TOKEN" +FIXTURE_COMPLETE_ENTRY = { + **FIXTURE_USER_INPUT, + CONF_REFRESH_TOKEN: FIXTURE_REFRESH_TOKEN, +} +FIXTURE_IMPORT_ENTRY = {**FIXTURE_USER_INPUT, CONF_REFRESH_TOKEN: None} + + +def login_sideeffect(self: MyBMWAuthentication): + """Mock logging in and setting a refresh token.""" + self.refresh_token = FIXTURE_REFRESH_TOKEN async def test_show_form(hass): @@ -50,8 +63,9 @@ async def test_connection_error(hass): async def test_full_user_flow_implementation(hass): """Test registering an integration and finishing flow works.""" with patch( - "bimmer_connected.account.MyBMWAccount.get_vehicles", - return_value=[], + "bimmer_connected.api.authentication.MyBMWAuthentication.login", + side_effect=login_sideeffect, + autospec=True, ), patch( "homeassistant.components.bmw_connected_drive.async_setup_entry", return_value=True,