From a4c75918b3e9cf48fa2bb91e9861f5f6fd74bd2e Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 11 May 2022 07:15:21 -0400 Subject: [PATCH] Remove unneeded `ActionGenerator` class. (#12691) It simply passes through to `BulkPushRuleEvaluator`, which can be called directly instead. --- changelog.d/12691.misc | 1 + synapse/handlers/federation_event.py | 4 +- synapse/handlers/message.py | 6 ++- synapse/push/__init__.py | 5 --- synapse/push/action_generator.py | 48 ------------------------ synapse/push/bulk_push_rule_evaluator.py | 7 ++++ synapse/server.py | 6 +-- 7 files changed, 17 insertions(+), 60 deletions(-) create mode 100644 changelog.d/12691.misc delete mode 100644 synapse/push/action_generator.py diff --git a/changelog.d/12691.misc b/changelog.d/12691.misc new file mode 100644 index 000000000..c63543421 --- /dev/null +++ b/changelog.d/12691.misc @@ -0,0 +1 @@ +Remove an unneeded class in the push code. diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index 6d11b32b6..761caa04b 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -103,7 +103,7 @@ class FederationEventHandler: self._event_creation_handler = hs.get_event_creation_handler() self._event_auth_handler = hs.get_event_auth_handler() self._message_handler = hs.get_message_handler() - self._action_generator = hs.get_action_generator() + self._bulk_push_rule_evaluator = hs.get_bulk_push_rule_evaluator() self._state_resolution_handler = hs.get_state_resolution_handler() # avoid a circular dependency by deferring execution here self._get_room_member_handler = hs.get_room_member_handler @@ -1913,7 +1913,7 @@ class FederationEventHandler: min_depth, ) else: - await self._action_generator.handle_push_actions_for_event( + await self._bulk_push_rule_evaluator.action_for_event_by_user( event, context ) diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index e47799e7f..4a4b535ba 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -426,7 +426,7 @@ class EventCreationHandler: # This is to stop us from diverging history *too* much. self.limiter = Linearizer(max_count=5, name="room_event_creation_limit") - self.action_generator = hs.get_action_generator() + self._bulk_push_rule_evaluator = hs.get_bulk_push_rule_evaluator() self.spam_checker = hs.get_spam_checker() self.third_party_event_rules: "ThirdPartyEventRules" = ( @@ -1249,7 +1249,9 @@ class EventCreationHandler: # and `state_groups` because they have `prev_events` that aren't persisted yet # (historical messages persisted in reverse-chronological order). if not event.internal_metadata.is_historical(): - await self.action_generator.handle_push_actions_for_event(event, context) + await self._bulk_push_rule_evaluator.action_for_event_by_user( + event, context + ) try: # If we're a worker we need to hit out to the master. diff --git a/synapse/push/__init__.py b/synapse/push/__init__.py index d1dfb406d..57c4d7046 100644 --- a/synapse/push/__init__.py +++ b/synapse/push/__init__.py @@ -43,11 +43,6 @@ The general interaction of the classes are: +---------------------------------------------+ | v - +-----------------+ - | ActionGenerator | - +-----------------+ - | - v +-----------------------+ +---------------------------+ | BulkPushRuleEvaluator |---->| PushRuleEvaluatorForEvent | +-----------------------+ +---------------------------+ diff --git a/synapse/push/action_generator.py b/synapse/push/action_generator.py deleted file mode 100644 index 730d9cd35..000000000 --- a/synapse/push/action_generator.py +++ /dev/null @@ -1,48 +0,0 @@ -# Copyright 2015 OpenMarket Ltd -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import logging -from typing import TYPE_CHECKING - -from synapse.events import EventBase -from synapse.events.snapshot import EventContext -from synapse.push.bulk_push_rule_evaluator import BulkPushRuleEvaluator -from synapse.util.metrics import Measure - -if TYPE_CHECKING: - from synapse.server import HomeServer - -logger = logging.getLogger(__name__) - - -class ActionGenerator: - def __init__(self, hs: "HomeServer"): - self.clock = hs.get_clock() - self.bulk_evaluator = BulkPushRuleEvaluator(hs) - # really we want to get all user ids and all profile tags too, - # since we want the actions for each profile tag for every user and - # also actions for a client with no profile tag for each user. - # Currently the event stream doesn't support profile tags on an - # event stream, so we just run the rules for a client with no profile - # tag (ie. we just need all the users). - - async def handle_push_actions_for_event( - self, event: EventBase, context: EventContext - ) -> None: - if event.internal_metadata.is_outlier(): - # This can happen due to out of band memberships - return - - with Measure(self.clock, "action_for_event_by_user"): - await self.bulk_evaluator.action_for_event_by_user(event, context) diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index 85ddb56c6..0ffafc882 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -29,6 +29,7 @@ from synapse.util.async_helpers import Linearizer from synapse.util.caches import CacheMetric, register_cache from synapse.util.caches.descriptors import lru_cache from synapse.util.caches.lrucache import LruCache +from synapse.util.metrics import measure_func from .push_rule_evaluator import PushRuleEvaluatorForEvent @@ -105,6 +106,7 @@ class BulkPushRuleEvaluator: def __init__(self, hs: "HomeServer"): self.hs = hs self.store = hs.get_datastores().main + self.clock = hs.get_clock() self._event_auth_handler = hs.get_event_auth_handler() # Used by `RulesForRoom` to ensure only one thing mutates the cache at a @@ -185,6 +187,7 @@ class BulkPushRuleEvaluator: return pl_event.content if pl_event else {}, sender_level + @measure_func("action_for_event_by_user") async def action_for_event_by_user( self, event: EventBase, context: EventContext ) -> None: @@ -192,6 +195,10 @@ class BulkPushRuleEvaluator: should increment the unread count, and insert the results into the event_push_actions_staging table. """ + if event.internal_metadata.is_outlier(): + # This can happen due to out of band memberships + return + count_as_unread = _should_count_as_unread(event, context) rules_by_user = await self._get_rules_for_event(event, context) diff --git a/synapse/server.py b/synapse/server.py index d49c76518..7daa7b933 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -119,7 +119,7 @@ from synapse.http.client import InsecureInterceptableContextFactory, SimpleHttpC from synapse.http.matrixfederationclient import MatrixFederationHttpClient from synapse.module_api import ModuleApi from synapse.notifier import Notifier -from synapse.push.action_generator import ActionGenerator +from synapse.push.bulk_push_rule_evaluator import BulkPushRuleEvaluator from synapse.push.pusherpool import PusherPool from synapse.replication.tcp.client import ReplicationDataHandler from synapse.replication.tcp.external_cache import ExternalCache @@ -644,8 +644,8 @@ class HomeServer(metaclass=abc.ABCMeta): return ReplicationCommandHandler(self) @cache_in_self - def get_action_generator(self) -> ActionGenerator: - return ActionGenerator(self) + def get_bulk_push_rule_evaluator(self) -> BulkPushRuleEvaluator: + return BulkPushRuleEvaluator(self) @cache_in_self def get_user_directory_handler(self) -> UserDirectoryHandler: