From 403c6b9e268ac0c5b67d92a925f31fba41407a3c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 14 Apr 2021 10:23:15 -1000 Subject: [PATCH] Stop ssdp scans when stop event happens (#49140) --- homeassistant/components/ssdp/__init__.py | 25 ++-- tests/components/ssdp/test_init.py | 161 +++++++++++++++++----- 2 files changed, 141 insertions(+), 45 deletions(-) diff --git a/homeassistant/components/ssdp/__init__.py b/homeassistant/components/ssdp/__init__.py index 8cad4a74bf8..b6e2897ade2 100644 --- a/homeassistant/components/ssdp/__init__.py +++ b/homeassistant/components/ssdp/__init__.py @@ -9,7 +9,7 @@ from async_upnp_client.search import async_search from defusedxml import ElementTree from netdisco import ssdp, util -from homeassistant.const import EVENT_HOMEASSISTANT_STARTED +from homeassistant.const import EVENT_HOMEASSISTANT_STARTED, EVENT_HOMEASSISTANT_STOP from homeassistant.core import callback from homeassistant.helpers.event import async_track_time_interval from homeassistant.loader import async_get_ssdp @@ -43,12 +43,18 @@ _LOGGER = logging.getLogger(__name__) async def async_setup(hass, config): """Set up the SSDP integration.""" - async def initialize(_): + async def _async_initialize(_): scanner = Scanner(hass, await async_get_ssdp(hass)) await scanner.async_scan(None) - async_track_time_interval(hass, scanner.async_scan, SCAN_INTERVAL) + cancel_scan = async_track_time_interval(hass, scanner.async_scan, SCAN_INTERVAL) - hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STARTED, initialize) + @callback + def _async_stop_scans(event): + cancel_scan() + + hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, _async_stop_scans) + + hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STARTED, _async_initialize) return True @@ -179,14 +185,13 @@ class Scanner: """Fetch an XML description.""" session = self.hass.helpers.aiohttp_client.async_get_clientsession() try: - resp = await session.get(xml_location, timeout=5) - xml = await resp.text(errors="replace") - - # Samsung Smart TV sometimes returns an empty document the - # first time. Retry once. - if not xml: + for _ in range(2): resp = await session.get(xml_location, timeout=5) xml = await resp.text(errors="replace") + # Samsung Smart TV sometimes returns an empty document the + # first time. Retry once. + if xml: + break except (aiohttp.ClientError, asyncio.TimeoutError) as err: _LOGGER.debug("Error fetching %s: %s", xml_location, err) return {} diff --git a/tests/components/ssdp/test_init.py b/tests/components/ssdp/test_init.py index 8ca82e93bfc..f0f4a94e562 100644 --- a/tests/components/ssdp/test_init.py +++ b/tests/components/ssdp/test_init.py @@ -1,31 +1,37 @@ """Test the SSDP integration.""" import asyncio -from unittest.mock import Mock, patch +from datetime import timedelta +from unittest.mock import patch import aiohttp import pytest from homeassistant.components import ssdp +from homeassistant.const import EVENT_HOMEASSISTANT_STARTED, EVENT_HOMEASSISTANT_STOP +from homeassistant.setup import async_setup_component +import homeassistant.util.dt as dt_util -from tests.common import mock_coro +from tests.common import async_fire_time_changed, mock_coro async def test_scan_match_st(hass, caplog): """Test matching based on ST.""" scanner = ssdp.Scanner(hass, {"mock-domain": [{"st": "mock-st"}]}) - async def _inject_entry(*args, **kwargs): - scanner.async_store_entry( - Mock( - st="mock-st", - location=None, - values={"usn": "mock-usn", "server": "mock-server", "ext": ""}, - ) + async def _mock_async_scan(*args, async_callback=None, **kwargs): + await async_callback( + { + "st": "mock-st", + "location": None, + "usn": "mock-usn", + "server": "mock-server", + "ext": "", + } ) with patch( "homeassistant.components.ssdp.async_search", - side_effect=_inject_entry, + side_effect=_mock_async_scan, ), patch.object( hass.config_entries.flow, "async_init", return_value=mock_coro() ) as mock_init: @@ -61,19 +67,25 @@ async def test_scan_match_upnp_devicedesc(hass, aioclient_mock, key): ) scanner = ssdp.Scanner(hass, {"mock-domain": [{key: "Paulus"}]}) - async def _inject_entry(*args, **kwargs): - scanner.async_store_entry( - Mock(st="mock-st", location="http://1.1.1.1", values={}) - ) + async def _mock_async_scan(*args, async_callback=None, **kwargs): + for _ in range(5): + await async_callback( + { + "st": "mock-st", + "location": "http://1.1.1.1", + } + ) with patch( "homeassistant.components.ssdp.async_search", - side_effect=_inject_entry, + side_effect=_mock_async_scan, ), patch.object( hass.config_entries.flow, "async_init", return_value=mock_coro() ) as mock_init: await scanner.async_scan(None) + # If we get duplicate respones, ensure we only look it up once + assert len(aioclient_mock.mock_calls) == 1 assert len(mock_init.mock_calls) == 1 assert mock_init.mock_calls[0][1][0] == "mock-domain" assert mock_init.mock_calls[0][2]["context"] == {"source": "ssdp"} @@ -103,14 +115,17 @@ async def test_scan_not_all_present(hass, aioclient_mock): }, ) - async def _inject_entry(*args, **kwargs): - scanner.async_store_entry( - Mock(st="mock-st", location="http://1.1.1.1", values={}) + async def _mock_async_scan(*args, async_callback=None, **kwargs): + await async_callback( + { + "st": "mock-st", + "location": "http://1.1.1.1", + } ) with patch( "homeassistant.components.ssdp.async_search", - side_effect=_inject_entry, + side_effect=_mock_async_scan, ), patch.object( hass.config_entries.flow, "async_init", return_value=mock_coro() ) as mock_init: @@ -144,14 +159,17 @@ async def test_scan_not_all_match(hass, aioclient_mock): }, ) - async def _inject_entry(*args, **kwargs): - scanner.async_store_entry( - Mock(st="mock-st", location="http://1.1.1.1", values={}) + async def _mock_async_scan(*args, async_callback=None, **kwargs): + await async_callback( + { + "st": "mock-st", + "location": "http://1.1.1.1", + } ) with patch( "homeassistant.components.ssdp.async_search", - side_effect=_inject_entry, + side_effect=_mock_async_scan, ), patch.object( hass.config_entries.flow, "async_init", return_value=mock_coro() ) as mock_init: @@ -166,14 +184,17 @@ async def test_scan_description_fetch_fail(hass, aioclient_mock, exc): aioclient_mock.get("http://1.1.1.1", exc=exc) scanner = ssdp.Scanner(hass, {}) - async def _inject_entry(*args, **kwargs): - scanner.async_store_entry( - Mock(st="mock-st", location="http://1.1.1.1", values={}) + async def _mock_async_scan(*args, async_callback=None, **kwargs): + await async_callback( + { + "st": "mock-st", + "location": "http://1.1.1.1", + } ) with patch( "homeassistant.components.ssdp.async_search", - side_effect=_inject_entry, + side_effect=_mock_async_scan, ): await scanner.async_scan(None) @@ -188,14 +209,17 @@ async def test_scan_description_parse_fail(hass, aioclient_mock): ) scanner = ssdp.Scanner(hass, {}) - async def _inject_entry(*args, **kwargs): - scanner.async_store_entry( - Mock(st="mock-st", location="http://1.1.1.1", values={}) + async def _mock_async_scan(*args, async_callback=None, **kwargs): + await async_callback( + { + "st": "mock-st", + "location": "http://1.1.1.1", + } ) with patch( "homeassistant.components.ssdp.async_search", - side_effect=_inject_entry, + side_effect=_mock_async_scan, ): await scanner.async_scan(None) @@ -224,14 +248,17 @@ async def test_invalid_characters(hass, aioclient_mock): }, ) - async def _inject_entry(*args, **kwargs): - scanner.async_store_entry( - Mock(st="mock-st", location="http://1.1.1.1", values={}) + async def _mock_async_scan(*args, async_callback=None, **kwargs): + await async_callback( + { + "st": "mock-st", + "location": "http://1.1.1.1", + } ) with patch( "homeassistant.components.ssdp.async_search", - side_effect=_inject_entry, + side_effect=_mock_async_scan, ), patch.object( hass.config_entries.flow, "async_init", return_value=mock_coro() ) as mock_init: @@ -246,3 +273,67 @@ async def test_invalid_characters(hass, aioclient_mock): "deviceType": "ABC", "serialNumber": "ÿÿÿÿ", } + + +@patch("homeassistant.components.ssdp.async_search") +async def test_start_stop_scanner(async_search_mock, hass): + """Test we start and stop the scanner.""" + assert await async_setup_component(hass, ssdp.DOMAIN, {ssdp.DOMAIN: {}}) + + hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) + await hass.async_block_till_done() + async_fire_time_changed(hass, dt_util.utcnow() + timedelta(seconds=200)) + await hass.async_block_till_done() + assert async_search_mock.call_count == 2 + + hass.bus.async_fire(EVENT_HOMEASSISTANT_STOP) + await hass.async_block_till_done() + async_fire_time_changed(hass, dt_util.utcnow() + timedelta(seconds=200)) + await hass.async_block_till_done() + assert async_search_mock.call_count == 2 + + +async def test_unexpected_exception_while_fetching(hass, aioclient_mock, caplog): + """Test unexpected exception while fetching.""" + aioclient_mock.get( + "http://1.1.1.1", + text=""" + + + ABC + \xff\xff\xff\xff + + + """, + ) + scanner = ssdp.Scanner( + hass, + { + "mock-domain": [ + { + ssdp.ATTR_UPNP_DEVICE_TYPE: "ABC", + } + ] + }, + ) + + async def _mock_async_scan(*args, async_callback=None, **kwargs): + await async_callback( + { + "st": "mock-st", + "location": "http://1.1.1.1", + } + ) + + with patch( + "homeassistant.components.ssdp.ElementTree.fromstring", side_effect=ValueError + ), patch( + "homeassistant.components.ssdp.async_search", + side_effect=_mock_async_scan, + ), patch.object( + hass.config_entries.flow, "async_init", return_value=mock_coro() + ) as mock_init: + await scanner.async_scan(None) + + assert len(mock_init.mock_calls) == 0 + assert "Failed to fetch ssdp data from: http://1.1.1.1" in caplog.text