From 7fcc2dd44e84711f07f2ff9e55b5979851be3fbd Mon Sep 17 00:00:00 2001 From: Erik Montnemery Date: Fri, 18 Aug 2023 20:15:00 +0200 Subject: [PATCH] Make the check_config script open issue_registry read only (#98545) * Don't blow up if validators can't access the issue registry * Make the check_config script open issue_registry read only * Update tests/helpers/test_issue_registry.py Co-authored-by: Martin Hjelmare --------- Co-authored-by: Martin Hjelmare --- homeassistant/helpers/config_validation.py | 1 + homeassistant/helpers/issue_registry.py | 8 ++- homeassistant/helpers/storage.py | 5 ++ homeassistant/scripts/check_config.py | 2 + tests/helpers/test_issue_registry.py | 73 +++++++++++++++++++++- tests/helpers/test_storage.py | 30 +++++++++ 6 files changed, 115 insertions(+), 4 deletions(-) diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index 122fd752a84..10f4918a20b 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -1122,6 +1122,7 @@ def _no_yaml_config_schema( # pylint: disable-next=import-outside-toplevel from .issue_registry import IssueSeverity, async_create_issue + # HomeAssistantError is raised if called from the wrong thread with contextlib.suppress(HomeAssistantError): hass = async_get_hass() async_create_issue( diff --git a/homeassistant/helpers/issue_registry.py b/homeassistant/helpers/issue_registry.py index 30866ccf7cd..27d568a13de 100644 --- a/homeassistant/helpers/issue_registry.py +++ b/homeassistant/helpers/issue_registry.py @@ -95,16 +95,18 @@ class IssueRegistryStore(Store[dict[str, list[dict[str, Any]]]]): class IssueRegistry: """Class to hold a registry of issues.""" - def __init__(self, hass: HomeAssistant) -> None: + def __init__(self, hass: HomeAssistant, *, read_only: bool = False) -> None: """Initialize the issue registry.""" self.hass = hass self.issues: dict[tuple[str, str], IssueEntry] = {} + self._read_only = read_only self._store = IssueRegistryStore( hass, STORAGE_VERSION_MAJOR, STORAGE_KEY, atomic_writes=True, minor_version=STORAGE_VERSION_MINOR, + read_only=read_only, ) @callback @@ -278,10 +280,10 @@ def async_get(hass: HomeAssistant) -> IssueRegistry: return cast(IssueRegistry, hass.data[DATA_REGISTRY]) -async def async_load(hass: HomeAssistant) -> None: +async def async_load(hass: HomeAssistant, *, read_only: bool = False) -> None: """Load issue registry.""" assert DATA_REGISTRY not in hass.data - hass.data[DATA_REGISTRY] = IssueRegistry(hass) + hass.data[DATA_REGISTRY] = IssueRegistry(hass, read_only=read_only) await hass.data[DATA_REGISTRY].async_load() diff --git a/homeassistant/helpers/storage.py b/homeassistant/helpers/storage.py index dd394c84f91..c83481365ab 100644 --- a/homeassistant/helpers/storage.py +++ b/homeassistant/helpers/storage.py @@ -93,6 +93,7 @@ class Store(Generic[_T]): atomic_writes: bool = False, encoder: type[JSONEncoder] | None = None, minor_version: int = 1, + read_only: bool = False, ) -> None: """Initialize storage class.""" self.version = version @@ -107,6 +108,7 @@ class Store(Generic[_T]): self._load_task: asyncio.Future[_T | None] | None = None self._encoder = encoder self._atomic_writes = atomic_writes + self._read_only = read_only @property def path(self): @@ -344,6 +346,9 @@ class Store(Generic[_T]): self._data = None + if self._read_only: + return + try: await self._async_write_data(self.path, data) except (json_util.SerializationError, WriteError) as err: diff --git a/homeassistant/scripts/check_config.py b/homeassistant/scripts/check_config.py index 5c81c4664da..38fa9cc2463 100644 --- a/homeassistant/scripts/check_config.py +++ b/homeassistant/scripts/check_config.py @@ -19,6 +19,7 @@ from homeassistant.helpers import ( area_registry as ar, device_registry as dr, entity_registry as er, + issue_registry as ir, ) from homeassistant.helpers.check_config import async_check_ha_config_file from homeassistant.util.yaml import Secrets @@ -237,6 +238,7 @@ async def async_check_config(config_dir): await ar.async_load(hass) await dr.async_load(hass) await er.async_load(hass) + await ir.async_load(hass, read_only=True) components = await async_check_ha_config_file(hass) await hass.async_stop(force=True) return components diff --git a/tests/helpers/test_issue_registry.py b/tests/helpers/test_issue_registry.py index d184ccf0a2b..88f97a65421 100644 --- a/tests/helpers/test_issue_registry.py +++ b/tests/helpers/test_issue_registry.py @@ -9,7 +9,7 @@ from homeassistant.helpers import issue_registry as ir from tests.common import async_capture_events, flush_store -async def test_load_issues(hass: HomeAssistant) -> None: +async def test_load_save_issues(hass: HomeAssistant) -> None: """Make sure that we can load/save data correctly.""" issues = [ { @@ -209,6 +209,77 @@ async def test_load_issues(hass: HomeAssistant) -> None: assert issue4_registry2 == issue4 +@pytest.mark.parametrize("load_registries", [False]) +async def test_load_save_issues_read_only( + hass: HomeAssistant, hass_storage: dict[str, Any] +) -> None: + """Make sure that we don't save data when opened in read-only mode.""" + hass_storage[ir.STORAGE_KEY] = { + "version": ir.STORAGE_VERSION_MAJOR, + "minor_version": ir.STORAGE_VERSION_MINOR, + "data": { + "issues": [ + { + "created": "2022-07-19T09:41:13.746514+00:00", + "dismissed_version": "2022.7.0.dev0", + "domain": "test", + "is_persistent": False, + "issue_id": "issue_1", + }, + ] + }, + } + + issues = [ + { + "breaks_in_ha_version": "2022.8", + "domain": "test", + "issue_id": "issue_2", + "is_fixable": True, + "is_persistent": False, + "learn_more_url": "https://theuselessweb.com/abc", + "severity": "other", + "translation_key": "even_worse", + "translation_placeholders": {"def": "456"}, + }, + ] + + events = async_capture_events(hass, ir.EVENT_REPAIRS_ISSUE_REGISTRY_UPDATED) + await ir.async_load(hass, read_only=True) + + for issue in issues: + ir.async_create_issue( + hass, + issue["domain"], + issue["issue_id"], + breaks_in_ha_version=issue["breaks_in_ha_version"], + is_fixable=issue["is_fixable"], + is_persistent=issue["is_persistent"], + learn_more_url=issue["learn_more_url"], + severity=issue["severity"], + translation_key=issue["translation_key"], + translation_placeholders=issue["translation_placeholders"], + ) + + await hass.async_block_till_done() + + assert len(events) == 1 + assert events[0].data == { + "action": "create", + "domain": "test", + "issue_id": "issue_2", + } + + registry = ir.async_get(hass) + assert len(registry.issues) == 2 + + registry2 = ir.IssueRegistry(hass) + await flush_store(registry._store) + await registry2.async_load() + + assert len(registry2.issues) == 1 + + @pytest.mark.parametrize("load_registries", [False]) async def test_loading_issues_from_storage( hass: HomeAssistant, hass_storage: dict[str, Any] diff --git a/tests/helpers/test_storage.py b/tests/helpers/test_storage.py index 81953c7d785..85aa4d2de0e 100644 --- a/tests/helpers/test_storage.py +++ b/tests/helpers/test_storage.py @@ -60,6 +60,12 @@ def store_v_2_1(hass): ) +@pytest.fixture +def read_only_store(hass): + """Fixture of a read only store.""" + return storage.Store(hass, MOCK_VERSION, MOCK_KEY, read_only=True) + + async def test_loading(hass: HomeAssistant, store) -> None: """Test we can save and load data.""" await store.async_save(MOCK_DATA) @@ -703,3 +709,27 @@ async def test_os_error_is_fatal(tmpdir: py.path.local) -> None: await store.async_load() await hass.async_stop(force=True) + + +async def test_read_only_store( + hass: HomeAssistant, read_only_store, hass_storage: dict[str, Any] +) -> None: + """Test store opened in read only mode does not save.""" + read_only_store.async_delay_save(lambda: MOCK_DATA, 1) + assert read_only_store.key not in hass_storage + + async_fire_time_changed(hass, dt_util.utcnow() + timedelta(seconds=1)) + await hass.async_block_till_done() + assert read_only_store.key not in hass_storage + + hass.bus.async_fire(EVENT_HOMEASSISTANT_STOP) + hass.state = CoreState.stopping + await hass.async_block_till_done() + + async_fire_time_changed(hass, dt_util.utcnow() + timedelta(seconds=10)) + await hass.async_block_till_done() + assert read_only_store.key not in hass_storage + + hass.bus.async_fire(EVENT_HOMEASSISTANT_FINAL_WRITE) + await hass.async_block_till_done() + assert read_only_store.key not in hass_storage