Add actual homeserver tests for local/remote interesting to appservic…

…e users

See #13958 (comment)

synapse/appservice/__init__.py

@@ -172,23 +172,14 @@ async def _matches_user_in_member_list(
         Returns:
             True if this service would like to know about this room.
         """
-        # We can use `get_local_users_in_room(...)` here because an application
-        # service can only act on behalf of users of the server it's on.
-        #
-        # In the future, we can consider re-using
-        # `store.get_app_service_users_in_room` which is very similar to this
-        # function but has a slightly worse performance than this because we
-        # have an early escape-hatch if we find a single user that the
-        # appservice is interested in. The juice would be worth the squeeze if
-        # `store.get_app_service_users_in_room` was used in more places besides
-        # an experimental MSC. But for now we can avoid doing more work and
-        # barely using it later.
-        local_user_ids = await store.get_local_users_in_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
         )
 
         # check joined member events
-        for user_id in local_user_ids:
+        for user_id in member_list:
             if self.is_interested_in_user(user_id):
                 return True
         return False
@@ -200,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.

tests/appservice/test_appservice.py

@@ -43,7 +43,7 @@ def setUp(self):
 
         self.store = Mock()
         self.store.get_aliases_for_room = simple_async_mock([])
-        self.store.get_local_users_in_room = simple_async_mock([])
+        self.store.get_users_in_room = simple_async_mock([])
 
     @defer.inlineCallbacks
     def test_regex_user_id_prefix_match(self):
@@ -129,7 +129,7 @@ def test_regex_alias_match(self):
         self.store.get_aliases_for_room = simple_async_mock(
             ["#irc_foobar:matrix.org", "#athing:matrix.org"]
         )
-        self.store.get_local_users_in_room = simple_async_mock([])
+        self.store.get_users_in_room = simple_async_mock([])
         self.assertTrue(
             (
                 yield defer.ensureDeferred(
@@ -184,7 +184,7 @@ def test_regex_alias_no_match(self):
         self.store.get_aliases_for_room = simple_async_mock(
             ["#xmpp_foobar:matrix.org", "#athing:matrix.org"]
         )
-        self.store.get_local_users_in_room = simple_async_mock([])
+        self.store.get_users_in_room = simple_async_mock([])
         self.assertFalse(
             (
                 yield defer.ensureDeferred(
@@ -203,7 +203,7 @@ def test_regex_multiple_matches(self):
         self.service.namespaces[ApplicationService.NS_USERS].append(_regex("@irc_.*"))
         self.event.sender = "@irc_foobar:matrix.org"
         self.store.get_aliases_for_room = simple_async_mock(["#irc_barfoo:matrix.org"])
-        self.store.get_local_users_in_room = simple_async_mock([])
+        self.store.get_users_in_room = simple_async_mock([])
         self.assertTrue(
             (
                 yield defer.ensureDeferred(

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,7 +386,8 @@ 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()
@@ -412,6 +413,138 @@ 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):
+        # 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):
+        # 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.