From 5bd113986796893ab9f7c2acbc6a3443379666cd Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Sun, 5 Dec 2021 13:02:37 -0800 Subject: [PATCH] Fix regression in nest event media player with multiple devices (#61064) --- homeassistant/components/nest/manifest.json | 2 +- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/nest/common.py | 36 +++----- tests/components/nest/test_config_flow_sdm.py | 12 +-- tests/components/nest/test_media_source.py | 82 ++++++++++++++++++- 6 files changed, 97 insertions(+), 39 deletions(-) diff --git a/homeassistant/components/nest/manifest.json b/homeassistant/components/nest/manifest.json index 3a4f64877d2..a82f8395733 100644 --- a/homeassistant/components/nest/manifest.json +++ b/homeassistant/components/nest/manifest.json @@ -4,7 +4,7 @@ "config_flow": true, "dependencies": ["ffmpeg", "http", "media_source"], "documentation": "https://www.home-assistant.io/integrations/nest", - "requirements": ["python-nest==4.1.0", "google-nest-sdm==0.4.2"], + "requirements": ["python-nest==4.1.0", "google-nest-sdm==0.4.3"], "codeowners": ["@allenporter"], "quality_scale": "platinum", "dhcp": [ diff --git a/requirements_all.txt b/requirements_all.txt index b65d183a7d6..63bbc61ff84 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -738,7 +738,7 @@ google-cloud-pubsub==2.1.0 google-cloud-texttospeech==0.4.0 # homeassistant.components.nest -google-nest-sdm==0.4.2 +google-nest-sdm==0.4.3 # homeassistant.components.google_travel_time googlemaps==2.5.1 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index b997f4cb571..18841a1f537 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -461,7 +461,7 @@ google-api-python-client==1.6.4 google-cloud-pubsub==2.1.0 # homeassistant.components.nest -google-nest-sdm==0.4.2 +google-nest-sdm==0.4.3 # homeassistant.components.google_travel_time googlemaps==2.5.1 diff --git a/tests/components/nest/common.py b/tests/components/nest/common.py index a0ba813ab28..35183a441a5 100644 --- a/tests/components/nest/common.py +++ b/tests/components/nest/common.py @@ -53,31 +53,12 @@ def create_config_entry(hass, token_expiration_time=None) -> MockConfigEntry: return config_entry -class FakeDeviceManager(DeviceManager): - """Fake DeviceManager that can supply a list of devices and structures.""" - - def __init__(self, devices: dict, structures: dict): - """Initialize FakeDeviceManager.""" - super().__init__() - self._devices = devices - - @property - def structures(self) -> dict: - """Override structures with fake result.""" - return self._structures - - @property - def devices(self) -> dict: - """Override devices with fake result.""" - return self._devices - - class FakeSubscriber(GoogleNestSubscriber): """Fake subscriber that supplies a FakeDeviceManager.""" - def __init__(self, device_manager: FakeDeviceManager): + def __init__(self): """Initialize Fake Subscriber.""" - self._device_manager = device_manager + self._device_manager = DeviceManager() def set_update_callback(self, callback: Callable[[EventMessage], Awaitable[None]]): """Capture the callback set by Home Assistant.""" @@ -121,8 +102,14 @@ async def async_setup_sdm_platform( """Set up the platform and prerequisites.""" if with_config: create_config_entry(hass) - device_manager = FakeDeviceManager(devices=devices, structures=structures) - subscriber = FakeSubscriber(device_manager) + subscriber = FakeSubscriber() + device_manager = await subscriber.async_get_device_manager() + if devices: + for device in devices.values(): + device_manager.add_device(device) + if structures: + for structure in structures.values(): + device_manager.add_structure(structure) with patch( "homeassistant.helpers.config_entry_oauth2_flow.async_get_config_entry_implementation" ), patch("homeassistant.components.nest.PLATFORMS", [platform]), patch( @@ -131,4 +118,7 @@ async def async_setup_sdm_platform( ): assert await async_setup_component(hass, DOMAIN, CONFIG) await hass.async_block_till_done() + # Disabled to reduce setup burden, and enabled manually by tests that + # need to exercise this + subscriber.cache_policy.fetch = False return subscriber diff --git a/tests/components/nest/test_config_flow_sdm.py b/tests/components/nest/test_config_flow_sdm.py index 5d6987f94f7..d4af62cb255 100644 --- a/tests/components/nest/test_config_flow_sdm.py +++ b/tests/components/nest/test_config_flow_sdm.py @@ -17,7 +17,7 @@ from homeassistant.const import CONF_CLIENT_ID, CONF_CLIENT_SECRET from homeassistant.core import HomeAssistant from homeassistant.helpers import config_entry_oauth2_flow -from .common import FakeDeviceManager, FakeSubscriber, MockConfigEntry +from .common import FakeSubscriber, MockConfigEntry CLIENT_ID = "1234" CLIENT_SECRET = "5678" @@ -43,15 +43,9 @@ APP_REDIRECT_URL = "urn:ietf:wg:oauth:2.0:oob" @pytest.fixture -def device_manager() -> FakeDeviceManager: - """Create FakeDeviceManager.""" - return FakeDeviceManager(devices={}, structures={}) - - -@pytest.fixture -def subscriber(device_manager: FakeDeviceManager) -> FakeSubscriber: +def subscriber() -> FakeSubscriber: """Create FakeSubscriber.""" - return FakeSubscriber(device_manager) + return FakeSubscriber() def get_config_entry(hass): diff --git a/tests/components/nest/test_media_source.py b/tests/components/nest/test_media_source.py index 67d6ba2f229..82c87579525 100644 --- a/tests/components/nest/test_media_source.py +++ b/tests/components/nest/test_media_source.py @@ -81,7 +81,7 @@ async def async_setup_devices(hass, auth, device_type, traits={}, events=[]): return subscriber -def create_event(event_id, event_type, timestamp=None): +def create_event(event_id, event_type, timestamp=None, device_id=None): """Create an EventMessage for a single event type.""" if not timestamp: timestamp = dt_util.now() @@ -91,17 +91,19 @@ def create_event(event_id, event_type, timestamp=None): "eventId": event_id, }, } - return create_event_message(event_id, event_data, timestamp) + return create_event_message(event_id, event_data, timestamp, device_id=device_id) -def create_event_message(event_id, event_data, timestamp): +def create_event_message(event_id, event_data, timestamp, device_id=None): """Create an EventMessage for a single event type.""" + if device_id is None: + device_id = DEVICE_ID return EventMessage( { "eventId": f"{event_id}-{timestamp}", "timestamp": timestamp.isoformat(timespec="seconds"), "resourceUpdate": { - "name": DEVICE_ID, + "name": device_id, "events": event_data, }, }, @@ -568,3 +570,75 @@ async def test_media_permission_unauthorized(hass, auth, hass_client, hass_admin assert response.status == HTTPStatus.UNAUTHORIZED, ( "Response not matched: %s" % response ) + + +async def test_multiple_devices(hass, auth, hass_client): + """Test events received for multiple devices.""" + device_id1 = f"{DEVICE_ID}-1" + device_id2 = f"{DEVICE_ID}-2" + + devices = { + device_id1: Device.MakeDevice( + { + "name": device_id1, + "type": CAMERA_DEVICE_TYPE, + "traits": CAMERA_TRAITS, + }, + auth=auth, + ), + device_id2: Device.MakeDevice( + { + "name": device_id2, + "type": CAMERA_DEVICE_TYPE, + "traits": CAMERA_TRAITS, + }, + auth=auth, + ), + } + subscriber = await async_setup_sdm_platform(hass, PLATFORM, devices=devices) + + device_registry = dr.async_get(hass) + device1 = device_registry.async_get_device({(DOMAIN, device_id1)}) + assert device1 + device2 = device_registry.async_get_device({(DOMAIN, device_id2)}) + assert device2 + + # Very no events have been received yet + browse = await media_source.async_browse_media( + hass, f"{const.URI_SCHEME}{DOMAIN}/{device1.id}" + ) + assert len(browse.children) == 0 + browse = await media_source.async_browse_media( + hass, f"{const.URI_SCHEME}{DOMAIN}/{device2.id}" + ) + assert len(browse.children) == 0 + + # Send events for device #1 + for i in range(0, 5): + await subscriber.async_receive_event( + create_event(f"event-id-{i}", PERSON_EVENT, device_id=device_id1) + ) + + browse = await media_source.async_browse_media( + hass, f"{const.URI_SCHEME}{DOMAIN}/{device1.id}" + ) + assert len(browse.children) == 5 + browse = await media_source.async_browse_media( + hass, f"{const.URI_SCHEME}{DOMAIN}/{device2.id}" + ) + assert len(browse.children) == 0 + + # Send events for device #2 + for i in range(0, 3): + await subscriber.async_receive_event( + create_event(f"other-id-{i}", PERSON_EVENT, device_id=device_id2) + ) + + browse = await media_source.async_browse_media( + hass, f"{const.URI_SCHEME}{DOMAIN}/{device1.id}" + ) + assert len(browse.children) == 5 + browse = await media_source.async_browse_media( + hass, f"{const.URI_SCHEME}{DOMAIN}/{device2.id}" + ) + assert len(browse.children) == 3