diff --git a/changelog.d/14000.misc b/changelog.d/14000.misc new file mode 100644 index 000000000000..ac178dc6f922 --- /dev/null +++ b/changelog.d/14000.misc @@ -0,0 +1 @@ +Add tests and clarify that an application service can be interested in local and remote users. diff --git a/synapse/appservice/__init__.py b/synapse/appservice/__init__.py index 0dfa00df44c7..357ccf109099 100644 --- a/synapse/appservice/__init__.py +++ b/synapse/appservice/__init__.py @@ -172,6 +172,8 @@ async def _matches_user_in_member_list( 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 @@ def is_interested_in_user( """ 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. diff --git a/synapse/storage/databases/main/appservice.py b/synapse/storage/databases/main/appservice.py index 64b70a7b28ee..9b33cd1dad60 100644 --- a/synapse/storage/databases/main/appservice.py +++ b/synapse/storage/databases/main/appservice.py @@ -157,6 +157,18 @@ async def get_app_service_users_in_room( 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 ) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 982e1f08e30b..5230efffb143 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -159,6 +159,9 @@ async def get_users_in_room(self, room_id: str) -> List[str]: 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 diff --git a/tests/handlers/test_appservice.py b/tests/handlers/test_appservice.py index af24c4984da5..e3152b488d58 100644 --- a/tests/handlers/test_appservice.py +++ b/tests/handlers/test_appservice.py @@ -22,7 +22,7 @@ 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.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 @@ def prepare(self, reactor, clock, hs): "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.