summary refs log tree commit diff
diff options
context:
space:
mode:
-rw-r--r--changelog.d/13332.bugfix1
-rw-r--r--synapse/push/bulk_push_rule_evaluator.py7
-rw-r--r--tests/push/test_push_rule_evaluator.py85
3 files changed, 93 insertions, 0 deletions
diff --git a/changelog.d/13332.bugfix b/changelog.d/13332.bugfix
new file mode 100644
index 0000000000..826ed1788f
--- /dev/null
+++ b/changelog.d/13332.bugfix
@@ -0,0 +1 @@
+Fix a bug introduced in Synapse 1.63.0 where push actions were incorrectly calculated for appservice users. This caused performance issues on servers with large numbers of appservices.
diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py
index e581af9a9a..713dcf6950 100644
--- a/synapse/push/bulk_push_rule_evaluator.py
+++ b/synapse/push/bulk_push_rule_evaluator.py
@@ -131,6 +131,13 @@ class BulkPushRuleEvaluator:
 
         local_users = await self.store.get_local_users_in_room(event.room_id)
 
+        # Filter out appservice users.
+        local_users = [
+            u
+            for u in local_users
+            if not self.store.get_if_app_services_interested_in_user(u)
+        ]
+
         # if this event is an invite event, we may need to run rules for the user
         # who's been invited, otherwise they won't get told they've been invited
         if event.type == EventTypes.Member and event.membership == Membership.INVITE:
diff --git a/tests/push/test_push_rule_evaluator.py b/tests/push/test_push_rule_evaluator.py
index 9b623d0033..718f489577 100644
--- a/tests/push/test_push_rule_evaluator.py
+++ b/tests/push/test_push_rule_evaluator.py
@@ -16,13 +16,23 @@ from typing import Dict, Optional, Set, Tuple, Union
 
 import frozendict
 
+from twisted.test.proto_helpers import MemoryReactor
+
+import synapse.rest.admin
+from synapse.api.constants import EventTypes, Membership
 from synapse.api.room_versions import RoomVersions
+from synapse.appservice import ApplicationService
 from synapse.events import FrozenEvent
 from synapse.push import push_rule_evaluator
 from synapse.push.push_rule_evaluator import PushRuleEvaluatorForEvent
+from synapse.rest.client import login, register, room
+from synapse.server import HomeServer
+from synapse.storage.databases.main.appservice import _make_exclusive_regex
 from synapse.types import JsonDict
+from synapse.util import Clock
 
 from tests import unittest
+from tests.test_utils.event_injection import create_event, inject_member_event
 
 
 class PushRuleEvaluatorTestCase(unittest.TestCase):
@@ -354,3 +364,78 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
             "event_type": "*.reaction",
         }
         self.assertTrue(evaluator.matches(condition, "@user:test", "foo"))
+
+
+class TestBulkPushRuleEvaluator(unittest.HomeserverTestCase):
+    """Tests for the bulk push rule evaluator"""
+
+    servlets = [
+        synapse.rest.admin.register_servlets_for_client_rest_resource,
+        login.register_servlets,
+        register.register_servlets,
+        room.register_servlets,
+    ]
+
+    def prepare(self, reactor: MemoryReactor, clock: Clock, homeserver: HomeServer):
+        # Define an application service so that we can register appservice users
+        self._service_token = "some_token"
+        self._service = ApplicationService(
+            self._service_token,
+            "as1",
+            "@as.sender:test",
+            namespaces={
+                "users": [
+                    {"regex": "@_as_.*:test", "exclusive": True},
+                    {"regex": "@as.sender:test", "exclusive": True},
+                ]
+            },
+            msc3202_transaction_extensions=True,
+        )
+        self.hs.get_datastores().main.services_cache = [self._service]
+        self.hs.get_datastores().main.exclusive_user_regex = _make_exclusive_regex(
+            [self._service]
+        )
+
+        self._as_user, _ = self.register_appservice_user(
+            "_as_user", self._service_token
+        )
+
+        self.evaluator = self.hs.get_bulk_push_rule_evaluator()
+
+    def test_ignore_appservice_users(self) -> None:
+        "Test that we don't generate push for appservice users"
+
+        user_id = self.register_user("user", "pass")
+        token = self.login("user", "pass")
+
+        room_id = self.helper.create_room_as(user_id, tok=token)
+        self.get_success(
+            inject_member_event(self.hs, room_id, self._as_user, Membership.JOIN)
+        )
+
+        event, context = self.get_success(
+            create_event(
+                self.hs,
+                type=EventTypes.Message,
+                room_id=room_id,
+                sender=user_id,
+                content={"body": "test", "msgtype": "m.text"},
+            )
+        )
+
+        # Assert the returned push rules do not contain the app service user
+        rules = self.get_success(self.evaluator._get_rules_for_event(event))
+        self.assertTrue(self._as_user not in rules)
+
+        # Assert that no push actions have been added to the staging table (the
+        # sender should not be pushed for the event)
+        users_with_push_actions = self.get_success(
+            self.hs.get_datastores().main.db_pool.simple_select_onecol(
+                table="event_push_actions_staging",
+                keyvalues={"event_id": event.event_id},
+                retcol="user_id",
+                desc="test_ignore_appservice_users",
+            )
+        )
+
+        self.assertEqual(len(users_with_push_actions), 0)