From cf6c72fdbd4038fdfd9f5598134456c2c256da8d Mon Sep 17 00:00:00 2001 From: sdb9696 <51370195+sdb9696@users.noreply.github.com> Date: Tue, 14 Nov 2023 07:15:19 +0000 Subject: [PATCH] Bump ring_doorbell to 0.8.0 and handle new exceptions (#103904) * Bump ring_doorbell to 0.8.0 and handle the new exceptions * Modify data update tests to not call coordinator internals --- homeassistant/components/ring/__init__.py | 30 ++-- homeassistant/components/ring/config_flow.py | 9 +- homeassistant/components/ring/manifest.json | 2 +- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/ring/conftest.py | 71 +++++++- tests/components/ring/test_config_flow.py | 107 ++++++++---- tests/components/ring/test_init.py | 164 ++++++++++++++++++- 8 files changed, 333 insertions(+), 54 deletions(-) diff --git a/homeassistant/components/ring/__init__.py b/homeassistant/components/ring/__init__.py index 56aad1a845b..a0863836a6c 100644 --- a/homeassistant/components/ring/__init__.py +++ b/homeassistant/components/ring/__init__.py @@ -8,9 +8,7 @@ from functools import partial import logging from typing import Any -from oauthlib.oauth2 import AccessDeniedError -import requests -from ring_doorbell import Auth, Ring +import ring_doorbell from homeassistant.config_entries import ConfigEntry from homeassistant.const import Platform, __version__ @@ -53,12 +51,14 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: ), ).result() - auth = Auth(f"HomeAssistant/{__version__}", entry.data["token"], token_updater) - ring = Ring(auth) + auth = ring_doorbell.Auth( + f"HomeAssistant/{__version__}", entry.data["token"], token_updater + ) + ring = ring_doorbell.Ring(auth) try: await hass.async_add_executor_job(ring.update_data) - except AccessDeniedError: + except ring_doorbell.AuthenticationError: _LOGGER.error("Access token is no longer valid. Please set up Ring again") return False @@ -144,7 +144,7 @@ class GlobalDataUpdater: hass: HomeAssistant, data_type: str, config_entry_id: str, - ring: Ring, + ring: ring_doorbell.Ring, update_method: str, update_interval: timedelta, ) -> None: @@ -187,17 +187,17 @@ class GlobalDataUpdater: await self.hass.async_add_executor_job( getattr(self.ring, self.update_method) ) - except AccessDeniedError: + except ring_doorbell.AuthenticationError: _LOGGER.error("Ring access token is no longer valid. Set up Ring again") await self.hass.config_entries.async_unload(self.config_entry_id) return - except requests.Timeout: + except ring_doorbell.RingTimeout: _LOGGER.warning( "Time out fetching Ring %s data", self.data_type, ) return - except requests.RequestException as err: + except ring_doorbell.RingError as err: _LOGGER.warning( "Error fetching Ring %s data: %s", self.data_type, @@ -217,8 +217,8 @@ class DeviceDataUpdater: hass: HomeAssistant, data_type: str, config_entry_id: str, - ring: Ring, - update_method: Callable[[Ring], Any], + ring: ring_doorbell.Ring, + update_method: Callable[[ring_doorbell.Ring], Any], update_interval: timedelta, ) -> None: """Initialize device data updater.""" @@ -276,20 +276,20 @@ class DeviceDataUpdater: for device_id, info in self.devices.items(): try: data = info["data"] = self.update_method(info["device"]) - except AccessDeniedError: + except ring_doorbell.AuthenticationError: _LOGGER.error("Ring access token is no longer valid. Set up Ring again") self.hass.add_job( self.hass.config_entries.async_unload(self.config_entry_id) ) return - except requests.Timeout: + except ring_doorbell.RingTimeout: _LOGGER.warning( "Time out fetching Ring %s data for device %s", self.data_type, device_id, ) continue - except requests.RequestException as err: + except ring_doorbell.RingError as err: _LOGGER.warning( "Error fetching Ring %s data for device %s: %s", self.data_type, diff --git a/homeassistant/components/ring/config_flow.py b/homeassistant/components/ring/config_flow.py index 9425b2f98a4..b22d59a78f5 100644 --- a/homeassistant/components/ring/config_flow.py +++ b/homeassistant/components/ring/config_flow.py @@ -2,8 +2,7 @@ import logging from typing import Any -from oauthlib.oauth2 import AccessDeniedError, MissingTokenError -from ring_doorbell import Auth +import ring_doorbell import voluptuous as vol from homeassistant import config_entries, core, exceptions @@ -17,7 +16,7 @@ _LOGGER = logging.getLogger(__name__) async def validate_input(hass: core.HomeAssistant, data): """Validate the user input allows us to connect.""" - auth = Auth(f"HomeAssistant/{ha_version}") + auth = ring_doorbell.Auth(f"HomeAssistant/{ha_version}") try: token = await hass.async_add_executor_job( @@ -26,9 +25,9 @@ async def validate_input(hass: core.HomeAssistant, data): data["password"], data.get("2fa"), ) - except MissingTokenError as err: + except ring_doorbell.Requires2FAError as err: raise Require2FA from err - except AccessDeniedError as err: + except ring_doorbell.AuthenticationError as err: raise InvalidAuth from err return token diff --git a/homeassistant/components/ring/manifest.json b/homeassistant/components/ring/manifest.json index 9cea738eb3a..8abf73e7fed 100644 --- a/homeassistant/components/ring/manifest.json +++ b/homeassistant/components/ring/manifest.json @@ -13,5 +13,5 @@ "documentation": "https://www.home-assistant.io/integrations/ring", "iot_class": "cloud_polling", "loggers": ["ring_doorbell"], - "requirements": ["ring-doorbell==0.7.3"] + "requirements": ["ring-doorbell==0.8.0"] } diff --git a/requirements_all.txt b/requirements_all.txt index 39f24f68c09..25f5e600a1a 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -2345,7 +2345,7 @@ rfk101py==0.0.1 rflink==0.0.65 # homeassistant.components.ring -ring-doorbell==0.7.3 +ring-doorbell==0.8.0 # homeassistant.components.fleetgo ritassist==0.9.2 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index d4c496cb645..87bb0f1adc9 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -1748,7 +1748,7 @@ reolink-aio==0.7.15 rflink==0.0.65 # homeassistant.components.ring -ring-doorbell==0.7.3 +ring-doorbell==0.8.0 # homeassistant.components.roku rokuecp==0.18.1 diff --git a/tests/components/ring/conftest.py b/tests/components/ring/conftest.py index 2b6edf86132..e9800393835 100644 --- a/tests/components/ring/conftest.py +++ b/tests/components/ring/conftest.py @@ -1,13 +1,74 @@ """Configuration for Ring tests.""" +from collections.abc import Generator import re +from unittest.mock import AsyncMock, Mock, patch import pytest import requests_mock -from tests.common import load_fixture +from homeassistant.components.ring import DOMAIN +from homeassistant.const import CONF_USERNAME +from homeassistant.core import HomeAssistant + +from tests.common import MockConfigEntry, load_fixture from tests.components.light.conftest import mock_light_profiles # noqa: F401 +@pytest.fixture +def mock_setup_entry() -> Generator[AsyncMock, None, None]: + """Override async_setup_entry.""" + with patch( + "homeassistant.components.ring.async_setup_entry", return_value=True + ) as mock_setup_entry: + yield mock_setup_entry + + +@pytest.fixture +def mock_ring_auth(): + """Mock ring_doorbell.Auth.""" + with patch("ring_doorbell.Auth", autospec=True) as mock_ring_auth: + mock_ring_auth.return_value.fetch_token.return_value = { + "access_token": "mock-token" + } + yield mock_ring_auth.return_value + + +@pytest.fixture +def mock_ring(): + """Mock ring_doorbell.Ring.""" + with patch("ring_doorbell.Ring", autospec=True) as mock_ring: + yield mock_ring.return_value + + +@pytest.fixture +def mock_config_entry() -> MockConfigEntry: + """Mock ConfigEntry.""" + return MockConfigEntry( + title="Ring", + domain=DOMAIN, + data={ + CONF_USERNAME: "foo@bar.com", + "token": {"access_token": "mock-token"}, + }, + unique_id="foo@bar.com", + ) + + +@pytest.fixture +async def mock_added_config_entry( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_ring_auth: Mock, + mock_ring: Mock, +) -> MockConfigEntry: + """Mock ConfigEntry that's been added to HA.""" + mock_config_entry.add_to_hass(hass) + await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + assert DOMAIN in hass.config_entries.async_domains() + return mock_config_entry + + @pytest.fixture(name="requests_mock") def requests_mock_fixture(): """Fixture to provide a requests mocker.""" @@ -52,5 +113,11 @@ def requests_mock_fixture(): re.compile(r"https:\/\/api\.ring\.com\/clients_api\/chimes\/\d+\/health"), text=load_fixture("chime_health_attrs.json", "ring"), ) - + mock.get( + re.compile( + r"https:\/\/api\.ring\.com\/clients_api\/dings\/\d+\/share/play" + ), + status_code=200, + json={"url": "http://127.0.0.1/foo"}, + ) yield mock diff --git a/tests/components/ring/test_config_flow.py b/tests/components/ring/test_config_flow.py index 3e0c354e8fa..0c1578e2c8d 100644 --- a/tests/components/ring/test_config_flow.py +++ b/tests/components/ring/test_config_flow.py @@ -1,13 +1,21 @@ """Test the Ring config flow.""" -from unittest.mock import Mock, patch +from unittest.mock import AsyncMock, Mock + +import pytest +import ring_doorbell from homeassistant import config_entries from homeassistant.components.ring import DOMAIN -from homeassistant.components.ring.config_flow import InvalidAuth +from homeassistant.const import CONF_PASSWORD, CONF_USERNAME from homeassistant.core import HomeAssistant +from homeassistant.data_entry_flow import FlowResultType -async def test_form(hass: HomeAssistant) -> None: +async def test_form( + hass: HomeAssistant, + mock_setup_entry: AsyncMock, + mock_ring_auth: Mock, +) -> None: """Test we get the form.""" result = await hass.config_entries.flow.async_init( @@ -16,20 +24,11 @@ async def test_form(hass: HomeAssistant) -> None: assert result["type"] == "form" assert result["errors"] == {} - with patch( - "homeassistant.components.ring.config_flow.Auth", - return_value=Mock( - fetch_token=Mock(return_value={"access_token": "mock-token"}) - ), - ), patch( - "homeassistant.components.ring.async_setup_entry", - return_value=True, - ) as mock_setup_entry: - result2 = await hass.config_entries.flow.async_configure( - result["flow_id"], - {"username": "hello@home-assistant.io", "password": "test-password"}, - ) - await hass.async_block_till_done() + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], + {"username": "hello@home-assistant.io", "password": "test-password"}, + ) + await hass.async_block_till_done() assert result2["type"] == "create_entry" assert result2["title"] == "hello@home-assistant.io" @@ -40,20 +39,72 @@ async def test_form(hass: HomeAssistant) -> None: assert len(mock_setup_entry.mock_calls) == 1 -async def test_form_invalid_auth(hass: HomeAssistant) -> None: +@pytest.mark.parametrize( + ("error_type", "errors_msg"), + [ + (ring_doorbell.AuthenticationError, "invalid_auth"), + (Exception, "unknown"), + ], + ids=["invalid-auth", "unknown-error"], +) +async def test_form_error( + hass: HomeAssistant, mock_ring_auth: Mock, error_type, errors_msg +) -> None: """Test we handle invalid auth.""" result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_USER} ) - - with patch( - "homeassistant.components.ring.config_flow.Auth.fetch_token", - side_effect=InvalidAuth, - ): - result2 = await hass.config_entries.flow.async_configure( - result["flow_id"], - {"username": "hello@home-assistant.io", "password": "test-password"}, - ) + mock_ring_auth.fetch_token.side_effect = error_type + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], + {"username": "hello@home-assistant.io", "password": "test-password"}, + ) assert result2["type"] == "form" - assert result2["errors"] == {"base": "invalid_auth"} + assert result2["errors"] == {"base": errors_msg} + + +async def test_form_2fa( + hass: HomeAssistant, + mock_setup_entry: AsyncMock, + mock_ring_auth: Mock, +) -> None: + """Test form flow for 2fa.""" + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + assert result["type"] == FlowResultType.FORM + assert result["errors"] == {} + + mock_ring_auth.fetch_token.side_effect = ring_doorbell.Requires2FAError + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], + { + CONF_USERNAME: "foo@bar.com", + CONF_PASSWORD: "fake-password", + }, + ) + await hass.async_block_till_done() + mock_ring_auth.fetch_token.assert_called_once_with( + "foo@bar.com", "fake-password", None + ) + + assert result2["type"] == FlowResultType.FORM + assert result2["step_id"] == "2fa" + mock_ring_auth.fetch_token.reset_mock(side_effect=True) + mock_ring_auth.fetch_token.return_value = "new-foobar" + result3 = await hass.config_entries.flow.async_configure( + result2["flow_id"], + user_input={"2fa": "123456"}, + ) + + mock_ring_auth.fetch_token.assert_called_once_with( + "foo@bar.com", "fake-password", "123456" + ) + assert result3["type"] == FlowResultType.CREATE_ENTRY + assert result3["title"] == "foo@bar.com" + assert result3["data"] == { + "username": "foo@bar.com", + "token": "new-foobar", + } + assert len(mock_setup_entry.mock_calls) == 1 diff --git a/tests/components/ring/test_init.py b/tests/components/ring/test_init.py index 7e3f5344f73..9fa79b21fab 100644 --- a/tests/components/ring/test_init.py +++ b/tests/components/ring/test_init.py @@ -1,12 +1,20 @@ """The tests for the Ring component.""" +from datetime import timedelta +from unittest.mock import patch + +import pytest import requests_mock +from ring_doorbell import AuthenticationError, RingError, RingTimeout import homeassistant.components.ring as ring +from homeassistant.components.ring import DOMAIN +from homeassistant.config_entries import ConfigEntryState from homeassistant.core import HomeAssistant from homeassistant.setup import async_setup_component +from homeassistant.util import dt as dt_util -from tests.common import load_fixture +from tests.common import MockConfigEntry, async_fire_time_changed, load_fixture async def test_setup(hass: HomeAssistant, requests_mock: requests_mock.Mocker) -> None: @@ -32,3 +40,157 @@ async def test_setup(hass: HomeAssistant, requests_mock: requests_mock.Mocker) - "https://api.ring.com/clients_api/doorbots/987652/health", text=load_fixture("doorboot_health_attrs.json", "ring"), ) + + +async def test_auth_failed_on_setup( + hass: HomeAssistant, + requests_mock: requests_mock.Mocker, + mock_config_entry: MockConfigEntry, + caplog, +) -> None: + """Test auth failure on setup entry.""" + mock_config_entry.add_to_hass(hass) + with patch( + "ring_doorbell.Ring.update_data", + side_effect=AuthenticationError, + ): + result = await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + assert result is False + assert "Access token is no longer valid. Please set up Ring again" in [ + record.message for record in caplog.records if record.levelname == "ERROR" + ] + + assert DOMAIN not in hass.data + assert mock_config_entry.state is ConfigEntryState.SETUP_ERROR + + +async def test_auth_failure_on_global_update( + hass: HomeAssistant, + requests_mock: requests_mock.Mocker, + mock_config_entry: MockConfigEntry, + caplog, +) -> None: + """Test authentication failure on global data update.""" + mock_config_entry.add_to_hass(hass) + await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + + with patch( + "ring_doorbell.Ring.update_devices", + side_effect=AuthenticationError, + ): + async_fire_time_changed(hass, dt_util.now() + timedelta(minutes=20)) + await hass.async_block_till_done() + + assert "Ring access token is no longer valid. Set up Ring again" in [ + record.message for record in caplog.records if record.levelname == "ERROR" + ] + + assert mock_config_entry.entry_id not in hass.data[DOMAIN] + + +async def test_auth_failure_on_device_update( + hass: HomeAssistant, + requests_mock: requests_mock.Mocker, + mock_config_entry: MockConfigEntry, + caplog, +) -> None: + """Test authentication failure on global data update.""" + mock_config_entry.add_to_hass(hass) + await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + + with patch( + "ring_doorbell.RingDoorBell.history", + side_effect=AuthenticationError, + ): + async_fire_time_changed(hass, dt_util.now() + timedelta(minutes=20)) + await hass.async_block_till_done() + + assert "Ring access token is no longer valid. Set up Ring again" in [ + record.message for record in caplog.records if record.levelname == "ERROR" + ] + + assert mock_config_entry.entry_id not in hass.data[DOMAIN] + + +@pytest.mark.parametrize( + ("error_type", "log_msg"), + [ + ( + RingTimeout, + "Time out fetching Ring device data", + ), + ( + RingError, + "Error fetching Ring device data: ", + ), + ], + ids=["timeout-error", "other-error"], +) +async def test_error_on_global_update( + hass: HomeAssistant, + requests_mock: requests_mock.Mocker, + mock_config_entry: MockConfigEntry, + caplog, + error_type, + log_msg, +) -> None: + """Test error on global data update.""" + mock_config_entry.add_to_hass(hass) + await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + + with patch( + "ring_doorbell.Ring.update_devices", + side_effect=error_type, + ): + async_fire_time_changed(hass, dt_util.now() + timedelta(minutes=20)) + await hass.async_block_till_done() + + assert log_msg in [ + record.message for record in caplog.records if record.levelname == "WARNING" + ] + + assert mock_config_entry.entry_id in hass.data[DOMAIN] + + +@pytest.mark.parametrize( + ("error_type", "log_msg"), + [ + ( + RingTimeout, + "Time out fetching Ring history data for device aacdef123", + ), + ( + RingError, + "Error fetching Ring history data for device aacdef123: ", + ), + ], + ids=["timeout-error", "other-error"], +) +async def test_error_on_device_update( + hass: HomeAssistant, + requests_mock: requests_mock.Mocker, + mock_config_entry: MockConfigEntry, + caplog, + error_type, + log_msg, +) -> None: + """Test auth failure on data update.""" + mock_config_entry.add_to_hass(hass) + await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + + with patch( + "ring_doorbell.RingDoorBell.history", + side_effect=error_type, + ): + async_fire_time_changed(hass, dt_util.now() + timedelta(minutes=20)) + await hass.async_block_till_done() + + assert log_msg in [ + record.message for record in caplog.records if record.levelname == "WARNING" + ] + assert mock_config_entry.entry_id in hass.data[DOMAIN]