Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Don't include appservice users when calculating push rules (#13332)
Browse files Browse the repository at this point in the history
This can cause a lot of extra load on servers with lots of appservice users. Introduced in #13078
  • Loading branch information
erikjohnston authored Jul 20, 2022
1 parent 6fccd72 commit b4ae3b0
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 0 deletions.
1 change: 1 addition & 0 deletions changelog.d/13332.bugfix
Original file line number Diff line number Diff line change
@@ -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.
7 changes: 7 additions & 0 deletions synapse/push/bulk_push_rule_evaluator.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,13 @@ async def _get_rules_for_event(

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:
Expand Down
85 changes: 85 additions & 0 deletions tests/push/test_push_rule_evaluator.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,23 @@

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):
Expand Down Expand Up @@ -354,3 +364,78 @@ def test_relation_match(self) -> None:
"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)

0 comments on commit b4ae3b0

Please sign in to comment.