From 52b045ed30cbb0c7968bedf09d9753e629b63d2e Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Fri, 14 Feb 2020 15:27:31 -0800 Subject: [PATCH] Fix person device_trackers null (#31829) --- homeassistant/components/person/__init__.py | 39 ++++++++++++++++----- homeassistant/helpers/collection.py | 6 +++- tests/components/person/test_init.py | 14 +++++++- 3 files changed, 49 insertions(+), 10 deletions(-) diff --git a/homeassistant/components/person/__init__.py b/homeassistant/components/person/__init__.py index a1620c578e34..9cd3e882c486 100644 --- a/homeassistant/components/person/__init__.py +++ b/homeassistant/components/person/__init__.py @@ -88,7 +88,11 @@ _UNDEF = object() async def async_create_person(hass, name, *, user_id=None, device_trackers=None): """Create a new person.""" await hass.data[DOMAIN][1].async_create_item( - {ATTR_NAME: name, ATTR_USER_ID: user_id, "device_trackers": device_trackers} + { + ATTR_NAME: name, + ATTR_USER_ID: user_id, + CONF_DEVICE_TRACKERS: device_trackers or [], + } ) @@ -103,14 +107,14 @@ async def async_add_user_device_tracker( if person.get(ATTR_USER_ID) != user_id: continue - device_trackers = person["device_trackers"] + device_trackers = person[CONF_DEVICE_TRACKERS] if device_tracker_entity_id in device_trackers: return await coll.async_update_item( person[collection.CONF_ID], - {"device_trackers": device_trackers + [device_tracker_entity_id]}, + {CONF_DEVICE_TRACKERS: device_trackers + [device_tracker_entity_id]}, ) break @@ -161,6 +165,23 @@ class PersonStorageCollection(collection.StorageCollection): super().__init__(store, logger, id_manager) self.yaml_collection = yaml_collection + async def _async_load_data(self) -> Optional[dict]: + """Load the data. + + A past bug caused onboarding to create invalid person objects. + This patches it up. + """ + data = await super()._async_load_data() + + if data is None: + return data + + for person in data["items"]: + if person[CONF_DEVICE_TRACKERS] is None: + person[CONF_DEVICE_TRACKERS] = [] + + return data + async def async_load(self) -> None: """Load the Storage collection.""" await super().async_load() @@ -179,14 +200,16 @@ class PersonStorageCollection(collection.StorageCollection): return for person in list(self.data.values()): - if entity_id not in person["device_trackers"]: + if entity_id not in person[CONF_DEVICE_TRACKERS]: continue await self.async_update_item( person[collection.CONF_ID], { - "device_trackers": [ - devt for devt in person["device_trackers"] if devt != entity_id + CONF_DEVICE_TRACKERS: [ + devt + for devt in person[CONF_DEVICE_TRACKERS] + if devt != entity_id ] }, ) @@ -408,7 +431,7 @@ class Person(RestoreEntity): self._unsub_track_device() self._unsub_track_device = None - trackers = self._config.get(CONF_DEVICE_TRACKERS) + trackers = self._config[CONF_DEVICE_TRACKERS] if trackers: _LOGGER.debug("Subscribe to device trackers for %s", self.entity_id) @@ -428,7 +451,7 @@ class Person(RestoreEntity): def _update_state(self): """Update the state.""" latest_non_gps_home = latest_not_home = latest_gps = latest = None - for entity_id in self._config.get(CONF_DEVICE_TRACKERS, []): + for entity_id in self._config[CONF_DEVICE_TRACKERS]: state = self.hass.states.get(entity_id) if not state or state.state in IGNORE_STATES: diff --git a/homeassistant/helpers/collection.py b/homeassistant/helpers/collection.py index 1b3721788f51..025c6c07dee6 100644 --- a/homeassistant/helpers/collection.py +++ b/homeassistant/helpers/collection.py @@ -158,9 +158,13 @@ class StorageCollection(ObservableCollection): """Home Assistant object.""" return self.store.hass + async def _async_load_data(self) -> Optional[dict]: + """Load the data.""" + return cast(Optional[dict], await self.store.async_load()) + async def async_load(self) -> None: """Load the storage Manager.""" - raw_storage = cast(Optional[dict], await self.store.async_load()) + raw_storage = await self._async_load_data() if raw_storage is None: raw_storage = {"items": []} diff --git a/tests/components/person/test_init.py b/tests/components/person/test_init.py index e5a414d95ad7..763506199839 100644 --- a/tests/components/person/test_init.py +++ b/tests/components/person/test_init.py @@ -1,7 +1,7 @@ """The tests for the person component.""" import logging -from unittest.mock import patch +from asynctest import patch import pytest from homeassistant.components import person @@ -773,3 +773,15 @@ async def test_reload(hass, hass_admin_user): assert state_2 is None assert state_3 is not None assert state_3.name == "Person 3" + + +async def test_person_storage_fixing_device_trackers(storage_collection): + """Test None device trackers become lists.""" + with patch.object( + storage_collection.store, + "async_load", + return_value={"items": [{"id": "bla", "name": "bla", "device_trackers": None}]}, + ): + await storage_collection.async_load() + + assert storage_collection.data["bla"]["device_trackers"] == []