Correct cleanup of sensor statistics repairs (#127826)

homeassistant/components/sensor/recorder.py

@@ -6,7 +6,6 @@ from collections import defaultdict
 from collections.abc import Callable, Iterable
 from contextlib import suppress
 import datetime
-from functools import partial
 import itertools
 import logging
 import math
@@ -39,6 +38,7 @@ from homeassistant.helpers.entity import entity_sources
 from homeassistant.helpers.typing import UNDEFINED, UndefinedType
 from homeassistant.loader import async_suggest_report_issue
 from homeassistant.util import dt as dt_util
+from homeassistant.util.async_ import run_callback_threadsafe
 from homeassistant.util.enum import try_parse_enum
 from homeassistant.util.hass_dict import HassKey
 
@@ -686,7 +686,6 @@ def list_statistic_ids(
 @callback
 def _update_issues(
     report_issue: Callable[[str, str, dict[str, Any]], None],
-    clear_issue: Callable[[str, str], None],
     sensor_states: list[State],
     metadatas: dict[str, tuple[int, StatisticMetaData]],
 ) -> None:
@@ -707,8 +706,6 @@ def _update_issues(
                     entity_id,
                     {"statistic_id": entity_id},
                 )
-            else:
-                clear_issue("state_class_removed", entity_id)
 
             metadata_unit = metadata[1]["unit_of_measurement"]
             converter = statistics.STATISTIC_UNIT_TO_UNIT_CONVERTER.get(metadata_unit)
@@ -725,8 +722,6 @@ def _update_issues(
                             "supported_unit": metadata_unit,
                         },
                     )
-                else:
-                    clear_issue("units_changed", entity_id)
             elif numeric and state_unit not in converter.VALID_UNITS:
                 # The state unit can't be converted to the unit in metadata
                 valid_units = (unit or "<None>" for unit in converter.VALID_UNITS)
@@ -741,8 +736,6 @@ def _update_issues(
                         "supported_unit": valid_units_str,
                     },
                 )
-            else:
-                clear_issue("units_changed", entity_id)
 
 
 def update_statistics_issues(
@@ -756,36 +749,50 @@ def update_statistics_issues(
         instance, session, statistic_source=RECORDER_DOMAIN
     )
 
+    @callback
+    def get_sensor_statistics_issues(hass: HomeAssistant) -> set[str]:
+        """Return a list of statistics issues."""
+        issues = set()
+        issue_registry = ir.async_get(hass)
+        for issue in issue_registry.issues.values():
+            if (
+                issue.domain != DOMAIN
+                or not (issue_data := issue.data)
+                or issue_data.get("issue_type")
+                not in ("state_class_removed", "units_changed")
+            ):
+                continue
+            issues.add(issue.issue_id)
+        return issues
+
+    issues = run_callback_threadsafe(
+        hass.loop, get_sensor_statistics_issues, hass
+    ).result()
+
     def create_issue_registry_issue(
         issue_type: str, statistic_id: str, data: dict[str, Any]
     ) -> None:
         """Create an issue registry issue."""
-        hass.loop.call_soon_threadsafe(
+        issue_id = f"{issue_type}_{statistic_id}"
-            partial(
+        issues.discard(issue_id)
-                ir.async_create_issue,
+        ir.create_issue(
-                hass,
+            hass,
-                DOMAIN,
+            DOMAIN,
-                f"{issue_type}_{statistic_id}",
+            issue_id,
-                data=data | {"issue_type": issue_type},
+            data=data | {"issue_type": issue_type},
-                is_fixable=False,
+            is_fixable=False,
-                severity=ir.IssueSeverity.WARNING,
+            severity=ir.IssueSeverity.WARNING,
-                translation_key=issue_type,
+            translation_key=issue_type,
-                translation_placeholders=data,
+            translation_placeholders=data,
-            )
-        )
-
-    def delete_issue_registry_issue(issue_type: str, statistic_id: str) -> None:
-        """Delete an issue registry issue."""
-        hass.loop.call_soon_threadsafe(
-            ir.async_delete_issue, hass, DOMAIN, f"{issue_type}_{statistic_id}"
         )
 
     _update_issues(
         create_issue_registry_issue,
-        delete_issue_registry_issue,
         sensor_states,
         metadatas,
     )
+    for issue_id in issues:
+        hass.loop.call_soon_threadsafe(ir.async_delete_issue, hass, DOMAIN, issue_id)
 
 
 def validate_statistics(
@@ -811,7 +818,6 @@ def validate_statistics(
 
     _update_issues(
         create_statistic_validation_issue,
-        lambda issue_type, statistic_id: None,
         sensor_states,
         metadatas,
     )

tests/components/sensor/test_recorder.py

@@ -4682,6 +4682,65 @@ async def test_validate_statistics_state_class_removed(
     await assert_validation_result(hass, client, {}, {})
 
 
+@pytest.mark.parametrize(
+    ("units", "attributes", "unit"),
+    [
+        (US_CUSTOMARY_SYSTEM, POWER_SENSOR_ATTRIBUTES, "W"),
+    ],
+)
+async def test_validate_statistics_state_class_removed_issue_cleaned_up(
+    hass: HomeAssistant,
+    hass_ws_client: WebSocketGenerator,
+    units,
+    attributes,
+    unit,
+) -> None:
+    """Test validate_statistics."""
+    now = get_start_time(dt_util.utcnow())
+
+    hass.config.units = units
+    await async_setup_component(hass, "sensor", {})
+    await async_recorder_block_till_done(hass)
+    client = await hass_ws_client()
+
+    # No statistics, no state - empty response
+    await assert_validation_result(hass, client, {}, {})
+
+    # No statistics, valid state - empty response
+    hass.states.async_set(
+        "sensor.test", 10, attributes=attributes, timestamp=now.timestamp()
+    )
+    await hass.async_block_till_done()
+    await assert_validation_result(hass, client, {}, {})
+
+    # Statistics has run, empty response
+    do_adhoc_statistics(hass, start=now)
+    await async_recorder_block_till_done(hass)
+    await assert_validation_result(hass, client, {}, {})
+
+    # State update with invalid state class, expect error
+    _attributes = dict(attributes)
+    _attributes.pop("state_class")
+    hass.states.async_set(
+        "sensor.test", 12, attributes=_attributes, timestamp=now.timestamp()
+    )
+    await hass.async_block_till_done()
+    expected = {
+        "sensor.test": [
+            {
+                "data": {"statistic_id": "sensor.test"},
+                "type": "state_class_removed",
+            }
+        ],
+    }
+    await assert_validation_result(hass, client, expected, {"state_class_removed"})
+
+    # Remove the statistics - empty response
+    get_instance(hass).async_clear_statistics(["sensor.test"])
+    await async_recorder_block_till_done(hass)
+    await assert_validation_result(hass, client, {}, {})
+
+
 @pytest.mark.parametrize(
     ("units", "attributes", "unit"),
     [
@@ -5371,3 +5430,51 @@ async def test_exclude_attributes(hass: HomeAssistant) -> None:
     assert len(states) == 1
     assert ATTR_OPTIONS not in states[0].attributes
     assert ATTR_FRIENDLY_NAME in states[0].attributes
+
+
+async def test_clean_up_repairs(
+    hass: HomeAssistant, hass_ws_client: WebSocketGenerator
+) -> None:
+    """Test cleaning up repairs."""
+    await async_setup_component(hass, "sensor", {})
+    issue_registry = ir.async_get(hass)
+    client = await hass_ws_client()
+
+    # Create some issues
+    def create_issue(domain: str, issue_id: str, data: dict | None) -> None:
+        ir.async_create_issue(
+            hass,
+            domain,
+            issue_id,
+            data=data,
+            is_fixable=False,
+            severity=ir.IssueSeverity.WARNING,
+            translation_key="",
+        )
+
+    create_issue("test", "test_issue", None)
+    create_issue(DOMAIN, "test_issue_1", None)
+    create_issue(DOMAIN, "test_issue_2", {"issue_type": "another_issue"})
+    create_issue(DOMAIN, "test_issue_3", {"issue_type": "state_class_removed"})
+    create_issue(DOMAIN, "test_issue_4", {"issue_type": "units_changed"})
+
+    # Check the issues
+    assert set(issue_registry.issues) == {
+        ("test", "test_issue"),
+        ("sensor", "test_issue_1"),
+        ("sensor", "test_issue_2"),
+        ("sensor", "test_issue_3"),
+        ("sensor", "test_issue_4"),
+    }
+
+    # Request update of issues
+    await client.send_json_auto_id({"type": "recorder/update_statistics_issues"})
+    response = await client.receive_json()
+    assert response["success"]
+
+    # Check the issues
+    assert set(issue_registry.issues) == {
+        ("test", "test_issue"),
+        ("sensor", "test_issue_1"),
+        ("sensor", "test_issue_2"),
+    }