From 953db2464908c2f260a31451b1d1414b53c03cd5 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 9 May 2022 09:28:04 -0500 Subject: [PATCH 1/6] cleanups --- homeassistant/components/logbook/__init__.py | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/homeassistant/components/logbook/__init__.py b/homeassistant/components/logbook/__init__.py index 5ec03b3aaa4b9c..7ddfd3a0d21188 100644 --- a/homeassistant/components/logbook/__init__.py +++ b/homeassistant/components/logbook/__init__.py @@ -391,15 +391,10 @@ def humanify( "state": row.state, "entity_id": entity_id, } - if icon := _row_attributes_extract(row, ICON_JSON_EXTRACT): data["icon"] = icon - if row.context_user_id: - data["context_user_id"] = row.context_user_id - context_augmenter.augment(data, entity_id, row) - yield data elif row.event_type in external_events: @@ -407,9 +402,6 @@ def humanify( data = describe_event(event_cache.get(row)) data["when"] = _row_time_fired_isoformat(row) data["domain"] = domain - if row.context_user_id: - data["context_user_id"] = row.context_user_id - entity_id = data.get(ATTR_ENTITY_ID) context_augmenter.augment(data, entity_id, row) yield data @@ -417,6 +409,7 @@ def humanify( elif row.event_type == EVENT_HOMEASSISTANT_START: if start_stop_events.get(row.time_fired.minute) == 2: continue + yield { "when": _row_time_fired_isoformat(row), "name": "Home Assistant", @@ -453,10 +446,6 @@ def humanify( "domain": domain, "entity_id": entity_id, } - - if row.context_user_id: - data["context_user_id"] = row.context_user_id - context_augmenter.augment(data, entity_id, row) yield data @@ -746,6 +735,9 @@ def __init__( def augment(self, data: dict[str, Any], entity_id: str | None, row: Row) -> None: """Augment data from the row and cache.""" + if context_user_id := row.context_user_id: + data["context_user_id"] = context_user_id + if not (context_row := self.context_lookup.get(row.context_id)): return From 5220391206f45271f79c91dad71643c01ac9d4c0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 9 May 2022 09:32:58 -0500 Subject: [PATCH 2/6] cleanups --- homeassistant/components/logbook/__init__.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/homeassistant/components/logbook/__init__.py b/homeassistant/components/logbook/__init__.py index 7ddfd3a0d21188..b335f994abc5d3 100644 --- a/homeassistant/components/logbook/__init__.py +++ b/homeassistant/components/logbook/__init__.py @@ -402,8 +402,7 @@ def humanify( data = describe_event(event_cache.get(row)) data["when"] = _row_time_fired_isoformat(row) data["domain"] = domain - entity_id = data.get(ATTR_ENTITY_ID) - context_augmenter.augment(data, entity_id, row) + context_augmenter.augment(data, data.get(ATTR_ENTITY_ID), row) yield data elif row.event_type == EVENT_HOMEASSISTANT_START: From af5e4022ab55188f7fd0aa5585e988ab10580b01 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 9 May 2022 09:46:36 -0500 Subject: [PATCH 3/6] remove stop/start grouping --- homeassistant/components/logbook/__init__.py | 180 ++++++++----------- tests/components/logbook/test_init.py | 14 +- 2 files changed, 75 insertions(+), 119 deletions(-) diff --git a/homeassistant/components/logbook/__init__.py b/homeassistant/components/logbook/__init__.py index b335f994abc5d3..facfcf5ab1c30c 100644 --- a/homeassistant/components/logbook/__init__.py +++ b/homeassistant/components/logbook/__init__.py @@ -5,7 +5,6 @@ from contextlib import suppress from datetime import datetime as dt, timedelta from http import HTTPStatus -from itertools import groupby import json import re from typing import Any, cast @@ -338,115 +337,76 @@ def humanify( """ external_events = hass.data.get(DOMAIN, {}) # Continuous sensors, will be excluded from the logbook - continuous_sensors = {} - - # Group events in batches of GROUP_BY_MINUTES - for _, g_rows in groupby( - rows, lambda row: row.time_fired.minute // GROUP_BY_MINUTES # type: ignore[no-any-return] - ): - - rows_batch = list(g_rows) - - # Group HA start/stop events - # Maps minute of event to 1: stop, 2: stop + start - start_stop_events = {} - - # Process events - for row in rows_batch: - if row.event_type == EVENT_STATE_CHANGED: - entity_id = row.entity_id - if ( - entity_id in continuous_sensors - or split_entity_id(entity_id)[0] != SENSOR_DOMAIN - ): - continue - assert entity_id is not None - continuous_sensors[entity_id] = _is_sensor_continuous(hass, entity_id) - - elif row.event_type == EVENT_HOMEASSISTANT_STOP: - if row.time_fired.minute in start_stop_events: - continue - - start_stop_events[row.time_fired.minute] = 1 - - elif row.event_type == EVENT_HOMEASSISTANT_START: - if row.time_fired.minute not in start_stop_events: - continue - - start_stop_events[row.time_fired.minute] = 2 - - # Yield entries - for row in rows_batch: - if row.event_type == EVENT_STATE_CHANGED: - entity_id = row.entity_id - assert entity_id is not None - - if continuous_sensors.get(entity_id): - # Skip continuous sensors - continue - - data = { - "when": _row_time_fired_isoformat(row), - "name": entity_name_cache.get(entity_id, row), - "state": row.state, - "entity_id": entity_id, - } - if icon := _row_attributes_extract(row, ICON_JSON_EXTRACT): - data["icon"] = icon - - context_augmenter.augment(data, entity_id, row) - yield data - - elif row.event_type in external_events: - domain, describe_event = external_events[row.event_type] - data = describe_event(event_cache.get(row)) - data["when"] = _row_time_fired_isoformat(row) - data["domain"] = domain - context_augmenter.augment(data, data.get(ATTR_ENTITY_ID), row) - yield data - - elif row.event_type == EVENT_HOMEASSISTANT_START: - if start_stop_events.get(row.time_fired.minute) == 2: - continue - - yield { - "when": _row_time_fired_isoformat(row), - "name": "Home Assistant", - "message": "started", - "domain": HA_DOMAIN, - } - - elif row.event_type == EVENT_HOMEASSISTANT_STOP: - if start_stop_events.get(row.time_fired.minute) == 2: - action = "restarted" - else: - action = "stopped" - - yield { - "when": _row_time_fired_isoformat(row), - "name": "Home Assistant", - "message": action, - "domain": HA_DOMAIN, - } - - elif row.event_type == EVENT_LOGBOOK_ENTRY: - event = event_cache.get(row) - event_data = event.data - domain = event_data.get(ATTR_DOMAIN) - entity_id = event_data.get(ATTR_ENTITY_ID) - if domain is None and entity_id is not None: - with suppress(IndexError): - domain = split_entity_id(str(entity_id))[0] - - data = { - "when": _row_time_fired_isoformat(row), - "name": event_data.get(ATTR_NAME), - "message": event_data.get(ATTR_MESSAGE), - "domain": domain, - "entity_id": entity_id, - } - context_augmenter.augment(data, entity_id, row) - yield data + continuous_sensors: dict[str, bool] = {} + + # Process events + for row in rows: + event_type = row.event_type + if event_type == EVENT_STATE_CHANGED: + entity_id = row.entity_id + assert entity_id is not None + # Skip continuous sensors + if ( + is_continuous := continuous_sensors.get(entity_id) + ) is None and split_entity_id(entity_id)[0] == SENSOR_DOMAIN: + is_continuous = _is_sensor_continuous(hass, entity_id) + continuous_sensors[entity_id] = is_continuous + if is_continuous: + continue + + data = { + "when": _row_time_fired_isoformat(row), + "name": entity_name_cache.get(entity_id, row), + "state": row.state, + "entity_id": entity_id, + } + if icon := _row_attributes_extract(row, ICON_JSON_EXTRACT): + data["icon"] = icon + + context_augmenter.augment(data, entity_id, row) + yield data + + elif event_type in external_events: + domain, describe_event = external_events[row.event_type] + data = describe_event(event_cache.get(row)) + data["when"] = _row_time_fired_isoformat(row) + data["domain"] = domain + context_augmenter.augment(data, data.get(ATTR_ENTITY_ID), row) + yield data + + elif event_type == EVENT_HOMEASSISTANT_START: + yield { + "when": _row_time_fired_isoformat(row), + "name": "Home Assistant", + "message": "started", + "domain": HA_DOMAIN, + } + elif event_type == EVENT_HOMEASSISTANT_STOP: + yield { + "when": _row_time_fired_isoformat(row), + "name": "Home Assistant", + "message": "stopped", + "domain": HA_DOMAIN, + } + + elif event_type == EVENT_LOGBOOK_ENTRY: + event = event_cache.get(row) + event_data = event.data + domain = event_data.get(ATTR_DOMAIN) + entity_id = event_data.get(ATTR_ENTITY_ID) + if domain is None and entity_id is not None: + with suppress(IndexError): + domain = split_entity_id(str(entity_id))[0] + + data = { + "when": _row_time_fired_isoformat(row), + "name": event_data.get(ATTR_NAME), + "message": event_data.get(ATTR_MESSAGE), + "domain": domain, + "entity_id": entity_id, + } + context_augmenter.augment(data, entity_id, row) + yield data def _get_events( diff --git a/tests/components/logbook/test_init.py b/tests/components/logbook/test_init.py index ad5423286d9b31..e55fca1133bd3a 100644 --- a/tests/components/logbook/test_init.py +++ b/tests/components/logbook/test_init.py @@ -210,11 +210,8 @@ async def test_filter_sensor(hass_: ha.HomeAssistant, hass_client): _assert_entry(entries[2], name="ble", entity_id=entity_id4, state="10") -def test_home_assistant_start_stop_grouped(hass_): - """Test if HA start and stop events are grouped. - - Events that are occurring in the same minute. - """ +def test_home_assistant_start_stop_not_grouped(hass_): + """Test if HA start and stop events are no longer grouped.""" entries = mock_humanify( hass_, ( @@ -223,10 +220,9 @@ def test_home_assistant_start_stop_grouped(hass_): ), ) - assert len(entries) == 1 - assert_entry( - entries[0], name="Home Assistant", message="restarted", domain=ha.DOMAIN - ) + assert len(entries) == 2 + assert_entry(entries[0], name="Home Assistant", message="stopped", domain=ha.DOMAIN) + assert_entry(entries[1], name="Home Assistant", message="started", domain=ha.DOMAIN) def test_home_assistant_start(hass_): From d385c4ec8e8fb93118bd78841e4b63df124605ae Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 9 May 2022 09:46:51 -0500 Subject: [PATCH 4/6] remove stop/start grouping --- homeassistant/components/logbook/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/logbook/__init__.py b/homeassistant/components/logbook/__init__.py index facfcf5ab1c30c..f4f420a5e8a311 100644 --- a/homeassistant/components/logbook/__init__.py +++ b/homeassistant/components/logbook/__init__.py @@ -367,7 +367,7 @@ def humanify( yield data elif event_type in external_events: - domain, describe_event = external_events[row.event_type] + domain, describe_event = external_events[event_type] data = describe_event(event_cache.get(row)) data["when"] = _row_time_fired_isoformat(row) data["domain"] = domain From 377b9f2a3d34ffd5c7cc9f31b4acb82aab2eab5b Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 9 May 2022 09:55:14 -0500 Subject: [PATCH 5/6] sped up row matching --- homeassistant/components/logbook/__init__.py | 29 ++++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/homeassistant/components/logbook/__init__.py b/homeassistant/components/logbook/__init__.py index f4f420a5e8a311..8f34402275e5c2 100644 --- a/homeassistant/components/logbook/__init__.py +++ b/homeassistant/components/logbook/__init__.py @@ -114,6 +114,7 @@ EVENT_COLUMNS = [ + Events.event_id.label("event_id"), Events.event_type.label("event_type"), Events.event_data.label("event_data"), Events.time_fired.label("time_fired"), @@ -123,6 +124,7 @@ ] STATE_COLUMNS = [ + States.state_id.label("state_id"), States.state.label("state"), States.entity_id.label("entity_id"), States.attributes.label("attributes"), @@ -130,6 +132,7 @@ ] EMPTY_STATE_COLUMNS = [ + literal(value=None, type_=sqlalchemy.Numeric).label("state_id"), literal(value=None, type_=sqlalchemy.String).label("state"), literal(value=None, type_=sqlalchemy.String).label("entity_id"), literal(value=None, type_=sqlalchemy.Text).label("attributes"), @@ -437,8 +440,9 @@ def yield_rows(query: Query) -> Generator[Row, None, None]: """Yield Events that are not filtered away.""" for row in query.yield_per(1000): context_lookup.setdefault(row.context_id, row) - if row.event_type != EVENT_CALL_SERVICE and ( - row.event_type == EVENT_STATE_CHANGED + event_type = row.event_type + if event_type != EVENT_CALL_SERVICE and ( + event_type == EVENT_STATE_CHANGED or _keep_row(hass, row, entities_filter) ): yield row @@ -508,6 +512,7 @@ def yield_rows(query: Query) -> Generator[Row, None, None]: def _generate_events_query_without_data(session: Session) -> Query: return session.query( + literal(value=None, type_=sqlalchemy.Numeric).label("event_id"), literal(value=EVENT_STATE_CHANGED, type_=sqlalchemy.String).label("event_type"), literal(value=None, type_=sqlalchemy.Text).label("event_data"), States.last_changed.label("time_fired"), @@ -530,10 +535,7 @@ def _generate_legacy_events_context_id_query( legacy_context_id_query = session.query( *EVENT_COLUMNS, literal(value=None, type_=sqlalchemy.String).label("shared_data"), - States.state, - States.entity_id, - States.attributes, - StateAttributes.shared_attrs, + *STATE_COLUMNS, ) legacy_context_id_query = _apply_event_time_filter( legacy_context_id_query, start_day, end_day @@ -777,12 +779,15 @@ def _is_sensor_continuous( def _rows_match(row: Row, other_row: Row) -> bool: - """Check of rows match by using the same method as Events __hash__.""" - return bool( - row.event_type == other_row.event_type - and row.context_id == other_row.context_id - and row.time_fired == other_row.time_fired - ) + """Match on the database unique id.""" + if ( + (state_id := row.state_id) is not None + and state_id == other_row.state_id + or (event_id := row.event_id) is not None + and event_id == other_row.event_id + ): + return True + return False def _row_event_data_extract(row: Row, extractor: re.Pattern) -> str | None: From 1b36926ab20decce74752cef9351b5e53f6f2223 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 9 May 2022 09:58:52 -0500 Subject: [PATCH 6/6] Revert "sped up row matching" This reverts commit 377b9f2a3d34ffd5c7cc9f31b4acb82aab2eab5b. --- homeassistant/components/logbook/__init__.py | 29 ++++++++------------ 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/homeassistant/components/logbook/__init__.py b/homeassistant/components/logbook/__init__.py index 8f34402275e5c2..f4f420a5e8a311 100644 --- a/homeassistant/components/logbook/__init__.py +++ b/homeassistant/components/logbook/__init__.py @@ -114,7 +114,6 @@ EVENT_COLUMNS = [ - Events.event_id.label("event_id"), Events.event_type.label("event_type"), Events.event_data.label("event_data"), Events.time_fired.label("time_fired"), @@ -124,7 +123,6 @@ ] STATE_COLUMNS = [ - States.state_id.label("state_id"), States.state.label("state"), States.entity_id.label("entity_id"), States.attributes.label("attributes"), @@ -132,7 +130,6 @@ ] EMPTY_STATE_COLUMNS = [ - literal(value=None, type_=sqlalchemy.Numeric).label("state_id"), literal(value=None, type_=sqlalchemy.String).label("state"), literal(value=None, type_=sqlalchemy.String).label("entity_id"), literal(value=None, type_=sqlalchemy.Text).label("attributes"), @@ -440,9 +437,8 @@ def yield_rows(query: Query) -> Generator[Row, None, None]: """Yield Events that are not filtered away.""" for row in query.yield_per(1000): context_lookup.setdefault(row.context_id, row) - event_type = row.event_type - if event_type != EVENT_CALL_SERVICE and ( - event_type == EVENT_STATE_CHANGED + if row.event_type != EVENT_CALL_SERVICE and ( + row.event_type == EVENT_STATE_CHANGED or _keep_row(hass, row, entities_filter) ): yield row @@ -512,7 +508,6 @@ def yield_rows(query: Query) -> Generator[Row, None, None]: def _generate_events_query_without_data(session: Session) -> Query: return session.query( - literal(value=None, type_=sqlalchemy.Numeric).label("event_id"), literal(value=EVENT_STATE_CHANGED, type_=sqlalchemy.String).label("event_type"), literal(value=None, type_=sqlalchemy.Text).label("event_data"), States.last_changed.label("time_fired"), @@ -535,7 +530,10 @@ def _generate_legacy_events_context_id_query( legacy_context_id_query = session.query( *EVENT_COLUMNS, literal(value=None, type_=sqlalchemy.String).label("shared_data"), - *STATE_COLUMNS, + States.state, + States.entity_id, + States.attributes, + StateAttributes.shared_attrs, ) legacy_context_id_query = _apply_event_time_filter( legacy_context_id_query, start_day, end_day @@ -779,15 +777,12 @@ def _is_sensor_continuous( def _rows_match(row: Row, other_row: Row) -> bool: - """Match on the database unique id.""" - if ( - (state_id := row.state_id) is not None - and state_id == other_row.state_id - or (event_id := row.event_id) is not None - and event_id == other_row.event_id - ): - return True - return False + """Check of rows match by using the same method as Events __hash__.""" + return bool( + row.event_type == other_row.event_type + and row.context_id == other_row.context_id + and row.time_fired == other_row.time_fired + ) def _row_event_data_extract(row: Row, extractor: re.Pattern) -> str | None: