From f70a2ba1f71fa7dd13ee3216abcb5cc4ff2458cb Mon Sep 17 00:00:00 2001 From: jjlawren Date: Thu, 9 Apr 2020 17:49:09 -0500 Subject: [PATCH] Improve Plex debounce/throttle logic (#33805) * Improve Plex debounce/throttle logic * Use Debouncer helper, rewrite affected tests * Mock storage so files aren't left behind * Don't bother with wrapper method, store debouncer call during init * Test cleanup from review * Don't patch own code in tests --- homeassistant/components/plex/server.py | 38 ++--- tests/components/plex/common.py | 20 --- tests/components/plex/test_config_flow.py | 6 +- tests/components/plex/test_init.py | 155 +++++++++++--------- tests/components/plex/test_server.py | 166 +++++++++++++--------- 5 files changed, 200 insertions(+), 185 deletions(-) delete mode 100644 tests/components/plex/common.py diff --git a/homeassistant/components/plex/server.py b/homeassistant/components/plex/server.py index 4134ad4e32b..d9e2d2bd9cc 100644 --- a/homeassistant/components/plex/server.py +++ b/homeassistant/components/plex/server.py @@ -1,5 +1,4 @@ """Shared class to maintain Plex server instances.""" -from functools import partial, wraps import logging import ssl from urllib.parse import urlparse @@ -13,8 +12,8 @@ import requests.exceptions from homeassistant.components.media_player import DOMAIN as MP_DOMAIN from homeassistant.const import CONF_TOKEN, CONF_URL, CONF_VERIFY_SSL from homeassistant.core import callback +from homeassistant.helpers.debounce import Debouncer from homeassistant.helpers.dispatcher import async_dispatcher_send -from homeassistant.helpers.event import async_call_later from .const import ( CONF_CLIENT_IDENTIFIER, @@ -43,31 +42,6 @@ plexapi.X_PLEX_PRODUCT = X_PLEX_PRODUCT plexapi.X_PLEX_VERSION = X_PLEX_VERSION -def debounce(func): - """Decorate function to debounce callbacks from Plex websocket.""" - - unsub = None - - async def call_later_listener(self, _): - """Handle call_later callback.""" - nonlocal unsub - unsub = None - await func(self) - - @wraps(func) - async def wrapper(self): - """Schedule async callback.""" - nonlocal unsub - if unsub: - _LOGGER.debug("Throttling update of %s", self.friendly_name) - unsub() # pylint: disable=not-callable - unsub = async_call_later( - self.hass, DEBOUNCE_TIMEOUT, partial(call_later_listener, self), - ) - - return wrapper - - class PlexServer: """Manages a single Plex server connection.""" @@ -87,6 +61,13 @@ class PlexServer: self._accounts = [] self._owner_username = None self._version = None + self.async_update_platforms = Debouncer( + hass, + _LOGGER, + cooldown=DEBOUNCE_TIMEOUT, + immediate=True, + function=self._async_update_platforms, + ).async_call # Header conditionally added as it is not available in config entry v1 if CONF_CLIENT_IDENTIFIER in server_config: @@ -192,8 +173,7 @@ class PlexServer: """Fetch all data from the Plex server in a single method.""" return (self._plex_server.clients(), self._plex_server.sessions()) - @debounce - async def async_update_platforms(self): + async def _async_update_platforms(self): """Update the platform entities.""" _LOGGER.debug("Updating devices") diff --git a/tests/components/plex/common.py b/tests/components/plex/common.py deleted file mode 100644 index adc6f4e0299..00000000000 --- a/tests/components/plex/common.py +++ /dev/null @@ -1,20 +0,0 @@ -"""Common fixtures and functions for Plex tests.""" -from datetime import timedelta - -from homeassistant.components.plex.const import ( - DEBOUNCE_TIMEOUT, - PLEX_UPDATE_PLATFORMS_SIGNAL, -) -from homeassistant.helpers.dispatcher import async_dispatcher_send -import homeassistant.util.dt as dt_util - -from tests.common import async_fire_time_changed - - -async def trigger_plex_update(hass, server_id): - """Update Plex by sending signal and jumping ahead by debounce timeout.""" - async_dispatcher_send(hass, PLEX_UPDATE_PLATFORMS_SIGNAL.format(server_id)) - await hass.async_block_till_done() - next_update = dt_util.utcnow() + timedelta(seconds=DEBOUNCE_TIMEOUT) - async_fire_time_changed(hass, next_update) - await hass.async_block_till_done() diff --git a/tests/components/plex/test_config_flow.py b/tests/components/plex/test_config_flow.py index bd5d45c0246..d839ccc674b 100644 --- a/tests/components/plex/test_config_flow.py +++ b/tests/components/plex/test_config_flow.py @@ -15,13 +15,14 @@ from homeassistant.components.plex.const import ( CONF_USE_EPISODE_ART, DOMAIN, PLEX_SERVER_CONFIG, + PLEX_UPDATE_PLATFORMS_SIGNAL, SERVERS, ) from homeassistant.config_entries import ENTRY_STATE_LOADED from homeassistant.const import CONF_HOST, CONF_PORT, CONF_TOKEN, CONF_URL +from homeassistant.helpers.dispatcher import async_dispatcher_send from homeassistant.setup import async_setup_component -from .common import trigger_plex_update from .const import DEFAULT_DATA, DEFAULT_OPTIONS, MOCK_SERVERS, MOCK_TOKEN from .mock_classes import MockPlexAccount, MockPlexServer @@ -415,7 +416,8 @@ async def test_option_flow_new_users_available(hass, caplog): server_id = mock_plex_server.machineIdentifier - await trigger_plex_update(hass, server_id) + async_dispatcher_send(hass, PLEX_UPDATE_PLATFORMS_SIGNAL.format(server_id)) + await hass.async_block_till_done() monitored_users = hass.data[DOMAIN][SERVERS][server_id].option_monitored_users diff --git a/tests/components/plex/test_init.py b/tests/components/plex/test_init.py index cd1ea8725bd..ef2199b11c5 100644 --- a/tests/components/plex/test_init.py +++ b/tests/components/plex/test_init.py @@ -3,8 +3,9 @@ import copy from datetime import timedelta import ssl -from asynctest import patch +from asynctest import ClockedTestCase, patch import plexapi +import pytest import requests from homeassistant.components.media_player import DOMAIN as MP_DOMAIN @@ -23,14 +24,19 @@ from homeassistant.const import ( CONF_URL, CONF_VERIFY_SSL, ) +from homeassistant.helpers.dispatcher import async_dispatcher_send from homeassistant.setup import async_setup_component import homeassistant.util.dt as dt_util -from .common import trigger_plex_update from .const import DEFAULT_DATA, DEFAULT_OPTIONS, MOCK_SERVERS, MOCK_TOKEN from .mock_classes import MockPlexAccount, MockPlexServer -from tests.common import MockConfigEntry, async_fire_time_changed +from tests.common import ( + MockConfigEntry, + async_fire_time_changed, + async_test_home_assistant, + mock_storage, +) async def test_setup_with_config(hass): @@ -67,70 +73,90 @@ async def test_setup_with_config(hass): assert loaded_server.plex_server == mock_plex_server - assert server_id in hass.data[const.DOMAIN][const.DISPATCHERS] - assert server_id in hass.data[const.DOMAIN][const.WEBSOCKETS] - assert ( - hass.data[const.DOMAIN][const.PLATFORMS_COMPLETED][server_id] == const.PLATFORMS - ) +class TestClockedPlex(ClockedTestCase): + """Create clock-controlled asynctest class.""" -async def test_setup_with_config_entry(hass, caplog): - """Test setup component with config.""" + @pytest.fixture(autouse=True) + def inject_fixture(self, caplog): + """Inject pytest fixtures as instance attributes.""" + self.caplog = caplog - mock_plex_server = MockPlexServer() + async def setUp(self): + """Initialize this test class.""" + self.hass = await async_test_home_assistant(self.loop) + self.mock_storage = mock_storage() + self.mock_storage.__enter__() - entry = MockConfigEntry( - domain=const.DOMAIN, - data=DEFAULT_DATA, - options=DEFAULT_OPTIONS, - unique_id=DEFAULT_DATA["server_id"], - ) + async def tearDown(self): + """Clean up the HomeAssistant instance.""" + await self.hass.async_stop() + self.mock_storage.__exit__(None, None, None) - with patch("plexapi.server.PlexServer", return_value=mock_plex_server), patch( - "homeassistant.components.plex.PlexWebsocket.listen" - ) as mock_listen: - entry.add_to_hass(hass) - assert await hass.config_entries.async_setup(entry.entry_id) + async def test_setup_with_config_entry(self): + """Test setup component with config.""" + hass = self.hass + + mock_plex_server = MockPlexServer() + + entry = MockConfigEntry( + domain=const.DOMAIN, + data=DEFAULT_DATA, + options=DEFAULT_OPTIONS, + unique_id=DEFAULT_DATA["server_id"], + ) + + with patch("plexapi.server.PlexServer", return_value=mock_plex_server), patch( + "homeassistant.components.plex.PlexWebsocket.listen" + ) as mock_listen: + entry.add_to_hass(hass) + assert await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + assert mock_listen.called + + assert len(hass.config_entries.async_entries(const.DOMAIN)) == 1 + assert entry.state == ENTRY_STATE_LOADED + + server_id = mock_plex_server.machineIdentifier + loaded_server = hass.data[const.DOMAIN][const.SERVERS][server_id] + + assert loaded_server.plex_server == mock_plex_server + + async_dispatcher_send( + hass, const.PLEX_UPDATE_PLATFORMS_SIGNAL.format(server_id) + ) await hass.async_block_till_done() - assert mock_listen.called + sensor = hass.states.get("sensor.plex_plex_server_1") + assert sensor.state == str(len(mock_plex_server.accounts)) - assert len(hass.config_entries.async_entries(const.DOMAIN)) == 1 - assert entry.state == ENTRY_STATE_LOADED - - server_id = mock_plex_server.machineIdentifier - loaded_server = hass.data[const.DOMAIN][const.SERVERS][server_id] - - assert loaded_server.plex_server == mock_plex_server - - assert server_id in hass.data[const.DOMAIN][const.DISPATCHERS] - assert server_id in hass.data[const.DOMAIN][const.WEBSOCKETS] - assert ( - hass.data[const.DOMAIN][const.PLATFORMS_COMPLETED][server_id] == const.PLATFORMS - ) - - await trigger_plex_update(hass, server_id) - - sensor = hass.states.get("sensor.plex_plex_server_1") - assert sensor.state == str(len(mock_plex_server.accounts)) - - await trigger_plex_update(hass, server_id) - - for test_exception in ( - plexapi.exceptions.BadRequest, - requests.exceptions.RequestException, - ): - with patch.object( - mock_plex_server, "clients", side_effect=test_exception - ) as patched_clients_bad_request: - await trigger_plex_update(hass, server_id) - - assert patched_clients_bad_request.called - assert ( - f"Could not connect to Plex server: {mock_plex_server.friendlyName}" - in caplog.text + # Ensure existing entities refresh + await self.advance(const.DEBOUNCE_TIMEOUT) + async_dispatcher_send( + hass, const.PLEX_UPDATE_PLATFORMS_SIGNAL.format(server_id) ) - caplog.clear() + await hass.async_block_till_done() + + for test_exception in ( + plexapi.exceptions.BadRequest, + requests.exceptions.RequestException, + ): + with patch.object( + mock_plex_server, "clients", side_effect=test_exception + ) as patched_clients_bad_request: + await self.advance(const.DEBOUNCE_TIMEOUT) + async_dispatcher_send( + hass, const.PLEX_UPDATE_PLATFORMS_SIGNAL.format(server_id) + ) + await hass.async_block_till_done() + + assert patched_clients_bad_request.called + assert ( + f"Could not connect to Plex server: {mock_plex_server.friendlyName}" + in self.caplog.text + ) + self.caplog.clear() async def test_set_config_entry_unique_id(hass): @@ -251,22 +277,12 @@ async def test_unload_config_entry(hass): assert loaded_server.plex_server == mock_plex_server - assert server_id in hass.data[const.DOMAIN][const.DISPATCHERS] - assert server_id in hass.data[const.DOMAIN][const.WEBSOCKETS] - assert ( - hass.data[const.DOMAIN][const.PLATFORMS_COMPLETED][server_id] == const.PLATFORMS - ) - with patch("homeassistant.components.plex.PlexWebsocket.close") as mock_close: await hass.config_entries.async_unload(entry.entry_id) assert mock_close.called assert entry.state == ENTRY_STATE_NOT_LOADED - assert server_id not in hass.data[const.DOMAIN][const.SERVERS] - assert server_id not in hass.data[const.DOMAIN][const.DISPATCHERS] - assert server_id not in hass.data[const.DOMAIN][const.WEBSOCKETS] - async def test_setup_with_photo_session(hass): """Test setup component with config.""" @@ -292,7 +308,8 @@ async def test_setup_with_photo_session(hass): server_id = mock_plex_server.machineIdentifier - await trigger_plex_update(hass, server_id) + async_dispatcher_send(hass, const.PLEX_UPDATE_PLATFORMS_SIGNAL.format(server_id)) + await hass.async_block_till_done() media_player = hass.states.get("media_player.plex_product_title") assert media_player.state == "idle" diff --git a/tests/components/plex/test_server.py b/tests/components/plex/test_server.py index 3b70f30189a..6eff97ae7dc 100644 --- a/tests/components/plex/test_server.py +++ b/tests/components/plex/test_server.py @@ -1,8 +1,7 @@ """Tests for Plex server.""" import copy -from datetime import timedelta -from asynctest import patch +from asynctest import ClockedTestCase, patch from homeassistant.components.media_player import DOMAIN as MP_DOMAIN from homeassistant.components.plex.const import ( @@ -14,13 +13,11 @@ from homeassistant.components.plex.const import ( SERVERS, ) from homeassistant.helpers.dispatcher import async_dispatcher_send -import homeassistant.util.dt as dt_util -from .common import trigger_plex_update from .const import DEFAULT_DATA, DEFAULT_OPTIONS from .mock_classes import MockPlexServer -from tests.common import MockConfigEntry, async_fire_time_changed +from tests.common import MockConfigEntry, async_test_home_assistant, mock_storage async def test_new_users_available(hass): @@ -48,7 +45,8 @@ async def test_new_users_available(hass): server_id = mock_plex_server.machineIdentifier - await trigger_plex_update(hass, server_id) + async_dispatcher_send(hass, PLEX_UPDATE_PLATFORMS_SIGNAL.format(server_id)) + await hass.async_block_till_done() monitored_users = hass.data[DOMAIN][SERVERS][server_id].option_monitored_users @@ -86,7 +84,8 @@ async def test_new_ignored_users_available(hass, caplog): server_id = mock_plex_server.machineIdentifier - await trigger_plex_update(hass, server_id) + async_dispatcher_send(hass, PLEX_UPDATE_PLATFORMS_SIGNAL.format(server_id)) + await hass.async_block_till_done() monitored_users = hass.data[DOMAIN][SERVERS][server_id].option_monitored_users @@ -100,72 +99,109 @@ async def test_new_ignored_users_available(hass, caplog): assert sensor.state == str(len(mock_plex_server.accounts)) -async def test_mark_sessions_idle(hass): - """Test marking media_players as idle when sessions end.""" - entry = MockConfigEntry( - domain=DOMAIN, - data=DEFAULT_DATA, - options=DEFAULT_OPTIONS, - unique_id=DEFAULT_DATA["server_id"], - ) +class TestClockedPlex(ClockedTestCase): + """Create clock-controlled asynctest class.""" - mock_plex_server = MockPlexServer(config_entry=entry) + async def setUp(self): + """Initialize this test class.""" + self.hass = await async_test_home_assistant(self.loop) + self.mock_storage = mock_storage() + self.mock_storage.__enter__() - with patch("plexapi.server.PlexServer", return_value=mock_plex_server), patch( - "homeassistant.components.plex.PlexWebsocket.listen" - ): - entry.add_to_hass(hass) - assert await hass.config_entries.async_setup(entry.entry_id) + async def tearDown(self): + """Clean up the HomeAssistant instance.""" + await self.hass.async_stop() + self.mock_storage.__exit__(None, None, None) + + async def test_mark_sessions_idle(self): + """Test marking media_players as idle when sessions end.""" + hass = self.hass + + entry = MockConfigEntry( + domain=DOMAIN, + data=DEFAULT_DATA, + options=DEFAULT_OPTIONS, + unique_id=DEFAULT_DATA["server_id"], + ) + + mock_plex_server = MockPlexServer(config_entry=entry) + + with patch("plexapi.server.PlexServer", return_value=mock_plex_server), patch( + "homeassistant.components.plex.PlexWebsocket.listen" + ): + entry.add_to_hass(hass) + assert await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + server_id = mock_plex_server.machineIdentifier + + async_dispatcher_send(hass, PLEX_UPDATE_PLATFORMS_SIGNAL.format(server_id)) await hass.async_block_till_done() - server_id = mock_plex_server.machineIdentifier + sensor = hass.states.get("sensor.plex_plex_server_1") + assert sensor.state == str(len(mock_plex_server.accounts)) - await trigger_plex_update(hass, server_id) + mock_plex_server.clear_clients() + mock_plex_server.clear_sessions() - sensor = hass.states.get("sensor.plex_plex_server_1") - assert sensor.state == str(len(mock_plex_server.accounts)) - - mock_plex_server.clear_clients() - mock_plex_server.clear_sessions() - - await trigger_plex_update(hass, server_id) - - sensor = hass.states.get("sensor.plex_plex_server_1") - assert sensor.state == "0" - - -async def test_debouncer(hass, caplog): - """Test debouncer decorator logic.""" - entry = MockConfigEntry( - domain=DOMAIN, - data=DEFAULT_DATA, - options=DEFAULT_OPTIONS, - unique_id=DEFAULT_DATA["server_id"], - ) - - mock_plex_server = MockPlexServer(config_entry=entry) - - with patch("plexapi.server.PlexServer", return_value=mock_plex_server), patch( - "homeassistant.components.plex.PlexWebsocket.listen" - ): - entry.add_to_hass(hass) - assert await hass.config_entries.async_setup(entry.entry_id) + await self.advance(DEBOUNCE_TIMEOUT) + async_dispatcher_send(hass, PLEX_UPDATE_PLATFORMS_SIGNAL.format(server_id)) await hass.async_block_till_done() - server_id = mock_plex_server.machineIdentifier + sensor = hass.states.get("sensor.plex_plex_server_1") + assert sensor.state == "0" - # First two updates are skipped - async_dispatcher_send(hass, PLEX_UPDATE_PLATFORMS_SIGNAL.format(server_id)) - await hass.async_block_till_done() - async_dispatcher_send(hass, PLEX_UPDATE_PLATFORMS_SIGNAL.format(server_id)) - await hass.async_block_till_done() - async_dispatcher_send(hass, PLEX_UPDATE_PLATFORMS_SIGNAL.format(server_id)) - await hass.async_block_till_done() + async def test_debouncer(self): + """Test debouncer behavior.""" + hass = self.hass - next_update = dt_util.utcnow() + timedelta(seconds=DEBOUNCE_TIMEOUT) - async_fire_time_changed(hass, next_update) - await hass.async_block_till_done() + entry = MockConfigEntry( + domain=DOMAIN, + data=DEFAULT_DATA, + options=DEFAULT_OPTIONS, + unique_id=DEFAULT_DATA["server_id"], + ) - assert ( - caplog.text.count(f"Throttling update of {mock_plex_server.friendlyName}") == 2 - ) + mock_plex_server = MockPlexServer(config_entry=entry) + + with patch("plexapi.server.PlexServer", return_value=mock_plex_server), patch( + "homeassistant.components.plex.PlexWebsocket.listen" + ): + entry.add_to_hass(hass) + assert await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + server_id = mock_plex_server.machineIdentifier + + with patch.object(mock_plex_server, "clients", return_value=[]), patch.object( + mock_plex_server, "sessions", return_value=[] + ) as mock_update: + # Called immediately + async_dispatcher_send(hass, PLEX_UPDATE_PLATFORMS_SIGNAL.format(server_id)) + await hass.async_block_till_done() + assert mock_update.call_count == 1 + + # Throttled + async_dispatcher_send(hass, PLEX_UPDATE_PLATFORMS_SIGNAL.format(server_id)) + await hass.async_block_till_done() + assert mock_update.call_count == 1 + + # Throttled + async_dispatcher_send(hass, PLEX_UPDATE_PLATFORMS_SIGNAL.format(server_id)) + await hass.async_block_till_done() + assert mock_update.call_count == 1 + + # Called from scheduler + await self.advance(DEBOUNCE_TIMEOUT) + await hass.async_block_till_done() + assert mock_update.call_count == 2 + + # Throttled + async_dispatcher_send(hass, PLEX_UPDATE_PLATFORMS_SIGNAL.format(server_id)) + await hass.async_block_till_done() + assert mock_update.call_count == 2 + + # Called from scheduler + await self.advance(DEBOUNCE_TIMEOUT) + await hass.async_block_till_done() + assert mock_update.call_count == 3