Ensure logbook performs well when filtering is configured (#37292)

homeassistant/components/history/__init__.py

@@ -394,16 +394,9 @@ def get_state(hass, utc_point_in_time, entity_id, run=None):
 
 async def async_setup(hass, config):
     """Set up the history hooks."""
-    filters = Filters()
     conf = config.get(DOMAIN, {})
-    exclude = conf.get(CONF_EXCLUDE)
+
-    if exclude:
+    filters = sqlalchemy_filter_from_include_exclude_conf(conf)
-        filters.excluded_entities = exclude.get(CONF_ENTITIES, [])
-        filters.excluded_domains = exclude.get(CONF_DOMAINS, [])
-    include = conf.get(CONF_INCLUDE)
-    if include:
-        filters.included_entities = include.get(CONF_ENTITIES, [])
-        filters.included_domains = include.get(CONF_DOMAINS, [])
     use_include_order = conf.get(CONF_ORDER)
 
     hass.http.register_view(HistoryPeriodView(filters, use_include_order))
@@ -530,6 +523,20 @@ class HistoryPeriodView(HomeAssistantView):
         return self.json(result)
 
 
+def sqlalchemy_filter_from_include_exclude_conf(conf):
+    """Build a sql filter from config."""
+    filters = Filters()
+    exclude = conf.get(CONF_EXCLUDE)
+    if exclude:
+        filters.excluded_entities = exclude.get(CONF_ENTITIES, [])
+        filters.excluded_domains = exclude.get(CONF_DOMAINS, [])
+    include = conf.get(CONF_INCLUDE)
+    if include:
+        filters.included_entities = include.get(CONF_ENTITIES, [])
+        filters.included_domains = include.get(CONF_DOMAINS, [])
+    return filters
+
+
 class Filters:
     """Container for the configured include and exclude filters."""
 
@@ -556,26 +563,34 @@ class Filters:
             return query.filter(States.entity_id.in_(entity_ids))
         query = query.filter(~States.domain.in_(IGNORE_DOMAINS))
 
-        filter_query = None
+        entity_filter = self.entity_filter()
+        if entity_filter is not None:
+            query = query.filter(entity_filter)
+
+        return query
+
+    def entity_filter(self):
+        """Generate the entity filter query."""
+        entity_filter = None
         # filter if only excluded domain is configured
         if self.excluded_domains and not self.included_domains:
-            filter_query = ~States.domain.in_(self.excluded_domains)
+            entity_filter = ~States.domain.in_(self.excluded_domains)
             if self.included_entities:
-                filter_query &= States.entity_id.in_(self.included_entities)
+                entity_filter &= States.entity_id.in_(self.included_entities)
         # filter if only included domain is configured
         elif not self.excluded_domains and self.included_domains:
-            filter_query = States.domain.in_(self.included_domains)
+            entity_filter = States.domain.in_(self.included_domains)
             if self.included_entities:
-                filter_query |= States.entity_id.in_(self.included_entities)
+                entity_filter |= States.entity_id.in_(self.included_entities)
         # filter if included and excluded domain is configured
         elif self.excluded_domains and self.included_domains:
-            filter_query = ~States.domain.in_(self.excluded_domains)
+            entity_filter = ~States.domain.in_(self.excluded_domains)
             if self.included_entities:
-                filter_query &= States.domain.in_(
+                entity_filter &= States.domain.in_(
                     self.included_domains
                 ) | States.entity_id.in_(self.included_entities)
             else:
-                filter_query &= States.domain.in_(
+                entity_filter &= States.domain.in_(
                     self.included_domains
                 ) & ~States.domain.in_(self.excluded_domains)
         # no domain filter just included entities
@@ -584,13 +599,17 @@ class Filters:
             and not self.included_domains
             and self.included_entities
         ):
-            filter_query = States.entity_id.in_(self.included_entities)
+            entity_filter = States.entity_id.in_(self.included_entities)
-        if filter_query is not None:
-            query = query.filter(filter_query)
         # finally apply excluded entities filter if configured
         if self.excluded_entities:
-            query = query.filter(~States.entity_id.in_(self.excluded_entities))
+            if entity_filter is not None:
-        return query
+                entity_filter = (entity_filter) & ~States.entity_id.in_(
+                    self.excluded_entities
+                )
+            else:
+                entity_filter = ~States.entity_id.in_(self.excluded_entities)
+
+        return entity_filter
 
 
 class LazyState(State):

homeassistant/components/logbook/__init__.py

@@ -3,14 +3,13 @@ from datetime import timedelta
 from itertools import groupby
 import json
 import logging
-import time
 
 import sqlalchemy
-from sqlalchemy.exc import SQLAlchemyError
 from sqlalchemy.orm import aliased
 import voluptuous as vol
 
 from homeassistant.components import sun
+from homeassistant.components.history import sqlalchemy_filter_from_include_exclude_conf
 from homeassistant.components.http import HomeAssistantView
 from homeassistant.components.recorder.models import (
     Events,
@@ -18,19 +17,13 @@ from homeassistant.components.recorder.models import (
     process_timestamp,
     process_timestamp_to_utc_isoformat,
 )
-from homeassistant.components.recorder.util import (
+from homeassistant.components.recorder.util import session_scope
-    QUERY_RETRY_WAIT,
-    RETRIES,
-    session_scope,
-)
 from homeassistant.const import (
     ATTR_DEVICE_CLASS,
     ATTR_DOMAIN,
     ATTR_ENTITY_ID,
     ATTR_FRIENDLY_NAME,
     ATTR_NAME,
-    CONF_EXCLUDE,
-    CONF_INCLUDE,
     EVENT_HOMEASSISTANT_START,
     EVENT_HOMEASSISTANT_STOP,
     EVENT_LOGBOOK_ENTRY,
@@ -123,12 +116,21 @@ async def async_setup(hass, config):
         message = message.async_render()
         async_log_entry(hass, name, message, domain, entity_id)
 
-    hass.http.register_view(LogbookView(config.get(DOMAIN, {})))
-
     hass.components.frontend.async_register_built_in_panel(
         "logbook", "logbook", "hass:format-list-bulleted-type"
     )
 
+    conf = config.get(DOMAIN, {})
+
+    if conf:
+        filters = sqlalchemy_filter_from_include_exclude_conf(conf)
+        entities_filter = convert_include_exclude_filter(conf)
+    else:
+        filters = None
+        entities_filter = None
+
+    hass.http.register_view(LogbookView(conf, filters, entities_filter))
+
     hass.services.async_register(DOMAIN, "log", log_message, schema=LOG_MESSAGE_SCHEMA)
 
     await async_process_integration_platforms(hass, DOMAIN, _process_logbook_platform)
@@ -154,9 +156,11 @@ class LogbookView(HomeAssistantView):
     name = "api:logbook"
     extra_urls = ["/api/logbook/{datetime}"]
 
-    def __init__(self, config):
+    def __init__(self, config, filters, entities_filter):
         """Initialize the logbook view."""
         self.config = config
+        self.filters = filters
+        self.entities_filter = entities_filter
 
     async def get(self, request, datetime=None):
         """Retrieve logbook entries."""
@@ -191,7 +195,15 @@ class LogbookView(HomeAssistantView):
         def json_events():
             """Fetch events and generate JSON."""
             return self.json(
-                _get_events(hass, self.config, start_day, end_day, entity_id)
+                _get_events(
+                    hass,
+                    self.config,
+                    start_day,
+                    end_day,
+                    entity_id,
+                    self.filters,
+                    self.entities_filter,
+                )
             )
 
         return await hass.async_add_job(json_events)
@@ -327,38 +339,9 @@ def humanify(hass, events, entity_attr_cache, prev_states=None):
                 }
 
 
-def _get_related_entity_ids(session, entity_filter):
+def _get_events(
-    timer_start = time.perf_counter()
+    hass, config, start_day, end_day, entity_id=None, filters=None, entities_filter=None
-
+):
-    query = session.query(States).with_entities(States.entity_id).distinct()
-
-    for tryno in range(RETRIES):
-        try:
-            result = [row.entity_id for row in query if entity_filter(row.entity_id)]
-
-            if _LOGGER.isEnabledFor(logging.DEBUG):
-                elapsed = time.perf_counter() - timer_start
-                _LOGGER.debug(
-                    "fetching %d distinct domain/entity_id pairs took %fs",
-                    len(result),
-                    elapsed,
-                )
-
-            return result
-        except SQLAlchemyError as err:
-            _LOGGER.error("Error executing query: %s", err)
-
-            if tryno == RETRIES - 1:
-                raise
-            time.sleep(QUERY_RETRY_WAIT)
-
-
-def _all_entities_filter(_):
-    """Filter that accepts all entities."""
-    return True
-
-
-def _get_events(hass, config, start_day, end_day, entity_id=None):
     """Get events for a period of time."""
     entity_attr_cache = EntityAttributeCache(hass)
 
@@ -373,12 +356,10 @@ def _get_events(hass, config, start_day, end_day, entity_id=None):
         if entity_id is not None:
             entity_ids = [entity_id.lower()]
             entities_filter = generate_filter([], entity_ids, [], [])
-        elif config.get(CONF_EXCLUDE) or config.get(CONF_INCLUDE):
+            apply_sql_entities_filter = False
-            entities_filter = convert_include_exclude_filter(config)
-            entity_ids = _get_related_entity_ids(session, entities_filter)
         else:
-            entities_filter = _all_entities_filter
             entity_ids = None
+            apply_sql_entities_filter = True
 
         old_state = aliased(States, name="old_state")
 
@@ -445,6 +426,13 @@ def _get_events(hass, config, start_day, end_day, entity_id=None):
                 | (States.state_id.is_(None))
             )
 
+        if apply_sql_entities_filter and filters:
+            entity_filter = filters.entity_filter()
+            if entity_filter is not None:
+                query = query.filter(
+                    entity_filter | (Events.event_type != EVENT_STATE_CHANGED)
+                )
+
         # When all data is schema v8 or later, prev_states can be removed
         prev_states = {}
         return list(humanify(hass, yield_events(query), entity_attr_cache, prev_states))
@@ -478,7 +466,7 @@ def _keep_event(hass, event, entities_filter, entity_attr_cache):
                 return False
             entity_id = f"{domain}."
 
-    return entities_filter(entity_id)
+    return entities_filter is None or entities_filter(entity_id)
 
 
 def _entry_message_from_event(hass, entity_id, domain, event, entity_attr_cache):

tests/components/logbook/test_init.py

@@ -20,6 +20,8 @@ from homeassistant.const import (
     ATTR_NAME,
     CONF_DOMAINS,
     CONF_ENTITIES,
+    CONF_EXCLUDE,
+    CONF_INCLUDE,
     EVENT_HOMEASSISTANT_START,
     EVENT_HOMEASSISTANT_STOP,
     EVENT_STATE_CHANGED,
@@ -240,7 +242,7 @@ class TestComponentLogbook(unittest.TestCase):
         config = logbook.CONFIG_SCHEMA(
             {
                 ha.DOMAIN: {},
-                logbook.DOMAIN: {logbook.CONF_EXCLUDE: {CONF_ENTITIES: [entity_id]}},
+                logbook.DOMAIN: {CONF_EXCLUDE: {CONF_ENTITIES: [entity_id]}},
             }
         )
         entities_filter = convert_include_exclude_filter(config[logbook.DOMAIN])
@@ -277,9 +279,7 @@ class TestComponentLogbook(unittest.TestCase):
         config = logbook.CONFIG_SCHEMA(
             {
                 ha.DOMAIN: {},
-                logbook.DOMAIN: {
+                logbook.DOMAIN: {CONF_EXCLUDE: {CONF_DOMAINS: ["switch", "alexa"]}},
-                    logbook.CONF_EXCLUDE: {CONF_DOMAINS: ["switch", "alexa"]}
-                },
             }
         )
         entities_filter = convert_include_exclude_filter(config[logbook.DOMAIN])
@@ -321,7 +321,7 @@ class TestComponentLogbook(unittest.TestCase):
             {
                 ha.DOMAIN: {},
                 logbook.DOMAIN: {
-                    logbook.CONF_EXCLUDE: {
+                    CONF_EXCLUDE: {
                         CONF_DOMAINS: ["switch", "alexa"],
                         CONF_ENTITY_GLOBS: "*.excluded",
                     }
@@ -365,7 +365,7 @@ class TestComponentLogbook(unittest.TestCase):
             {
                 ha.DOMAIN: {},
                 logbook.DOMAIN: {
-                    logbook.CONF_INCLUDE: {
+                    CONF_INCLUDE: {
                         CONF_DOMAINS: ["homeassistant"],
                         CONF_ENTITIES: [entity_id2],
                     }
@@ -413,9 +413,7 @@ class TestComponentLogbook(unittest.TestCase):
             {
                 ha.DOMAIN: {},
                 logbook.DOMAIN: {
-                    logbook.CONF_INCLUDE: {
+                    CONF_INCLUDE: {CONF_DOMAINS: ["homeassistant", "sensor", "alexa"]}
-                        CONF_DOMAINS: ["homeassistant", "sensor", "alexa"]
-                    }
                 },
             }
         )
@@ -465,7 +463,7 @@ class TestComponentLogbook(unittest.TestCase):
             {
                 ha.DOMAIN: {},
                 logbook.DOMAIN: {
-                    logbook.CONF_INCLUDE: {
+                    CONF_INCLUDE: {
                         CONF_DOMAINS: ["homeassistant", "sensor", "alexa"],
                         CONF_ENTITY_GLOBS: ["*.included"],
                     }
@@ -517,11 +515,11 @@ class TestComponentLogbook(unittest.TestCase):
             {
                 ha.DOMAIN: {},
                 logbook.DOMAIN: {
-                    logbook.CONF_INCLUDE: {
+                    CONF_INCLUDE: {
                         CONF_DOMAINS: ["sensor", "homeassistant"],
                         CONF_ENTITIES: ["switch.bla"],
                     },
-                    logbook.CONF_EXCLUDE: {
+                    CONF_EXCLUDE: {
                         CONF_DOMAINS: ["switch"],
                         CONF_ENTITIES: ["sensor.bli"],
                     },
@@ -586,12 +584,12 @@ class TestComponentLogbook(unittest.TestCase):
             {
                 ha.DOMAIN: {},
                 logbook.DOMAIN: {
-                    logbook.CONF_INCLUDE: {
+                    CONF_INCLUDE: {
                         CONF_DOMAINS: ["sensor", "homeassistant"],
                         CONF_ENTITIES: ["switch.bla"],
                         CONF_ENTITY_GLOBS: ["*.included"],
                     },
-                    logbook.CONF_EXCLUDE: {
+                    CONF_EXCLUDE: {
                         CONF_DOMAINS: ["switch"],
                         CONF_ENTITY_GLOBS: ["*.excluded"],
                         CONF_ENTITIES: ["sensor.bli"],
@@ -1617,10 +1615,7 @@ async def test_exclude_described_event(hass, hass_client):
         logbook.DOMAIN,
         {
             logbook.DOMAIN: {
-                logbook.CONF_EXCLUDE: {
+                CONF_EXCLUDE: {CONF_DOMAINS: ["sensor"], CONF_ENTITIES: [entity_id]}
-                    CONF_DOMAINS: ["sensor"],
-                    CONF_ENTITIES: [entity_id],
-                }
             }
         },
     )