From 3b29cbcd61fdf9007b426bad0d4fd73590144cff Mon Sep 17 00:00:00 2001 From: Erik Montnemery Date: Fri, 5 Aug 2022 10:11:20 +0200 Subject: [PATCH] Support creating persistent repairs issues (#76211) --- .../components/repairs/issue_handler.py | 4 + .../components/repairs/issue_registry.py | 115 ++++++++++++----- .../components/repairs/websocket_api.py | 2 +- tests/components/repairs/test_init.py | 8 ++ .../components/repairs/test_issue_registry.py | 117 ++++++++++++++++-- .../components/repairs/test_websocket_api.py | 5 +- 6 files changed, 209 insertions(+), 42 deletions(-) diff --git a/homeassistant/components/repairs/issue_handler.py b/homeassistant/components/repairs/issue_handler.py index c139026ec48..a08fff29598 100644 --- a/homeassistant/components/repairs/issue_handler.py +++ b/homeassistant/components/repairs/issue_handler.py @@ -89,6 +89,7 @@ def async_create_issue( issue_domain: str | None = None, breaks_in_ha_version: str | None = None, is_fixable: bool, + is_persistent: bool = False, learn_more_url: str | None = None, severity: IssueSeverity, translation_key: str, @@ -110,6 +111,7 @@ def async_create_issue( issue_domain=issue_domain, breaks_in_ha_version=breaks_in_ha_version, is_fixable=is_fixable, + is_persistent=is_persistent, learn_more_url=learn_more_url, severity=severity, translation_key=translation_key, @@ -124,6 +126,7 @@ def create_issue( *, breaks_in_ha_version: str | None = None, is_fixable: bool, + is_persistent: bool = False, learn_more_url: str | None = None, severity: IssueSeverity, translation_key: str, @@ -139,6 +142,7 @@ def create_issue( issue_id, breaks_in_ha_version=breaks_in_ha_version, is_fixable=is_fixable, + is_persistent=is_persistent, learn_more_url=learn_more_url, severity=severity, translation_key=translation_key, diff --git a/homeassistant/components/repairs/issue_registry.py b/homeassistant/components/repairs/issue_registry.py index bd201f1007c..c7502ecf397 100644 --- a/homeassistant/components/repairs/issue_registry.py +++ b/homeassistant/components/repairs/issue_registry.py @@ -3,7 +3,7 @@ from __future__ import annotations import dataclasses from datetime import datetime -from typing import Optional, cast +from typing import Any, cast from homeassistant.const import __version__ as ha_version from homeassistant.core import HomeAssistant, callback @@ -15,7 +15,8 @@ from .models import IssueSeverity DATA_REGISTRY = "issue_registry" EVENT_REPAIRS_ISSUE_REGISTRY_UPDATED = "repairs_issue_registry_updated" STORAGE_KEY = "repairs.issue_registry" -STORAGE_VERSION = 1 +STORAGE_VERSION_MAJOR = 1 +STORAGE_VERSION_MINOR = 2 SAVE_DELAY = 10 @@ -29,14 +30,53 @@ class IssueEntry: dismissed_version: str | None domain: str is_fixable: bool | None - issue_id: str + is_persistent: bool # Used if an integration creates issues for other integrations (ie alerts) issue_domain: str | None + issue_id: str learn_more_url: str | None severity: IssueSeverity | None translation_key: str | None translation_placeholders: dict[str, str] | None + def to_json(self) -> dict[str, Any]: + """Return a JSON serializable representation for storage.""" + result = { + "created": self.created.isoformat(), + "dismissed_version": self.dismissed_version, + "domain": self.domain, + "is_persistent": False, + "issue_id": self.issue_id, + } + if not self.is_persistent: + return result + return { + **result, + "breaks_in_ha_version": self.breaks_in_ha_version, + "is_fixable": self.is_fixable, + "is_persistent": True, + "issue_domain": self.issue_domain, + "issue_id": self.issue_id, + "learn_more_url": self.learn_more_url, + "severity": self.severity, + "translation_key": self.translation_key, + "translation_placeholders": self.translation_placeholders, + } + + +class IssueRegistryStore(Store[dict[str, list[dict[str, Any]]]]): + """Store entity registry data.""" + + async def _async_migrate_func( + self, old_major_version: int, old_minor_version: int, old_data: dict[str, Any] + ) -> dict[str, Any]: + """Migrate to the new version.""" + if old_major_version == 1 and old_minor_version < 2: + # Version 1.2 adds is_persistent + for issue in old_data["issues"]: + issue["is_persistent"] = False + return old_data + class IssueRegistry: """Class to hold a registry of issues.""" @@ -45,8 +85,12 @@ class IssueRegistry: """Initialize the issue registry.""" self.hass = hass self.issues: dict[tuple[str, str], IssueEntry] = {} - self._store = Store[dict[str, list[dict[str, Optional[str]]]]]( - hass, STORAGE_VERSION, STORAGE_KEY, atomic_writes=True + self._store = IssueRegistryStore( + hass, + STORAGE_VERSION_MAJOR, + STORAGE_KEY, + atomic_writes=True, + minor_version=STORAGE_VERSION_MINOR, ) @callback @@ -63,6 +107,7 @@ class IssueRegistry: issue_domain: str | None = None, breaks_in_ha_version: str | None = None, is_fixable: bool, + is_persistent: bool, learn_more_url: str | None = None, severity: IssueSeverity, translation_key: str, @@ -78,6 +123,7 @@ class IssueRegistry: dismissed_version=None, domain=domain, is_fixable=is_fixable, + is_persistent=is_persistent, issue_domain=issue_domain, issue_id=issue_id, learn_more_url=learn_more_url, @@ -97,6 +143,7 @@ class IssueRegistry: active=True, breaks_in_ha_version=breaks_in_ha_version, is_fixable=is_fixable, + is_persistent=is_persistent, issue_domain=issue_domain, learn_more_url=learn_more_url, severity=severity, @@ -151,21 +198,39 @@ class IssueRegistry: if isinstance(data, dict): for issue in data["issues"]: - assert issue["created"] and issue["domain"] and issue["issue_id"] - issues[(issue["domain"], issue["issue_id"])] = IssueEntry( - active=False, - breaks_in_ha_version=None, - created=cast(datetime, dt_util.parse_datetime(issue["created"])), - dismissed_version=issue["dismissed_version"], - domain=issue["domain"], - is_fixable=None, - issue_id=issue["issue_id"], - issue_domain=None, - learn_more_url=None, - severity=None, - translation_key=None, - translation_placeholders=None, - ) + created = cast(datetime, dt_util.parse_datetime(issue["created"])) + if issue["is_persistent"]: + issues[(issue["domain"], issue["issue_id"])] = IssueEntry( + active=True, + breaks_in_ha_version=issue["breaks_in_ha_version"], + created=created, + dismissed_version=issue["dismissed_version"], + domain=issue["domain"], + is_fixable=issue["is_fixable"], + is_persistent=issue["is_persistent"], + issue_id=issue["issue_id"], + issue_domain=issue["issue_domain"], + learn_more_url=issue["learn_more_url"], + severity=issue["severity"], + translation_key=issue["translation_key"], + translation_placeholders=issue["translation_placeholders"], + ) + else: + issues[(issue["domain"], issue["issue_id"])] = IssueEntry( + active=False, + breaks_in_ha_version=None, + created=created, + dismissed_version=issue["dismissed_version"], + domain=issue["domain"], + is_fixable=None, + is_persistent=issue["is_persistent"], + issue_id=issue["issue_id"], + issue_domain=None, + learn_more_url=None, + severity=None, + translation_key=None, + translation_placeholders=None, + ) self.issues = issues @@ -179,15 +244,7 @@ class IssueRegistry: """Return data of issue registry to store in a file.""" data = {} - data["issues"] = [ - { - "created": entry.created.isoformat(), - "dismissed_version": entry.dismissed_version, - "domain": entry.domain, - "issue_id": entry.issue_id, - } - for entry in self.issues.values() - ] + data["issues"] = [entry.to_json() for entry in self.issues.values()] return data diff --git a/homeassistant/components/repairs/websocket_api.py b/homeassistant/components/repairs/websocket_api.py index 2e9fcc5f8e4..ff0ac5ba8f9 100644 --- a/homeassistant/components/repairs/websocket_api.py +++ b/homeassistant/components/repairs/websocket_api.py @@ -64,7 +64,7 @@ def ws_list_issues( """Return a list of issues.""" def ws_dict(kv_pairs: list[tuple[Any, Any]]) -> dict[Any, Any]: - result = {k: v for k, v in kv_pairs if k not in ("active")} + result = {k: v for k, v in kv_pairs if k not in ("active", "is_persistent")} result["ignored"] = result["dismissed_version"] is not None result["created"] = result["created"].isoformat() return result diff --git a/tests/components/repairs/test_init.py b/tests/components/repairs/test_init.py index d70f6c6e11d..1fc8367e4c3 100644 --- a/tests/components/repairs/test_init.py +++ b/tests/components/repairs/test_init.py @@ -68,6 +68,7 @@ async def test_create_update_issue(hass: HomeAssistant, hass_ws_client) -> None: issue["issue_id"], breaks_in_ha_version=issue["breaks_in_ha_version"], is_fixable=issue["is_fixable"], + is_persistent=False, learn_more_url=issue["learn_more_url"], severity=issue["severity"], translation_key=issue["translation_key"], @@ -98,6 +99,7 @@ async def test_create_update_issue(hass: HomeAssistant, hass_ws_client) -> None: issues[0]["issue_id"], breaks_in_ha_version=issues[0]["breaks_in_ha_version"], is_fixable=issues[0]["is_fixable"], + is_persistent=False, issue_domain="my_issue_domain", learn_more_url="blablabla", severity=issues[0]["severity"], @@ -146,6 +148,7 @@ async def test_create_issue_invalid_version( issue["issue_id"], breaks_in_ha_version=issue["breaks_in_ha_version"], is_fixable=issue["is_fixable"], + is_persistent=False, learn_more_url=issue["learn_more_url"], severity=issue["severity"], translation_key=issue["translation_key"], @@ -192,6 +195,7 @@ async def test_ignore_issue(hass: HomeAssistant, hass_ws_client) -> None: issue["issue_id"], breaks_in_ha_version=issue["breaks_in_ha_version"], is_fixable=issue["is_fixable"], + is_persistent=False, learn_more_url=issue["learn_more_url"], severity=issue["severity"], translation_key=issue["translation_key"], @@ -283,6 +287,7 @@ async def test_ignore_issue(hass: HomeAssistant, hass_ws_client) -> None: issues[0]["issue_id"], breaks_in_ha_version=issues[0]["breaks_in_ha_version"], is_fixable=issues[0]["is_fixable"], + is_persistent=False, learn_more_url="blablabla", severity=issues[0]["severity"], translation_key=issues[0]["translation_key"], @@ -351,6 +356,7 @@ async def test_delete_issue(hass: HomeAssistant, hass_ws_client, freezer) -> Non issue["issue_id"], breaks_in_ha_version=issue["breaks_in_ha_version"], is_fixable=issue["is_fixable"], + is_persistent=False, learn_more_url=issue["learn_more_url"], severity=issue["severity"], translation_key=issue["translation_key"], @@ -422,6 +428,7 @@ async def test_delete_issue(hass: HomeAssistant, hass_ws_client, freezer) -> Non issue["issue_id"], breaks_in_ha_version=issue["breaks_in_ha_version"], is_fixable=issue["is_fixable"], + is_persistent=False, learn_more_url=issue["learn_more_url"], severity=issue["severity"], translation_key=issue["translation_key"], @@ -492,6 +499,7 @@ async def test_sync_methods( "sync_issue", breaks_in_ha_version="2022.9", is_fixable=True, + is_persistent=False, learn_more_url="https://theuselessweb.com", severity=IssueSeverity.ERROR, translation_key="abc_123", diff --git a/tests/components/repairs/test_issue_registry.py b/tests/components/repairs/test_issue_registry.py index 523f75bfdc2..ff6c4b996da 100644 --- a/tests/components/repairs/test_issue_registry.py +++ b/tests/components/repairs/test_issue_registry.py @@ -21,6 +21,7 @@ async def test_load_issues(hass: HomeAssistant) -> None: "domain": "test", "issue_id": "issue_1", "is_fixable": True, + "is_persistent": False, "learn_more_url": "https://theuselessweb.com", "severity": "error", "translation_key": "abc_123", @@ -31,6 +32,7 @@ async def test_load_issues(hass: HomeAssistant) -> None: "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", @@ -41,11 +43,23 @@ async def test_load_issues(hass: HomeAssistant) -> None: "domain": "test", "issue_id": "issue_3", "is_fixable": True, + "is_persistent": False, "learn_more_url": "https://checkboxrace.com", "severity": "other", "translation_key": "even_worse", "translation_placeholders": {"def": "789"}, }, + { + "breaks_in_ha_version": "2022.6", + "domain": "test", + "issue_id": "issue_4", + "is_fixable": True, + "is_persistent": True, + "learn_more_url": "https://checkboxrace.com/blah", + "severity": "other", + "translation_key": "even_worse", + "translation_placeholders": {"xyz": "abc"}, + }, ] events = async_capture_events( @@ -59,6 +73,7 @@ async def test_load_issues(hass: HomeAssistant) -> None: 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"], @@ -67,7 +82,7 @@ async def test_load_issues(hass: HomeAssistant) -> None: await hass.async_block_till_done() - assert len(events) == 3 + assert len(events) == 4 assert events[0].data == { "action": "create", "domain": "test", @@ -83,12 +98,17 @@ async def test_load_issues(hass: HomeAssistant) -> None: "domain": "test", "issue_id": "issue_3", } + assert events[3].data == { + "action": "create", + "domain": "test", + "issue_id": "issue_4", + } async_ignore_issue(hass, issues[0]["domain"], issues[0]["issue_id"], True) await hass.async_block_till_done() - assert len(events) == 4 - assert events[3].data == { + assert len(events) == 5 + assert events[4].data == { "action": "update", "domain": "test", "issue_id": "issue_1", @@ -97,17 +117,18 @@ async def test_load_issues(hass: HomeAssistant) -> None: async_delete_issue(hass, issues[2]["domain"], issues[2]["issue_id"]) await hass.async_block_till_done() - assert len(events) == 5 - assert events[4].data == { + assert len(events) == 6 + assert events[5].data == { "action": "remove", "domain": "test", "issue_id": "issue_3", } registry: issue_registry.IssueRegistry = hass.data[issue_registry.DATA_REGISTRY] - assert len(registry.issues) == 2 + assert len(registry.issues) == 3 issue1 = registry.async_get_issue("test", "issue_1") issue2 = registry.async_get_issue("test", "issue_2") + issue4 = registry.async_get_issue("test", "issue_4") registry2 = issue_registry.IssueRegistry(hass) await flush_store(registry._store) @@ -116,17 +137,91 @@ async def test_load_issues(hass: HomeAssistant) -> None: assert list(registry.issues) == list(registry2.issues) issue1_registry2 = registry2.async_get_issue("test", "issue_1") - assert issue1_registry2.created == issue1.created - assert issue1_registry2.dismissed_version == issue1.dismissed_version + assert issue1_registry2 == issue_registry.IssueEntry( + active=False, + breaks_in_ha_version=None, + created=issue1.created, + dismissed_version=issue1.dismissed_version, + domain=issue1.domain, + is_fixable=None, + is_persistent=issue1.is_persistent, + issue_domain=None, + issue_id=issue1.issue_id, + learn_more_url=None, + severity=None, + translation_key=None, + translation_placeholders=None, + ) issue2_registry2 = registry2.async_get_issue("test", "issue_2") - assert issue2_registry2.created == issue2.created - assert issue2_registry2.dismissed_version == issue2.dismissed_version + assert issue2_registry2 == issue_registry.IssueEntry( + active=False, + breaks_in_ha_version=None, + created=issue2.created, + dismissed_version=issue2.dismissed_version, + domain=issue2.domain, + is_fixable=None, + is_persistent=issue2.is_persistent, + issue_domain=None, + issue_id=issue2.issue_id, + learn_more_url=None, + severity=None, + translation_key=None, + translation_placeholders=None, + ) + issue4_registry2 = registry2.async_get_issue("test", "issue_4") + assert issue4_registry2 == issue4 async def test_loading_issues_from_storage(hass: HomeAssistant, hass_storage) -> None: """Test loading stored issues on start.""" hass_storage[issue_registry.STORAGE_KEY] = { - "version": issue_registry.STORAGE_VERSION, + "version": issue_registry.STORAGE_VERSION_MAJOR, + "minor_version": issue_registry.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", + }, + { + "created": "2022-07-19T19:41:13.746514+00:00", + "dismissed_version": None, + "domain": "test", + "is_persistent": False, + "issue_id": "issue_2", + }, + { + "breaks_in_ha_version": "2022.6", + "created": "2022-07-19T19:41:13.746514+00:00", + "dismissed_version": None, + "domain": "test", + "issue_domain": "blubb", + "issue_id": "issue_4", + "is_fixable": True, + "is_persistent": True, + "learn_more_url": "https://checkboxrace.com/blah", + "severity": "other", + "translation_key": "even_worse", + "translation_placeholders": {"xyz": "abc"}, + }, + ] + }, + } + + assert await async_setup_component(hass, DOMAIN, {}) + + registry: issue_registry.IssueRegistry = hass.data[issue_registry.DATA_REGISTRY] + assert len(registry.issues) == 3 + + +async def test_migration_1_1(hass: HomeAssistant, hass_storage) -> None: + """Test migration from version 1.1.""" + hass_storage[issue_registry.STORAGE_KEY] = { + "version": 1, + "minor_version": 1, "data": { "issues": [ { diff --git a/tests/components/repairs/test_websocket_api.py b/tests/components/repairs/test_websocket_api.py index d778b043832..359024f9fe5 100644 --- a/tests/components/repairs/test_websocket_api.py +++ b/tests/components/repairs/test_websocket_api.py @@ -44,6 +44,7 @@ async def create_issues(hass, ws_client): issue["issue_id"], breaks_in_ha_version=issue["breaks_in_ha_version"], is_fixable=issue["is_fixable"], + is_persistent=False, learn_more_url=issue["learn_more_url"], severity=issue["severity"], translation_key=issue["translation_key"], @@ -379,13 +380,14 @@ async def test_list_issues(hass: HomeAssistant, hass_storage, hass_ws_client) -> # Add an inactive issue, this should not be exposed in the list hass_storage[issue_registry.STORAGE_KEY] = { - "version": issue_registry.STORAGE_VERSION, + "version": issue_registry.STORAGE_VERSION_MAJOR, "data": { "issues": [ { "created": "2022-07-19T09:41:13.746514+00:00", "dismissed_version": None, "domain": "test", + "is_persistent": False, "issue_id": "issue_3_inactive", "issue_domain": None, }, @@ -435,6 +437,7 @@ async def test_list_issues(hass: HomeAssistant, hass_storage, hass_ws_client) -> issue["issue_id"], breaks_in_ha_version=issue["breaks_in_ha_version"], is_fixable=issue["is_fixable"], + is_persistent=False, learn_more_url=issue["learn_more_url"], severity=issue["severity"], translation_key=issue["translation_key"],