From d18c250acfff9fe612e53d37a64bf57a57a6b902 Mon Sep 17 00:00:00 2001 From: Erik Montnemery Date: Thu, 18 Nov 2021 17:15:40 +0100 Subject: [PATCH] Add minor version support to storage.Store (#59882) --- .../components/onboarding/__init__.py | 8 +- homeassistant/components/person/__init__.py | 2 +- homeassistant/helpers/storage.py | 48 +++++- .../google_assistant/test_helpers.py | 3 + tests/helpers/test_storage.py | 157 ++++++++++++++++++ tests/test_config.py | 1 + 6 files changed, 206 insertions(+), 13 deletions(-) diff --git a/homeassistant/components/onboarding/__init__.py b/homeassistant/components/onboarding/__init__.py index c2ab9a495bf74..6ffee319d3300 100644 --- a/homeassistant/components/onboarding/__init__.py +++ b/homeassistant/components/onboarding/__init__.py @@ -20,14 +20,14 @@ class OnboadingStorage(Store): """Store onboarding data.""" - async def _async_migrate_func(self, old_version, old_data): + async def _async_migrate_func(self, old_major_version, old_minor_version, old_data): """Migrate to the new version.""" # From version 1 -> 2, we automatically mark the integration step done - if old_version < 2: + if old_major_version < 2: old_data["done"].append(STEP_INTEGRATION) - if old_version < 3: + if old_major_version < 3: old_data["done"].append(STEP_CORE_CONFIG) - if old_version < 4: + if old_major_version < 4: old_data["done"].append(STEP_ANALYTICS) return old_data diff --git a/homeassistant/components/person/__init__.py b/homeassistant/components/person/__init__.py index 23b9bafcc4749..3ec08ae518e3c 100644 --- a/homeassistant/components/person/__init__.py +++ b/homeassistant/components/person/__init__.py @@ -148,7 +148,7 @@ async def async_add_user_device_tracker( class PersonStore(Store): """Person storage.""" - async def _async_migrate_func(self, old_version, old_data): + async def _async_migrate_func(self, old_major_version, old_minor_version, old_data): """Migrate to the new version. Migrate storage to use format of collection helper. diff --git a/homeassistant/helpers/storage.py b/homeassistant/helpers/storage.py index 59cf4ff8c2235..ba27696929995 100644 --- a/homeassistant/helpers/storage.py +++ b/homeassistant/helpers/storage.py @@ -4,6 +4,7 @@ import asyncio from collections.abc import Callable from contextlib import suppress +import inspect from json import JSONEncoder import logging import os @@ -75,11 +76,13 @@ def __init__( key: str, private: bool = False, *, - encoder: type[JSONEncoder] | None = None, atomic_writes: bool = False, + encoder: type[JSONEncoder] | None = None, + minor_version: int = 1, ) -> None: """Initialize storage class.""" self.version = version + self.minor_version = minor_version self.key = key self.hass = hass self._private = private @@ -99,8 +102,8 @@ def path(self): async def async_load(self) -> dict | list | None: """Load data. - If the expected version does not match the given version, the migrate - function will be invoked with await migrate_func(version, config). + If the expected version and minor version do not match the given versions, the + migrate function will be invoked with migrate_func(version, minor_version, config). Will ensure that when a call comes in while another one is in progress, the second call will wait and return the result of the first call. @@ -137,7 +140,15 @@ async def _async_load_data(self): if data == {}: return None - if data["version"] == self.version: + + # Add minor_version if not set + if "minor_version" not in data: + data["minor_version"] = 1 + + if ( + data["version"] == self.version + and data["minor_version"] == self.minor_version + ): stored = data["data"] else: _LOGGER.info( @@ -146,13 +157,29 @@ async def _async_load_data(self): data["version"], self.version, ) - stored = await self._async_migrate_func(data["version"], data["data"]) + if len(inspect.signature(self._async_migrate_func).parameters) == 2: + # pylint: disable-next=no-value-for-parameter + stored = await self._async_migrate_func(data["version"], data["data"]) + else: + try: + stored = await self._async_migrate_func( + data["version"], data["minor_version"], data["data"] + ) + except NotImplementedError: + if data["version"] != self.version: + raise + stored = data["data"] return stored async def async_save(self, data: dict | list) -> None: """Save data.""" - self._data = {"version": self.version, "key": self.key, "data": data} + self._data = { + "version": self.version, + "minor_version": self.minor_version, + "key": self.key, + "data": data, + } if self.hass.state == CoreState.stopping: self._async_ensure_final_write_listener() @@ -163,7 +190,12 @@ async def async_save(self, data: dict | list) -> None: @callback def async_delay_save(self, data_func: Callable[[], dict], delay: float = 0) -> None: """Save data with an optional delay.""" - self._data = {"version": self.version, "key": self.key, "data_func": data_func} + self._data = { + "version": self.version, + "minor_version": self.minor_version, + "key": self.key, + "data_func": data_func, + } self._async_cleanup_delay_listener() self._async_ensure_final_write_listener() @@ -248,7 +280,7 @@ def _write_data(self, path: str, data: dict) -> None: atomic_writes=self._atomic_writes, ) - async def _async_migrate_func(self, old_version, old_data): + async def _async_migrate_func(self, old_major_version, old_minor_version, old_data): """Migrate to the new version.""" raise NotImplementedError diff --git a/tests/components/google_assistant/test_helpers.py b/tests/components/google_assistant/test_helpers.py index 9e54e6cff3f59..a260ef039487f 100644 --- a/tests/components/google_assistant/test_helpers.py +++ b/tests/components/google_assistant/test_helpers.py @@ -169,6 +169,7 @@ async def test_agent_user_id_storage(hass, hass_storage): hass_storage["google_assistant"] = { "version": 1, + "minor_version": 1, "key": "google_assistant", "data": {"agent_user_ids": {"agent_1": {}}}, } @@ -178,6 +179,7 @@ async def test_agent_user_id_storage(hass, hass_storage): assert hass_storage["google_assistant"] == { "version": 1, + "minor_version": 1, "key": "google_assistant", "data": {"agent_user_ids": {"agent_1": {}}}, } @@ -188,6 +190,7 @@ async def _check_after_delay(data): assert hass_storage["google_assistant"] == { "version": 1, + "minor_version": 1, "key": "google_assistant", "data": data, } diff --git a/tests/helpers/test_storage.py b/tests/helpers/test_storage.py index 61bf9fa8d0e17..d6a7834012701 100644 --- a/tests/helpers/test_storage.py +++ b/tests/helpers/test_storage.py @@ -17,6 +17,9 @@ from tests.common import async_fire_time_changed MOCK_VERSION = 1 +MOCK_VERSION_2 = 2 +MOCK_MINOR_VERSION_1 = 1 +MOCK_MINOR_VERSION_2 = 2 MOCK_KEY = "storage-test" MOCK_DATA = {"hello": "world"} MOCK_DATA2 = {"goodbye": "cruel world"} @@ -28,6 +31,30 @@ def store(hass): yield storage.Store(hass, MOCK_VERSION, MOCK_KEY) +@pytest.fixture +def store_v_1_1(hass): + """Fixture of a store that prevents writing on Home Assistant stop.""" + yield storage.Store( + hass, MOCK_VERSION, MOCK_KEY, minor_version=MOCK_MINOR_VERSION_1 + ) + + +@pytest.fixture +def store_v_1_2(hass): + """Fixture of a store that prevents writing on Home Assistant stop.""" + yield storage.Store( + hass, MOCK_VERSION, MOCK_KEY, minor_version=MOCK_MINOR_VERSION_2 + ) + + +@pytest.fixture +def store_v_2_1(hass): + """Fixture of a store that prevents writing on Home Assistant stop.""" + yield storage.Store( + hass, MOCK_VERSION_2, MOCK_KEY, minor_version=MOCK_MINOR_VERSION_1 + ) + + async def test_loading(hass, store): """Test we can save and load data.""" await store.async_save(MOCK_DATA) @@ -78,6 +105,7 @@ async def test_saving_with_delay(hass, store, hass_storage): await hass.async_block_till_done() assert hass_storage[store.key] == { "version": MOCK_VERSION, + "minor_version": 1, "key": MOCK_KEY, "data": MOCK_DATA, } @@ -101,6 +129,7 @@ async def test_saving_on_final_write(hass, hass_storage): await hass.async_block_till_done() assert hass_storage[store.key] == { "version": MOCK_VERSION, + "minor_version": 1, "key": MOCK_KEY, "data": MOCK_DATA, } @@ -148,6 +177,7 @@ async def test_loading_while_delay(hass, store, hass_storage): await store.async_save({"delay": "no"}) assert hass_storage[store.key] == { "version": MOCK_VERSION, + "minor_version": 1, "key": MOCK_KEY, "data": {"delay": "no"}, } @@ -155,6 +185,7 @@ async def test_loading_while_delay(hass, store, hass_storage): store.async_delay_save(lambda: {"delay": "yes"}, 1) assert hass_storage[store.key] == { "version": MOCK_VERSION, + "minor_version": 1, "key": MOCK_KEY, "data": {"delay": "no"}, } @@ -170,6 +201,7 @@ async def test_writing_while_writing_delay(hass, store, hass_storage): await store.async_save({"delay": "no"}) assert hass_storage[store.key] == { "version": MOCK_VERSION, + "minor_version": 1, "key": MOCK_KEY, "data": {"delay": "no"}, } @@ -178,6 +210,7 @@ async def test_writing_while_writing_delay(hass, store, hass_storage): await hass.async_block_till_done() assert hass_storage[store.key] == { "version": MOCK_VERSION, + "minor_version": 1, "key": MOCK_KEY, "data": {"delay": "no"}, } @@ -196,6 +229,7 @@ async def test_multiple_delay_save_calls(hass, store, hass_storage): await store.async_save({"delay": "no"}) assert hass_storage[store.key] == { "version": MOCK_VERSION, + "minor_version": 1, "key": MOCK_KEY, "data": {"delay": "no"}, } @@ -204,6 +238,7 @@ async def test_multiple_delay_save_calls(hass, store, hass_storage): await hass.async_block_till_done() assert hass_storage[store.key] == { "version": MOCK_VERSION, + "minor_version": 1, "key": MOCK_KEY, "data": {"delay": "no"}, } @@ -221,6 +256,7 @@ async def test_multiple_save_calls(hass, store, hass_storage): await asyncio.gather(*tasks) assert hass_storage[store.key] == { "version": MOCK_VERSION, + "minor_version": 1, "key": MOCK_KEY, "data": {"savecount": 5}, } @@ -252,6 +288,7 @@ async def test_migrator_existing_config(hass, store, hass_storage): assert hass_storage[store.key] == { "key": MOCK_KEY, "version": MOCK_VERSION, + "minor_version": 1, "data": data, } @@ -277,5 +314,125 @@ async def old_conf_migrate_func(old_config): assert hass_storage[store.key] == { "key": MOCK_KEY, "version": MOCK_VERSION, + "minor_version": 1, "data": data, } + + +async def test_minor_version_default(hass, store, hass_storage): + """Test minor version default.""" + + await store.async_save(MOCK_DATA) + assert hass_storage[store.key]["minor_version"] == 1 + + +async def test_minor_version(hass, store_v_1_2, hass_storage): + """Test minor version.""" + + await store_v_1_2.async_save(MOCK_DATA) + assert hass_storage[store_v_1_2.key]["minor_version"] == MOCK_MINOR_VERSION_2 + + +async def test_migrate_major_not_implemented_raises(hass, store, store_v_2_1): + """Test migrating between major versions fails if not implemented.""" + + await store_v_2_1.async_save(MOCK_DATA) + with pytest.raises(NotImplementedError): + await store.async_load() + + +async def test_migrate_minor_not_implemented( + hass, hass_storage, store_v_1_1, store_v_1_2 +): + """Test migrating between minor versions does not fail if not implemented.""" + + assert store_v_1_1.key == store_v_1_2.key + + await store_v_1_1.async_save(MOCK_DATA) + assert hass_storage[store_v_1_1.key] == { + "key": MOCK_KEY, + "version": MOCK_VERSION, + "minor_version": MOCK_MINOR_VERSION_1, + "data": MOCK_DATA, + } + data = await store_v_1_2.async_load() + assert hass_storage[store_v_1_1.key]["data"] == data + + await store_v_1_2.async_save(MOCK_DATA) + assert hass_storage[store_v_1_2.key] == { + "key": MOCK_KEY, + "version": MOCK_VERSION, + "minor_version": MOCK_MINOR_VERSION_2, + "data": MOCK_DATA, + } + + +async def test_migration(hass, hass_storage, store_v_1_2): + """Test migration.""" + calls = 0 + + class CustomStore(storage.Store): + async def _async_migrate_func( + self, old_major_version, old_minor_version, old_data: dict + ): + nonlocal calls + calls += 1 + assert old_major_version == store_v_1_2.version + assert old_minor_version == store_v_1_2.minor_version + return old_data + + await store_v_1_2.async_save(MOCK_DATA) + assert hass_storage[store_v_1_2.key] == { + "key": MOCK_KEY, + "version": MOCK_VERSION, + "minor_version": MOCK_MINOR_VERSION_2, + "data": MOCK_DATA, + } + assert calls == 0 + + legacy_store = CustomStore(hass, 2, store_v_1_2.key, minor_version=1) + data = await legacy_store.async_load() + assert calls == 1 + assert hass_storage[store_v_1_2.key]["data"] == data + + await legacy_store.async_save(MOCK_DATA) + assert hass_storage[legacy_store.key] == { + "key": MOCK_KEY, + "version": 2, + "minor_version": 1, + "data": MOCK_DATA, + } + + +async def test_legacy_migration(hass, hass_storage, store_v_1_2): + """Test legacy migration method signature.""" + calls = 0 + + class LegacyStore(storage.Store): + async def _async_migrate_func(self, old_version, old_data: dict): + nonlocal calls + calls += 1 + assert old_version == store_v_1_2.version + return old_data + + await store_v_1_2.async_save(MOCK_DATA) + assert hass_storage[store_v_1_2.key] == { + "key": MOCK_KEY, + "version": MOCK_VERSION, + "minor_version": MOCK_MINOR_VERSION_2, + "data": MOCK_DATA, + } + assert calls == 0 + + legacy_store = LegacyStore(hass, 2, store_v_1_2.key, minor_version=1) + data = await legacy_store.async_load() + assert calls == 1 + assert hass_storage[store_v_1_2.key]["data"] == data + + await legacy_store.async_save(MOCK_DATA) + assert hass_storage[legacy_store.key] == { + "key": MOCK_KEY, + "version": 2, + "minor_version": 1, + "data": MOCK_DATA, + } diff --git a/tests/test_config.py b/tests/test_config.py index 441029d27dc65..9f2cc56b1b78c 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -444,6 +444,7 @@ async def test_updating_configuration(hass, hass_storage): }, "key": "core.config", "version": 1, + "minor_version": 1, } hass_storage["core.config"] = dict(core_data) await config_util.async_process_ha_core_config(