Clarify that an appservice can be interested in local and remote users

changelog.d/14000.misc

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

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.

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
         )

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

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,146 @@ 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
+                )
+            )
+        )
+
+    def test_match_local_room_members(self):
+        """
+        Test to make sure that a user local to the server and in the room is notified
+        when someone else in the room sends a message.
+        """
+        # Register an application service that's interested in local and remote user
+        interested_appservice = self._register_application_service(
+            namespaces={
+                ApplicationService.NS_USERS: [
+                    {
+                        "regex": "@local_as_user:test",
+                        "exclusive": True,
+                    },
+                ],
+            },
+        )
+
+        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, "@local_as_user:test", "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 uninsteresting 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_match_remote_room_members(self):
+        """
+        Test to make sure that a remote user that is in the room is notified when
+        someone else in the room sends a message.
+        """
+        # Register an application service that's interested in a remote user
+        interested_appservice = self._register_application_service(
+            namespaces={
+                ApplicationService.NS_USERS: [
+                    {
+                        "regex": "@interesting_user:remote",
+                        "exclusive": True,
+                    },
+                ],
+            },
+        )
+
+        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:remote", "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 uninsteresting 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.