Compare commits

...

16 Commits

Author SHA1 Message Date
Eric Eastwood
8cad7023a9 Typo "uninteresting"
Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
2022-10-03 16:25:16 -05:00
Eric Eastwood
ad6fedd5d4 Fix comment 2022-09-30 14:21:39 -05:00
Eric Eastwood
0214b5d54e Parameterize same tests 2022-09-30 14:20:41 -05:00
Eric Eastwood
e2d1b48f38 Add changelog 2022-09-30 14:06:22 -05:00
Eric Eastwood
d92fd2aeac Revert other local change that we can't do 2022-09-30 13:58:17 -05:00
Eric Eastwood
7bd38034f9 Add test descriptions 2022-09-30 13:55:02 -05:00
Eric Eastwood
1218f03a8f Revert mock 2022-09-30 13:52:36 -05:00
Eric Eastwood
4451998d38 Clarify interested/control and lints 2022-09-30 13:51:06 -05:00
Eric Eastwood
76435c7915 Add actual homeserver tests for local/remote interesting to appservice users
See https://github.com/matrix-org/synapse/pull/13958#discussion_r984470972
2022-09-30 13:46:25 -05:00
Eric Eastwood
5d3c6a31dd Wrapping happens at 88 chars
See https://github.com/matrix-org/synapse/pull/13958#discussion_r984106210
2022-09-30 12:08:48 -05:00
Eric Eastwood
92b9da2f17 Fix tests 2022-09-29 19:22:47 -05:00
Eric Eastwood
72985dffdb Rename variables to 'local' to be obvious our intention 2022-09-29 18:59:41 -05:00
Eric Eastwood
a8be41bb80 Revert back to using our own _matches_user_in_member_list thing
See https://github.com/matrix-org/synapse/pull/13958#discussion_r984074247
2022-09-29 18:57:07 -05:00
Eric Eastwood
5f0f81591b Add changelog 2022-09-29 18:52:23 -05:00
Eric Eastwood
806b25512e Remove non-appservice usages split out to other PRs 2022-09-29 18:49:19 -05:00
Eric Eastwood
99a623d209 Check appservice user interest against the local users instead of all users
`get_local_users_in_room` is way more performant since it looks at a single
table (`local_current_membership`) and is looking through way less
data since it only worries about the local users in the room instead
of everyone in the room across the federation.

Fix https://github.com/matrix-org/synapse/issues/13942

Related to:

 - https://github.com/matrix-org/synapse/pull/13605
 - https://github.com/matrix-org/synapse/pull/13608
 - https://github.com/matrix-org/synapse/pull/13606
2022-09-29 17:24:26 -05:00
5 changed files with 104 additions and 8 deletions

1
changelog.d/14000.misc Normal file
View File

@@ -0,0 +1 @@
Add tests and clarify that an application service can be interested in local and remote users.

View File

@@ -172,6 +172,8 @@ class ApplicationService:
Returns:
True if this service would like to know about this room.
"""
# We need to get all users (local and remote) as an application service can be
# interested in anyone.
member_list = await store.get_users_in_room(
room_id, on_invalidate=cache_context.invalidate
)
@@ -189,9 +191,9 @@ class ApplicationService:
"""
Returns whether the application is interested in a given user ID.
The appservice is considered to be interested in a user if either: the
user ID is in the appservice's user namespace, or if the user is the
appservice's configured sender_localpart.
The appservice is considered to be interested in a user if either: the user ID
is in the appservice's user namespace, or if the user is the appservice's
configured sender_localpart. The user can be local or remote.
Args:
user_id: The ID of the user to check.

View File

@@ -157,6 +157,18 @@ class ApplicationServiceWorkerStore(RoomMemberWorkerStore):
app_service: "ApplicationService",
cache_context: _CacheContext,
) -> List[str]:
"""
Get all users in a room that the appservice is interested in.
Args:
room_id: The room to check in.
app_service: The application service to check interest against
Returns:
List of user IDs that the appservice is interested in.
"""
# We need to get all users (local and remote) as an application service can be
# interested in anyone.
users_in_room = await self.get_users_in_room(
room_id, on_invalidate=cache_context.invalidate
)

View File

@@ -159,6 +159,9 @@ class RoomMemberWorkerStore(EventsWorkerStore):
the forward extremities of those rooms will exclude most members. We may also
calculate room state incorrectly for such rooms and believe that a member is or
is not in the room when the opposite is true.
Note: If you only care about users in the room local to the homeserver, use
`get_local_users_in_room(...)` instead which will be more performant.
"""
return await self.db_pool.runInteraction(
"get_users_in_room", self.get_users_in_room_txn, room_id

View File

@@ -22,7 +22,7 @@ from twisted.test.proto_helpers import MemoryReactor
import synapse.rest.admin
import synapse.storage
from synapse.api.constants import EduTypes
from synapse.api.constants import EduTypes, EventTypes
from synapse.appservice import (
ApplicationService,
TransactionOneTimeKeyCounts,
@@ -36,7 +36,7 @@ from synapse.util import Clock
from synapse.util.stringutils import random_string
from tests import unittest
from tests.test_utils import make_awaitable, simple_async_mock
from tests.test_utils import event_injection, make_awaitable, simple_async_mock
from tests.unittest import override_config
from tests.utils import MockClock
@@ -386,15 +386,16 @@ class ApplicationServicesHandlerSendEventsTestCase(unittest.HomeserverTestCase):
receipts.register_servlets,
]
def prepare(self, reactor, clock, hs):
def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer):
self.hs = hs
# Mock the ApplicationServiceScheduler's _TransactionController's send method so that
# we can track any outgoing ephemeral events
self.send_mock = simple_async_mock()
hs.get_application_service_handler().scheduler.txn_ctrl.send = self.send_mock
hs.get_application_service_handler().scheduler.txn_ctrl.send = self.send_mock # type: ignore[assignment]
# Mock out application services, and allow defining our own in tests
self._services: List[ApplicationService] = []
self.hs.get_datastores().main.get_app_services = Mock(
self.hs.get_datastores().main.get_app_services = Mock( # type: ignore[assignment]
return_value=self._services
)
@@ -412,6 +413,83 @@ class ApplicationServicesHandlerSendEventsTestCase(unittest.HomeserverTestCase):
"exclusive_as_user", "password", self.exclusive_as_user_device_id
)
def _notify_interested_services(self):
# This is normally set in `notify_interested_services` but we need to call the
# internal async version so the reactor gets pushed to completion.
self.hs.get_application_service_handler().current_max += 1
self.get_success(
self.hs.get_application_service_handler()._notify_interested_services(
RoomStreamToken(
None, self.hs.get_application_service_handler().current_max
)
)
)
@parameterized.expand(["@local_as_user:test", "@interesting_user:remote"])
def test_match_interesting_room_members(self, interesting_user):
"""
Test to make sure that a interesting user (local or remote) in the room is notified
when someone else in the room sends a message.
"""
# Register an application service that's interested in the `interesting_user`
interested_appservice = self._register_application_service(
namespaces={
ApplicationService.NS_USERS: [
{
"regex": interesting_user,
"exclusive": False,
},
],
},
)
alice = self.register_user("alice", "pass")
alice_access_token = self.login("alice", "pass")
room_id = self.helper.create_room_as(room_creator=alice, tok=alice_access_token)
# Join the interesting user to the room
self.get_success(
event_injection.inject_member_event(
self.hs, room_id, interesting_user, "join"
)
)
# Kick the appservice into checking this membership event to get it out of the
# way
self._notify_interested_services()
# We don't care about the interesting user join event (this test is making sure
# the next thing works)
self.send_mock.reset_mock()
# Send a message from an uninteresting user
self.helper.send_event(
room_id,
type=EventTypes.Message,
content={
"msgtype": "m.text",
"body": "message from uninteresting user",
},
tok=alice_access_token,
)
# Kick the appservice into checking this new event
self._notify_interested_services()
self.send_mock.assert_called_once()
(
service,
events,
_ephemeral,
_to_device_messages,
_otks,
_fbks,
_device_list_summary,
) = self.send_mock.call_args[0]
# Even though the message came from an uninteresting user, it should still
# notify us because the interesting user is joined to the room.
self.assertEqual(service, interested_appservice)
self.assertEqual(events[0]["type"], "m.room.message")
self.assertEqual(events[0]["sender"], alice)
def test_sending_read_receipt_batches_to_application_services(self):
"""Tests that a large batch of read receipts are sent correctly to
interested application services.