From 99a623d2095f25909136b60043672a539266e167 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Thu, 29 Sep 2022 17:24:26 -0500
Subject: [PATCH 01/20] 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
---
 synapse/appservice/__init__.py               | 10 +++-------
 synapse/handlers/room_member.py              |  2 +-
 synapse/handlers/user_directory.py           |  1 +
 synapse/storage/databases/main/appservice.py | 14 +++++++++++++-
 synapse/storage/databases/main/roommember.py | 19 ++++++++++++-------
 5 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/synapse/appservice/__init__.py b/synapse/appservice/__init__.py
index 0dfa00df44c7..71b534b0397f 100644
--- a/synapse/appservice/__init__.py
+++ b/synapse/appservice/__init__.py
@@ -172,15 +172,11 @@ async def _matches_user_in_member_list(
         Returns:
             True if this service would like to know about this room.
         """
-        member_list = await store.get_users_in_room(
-            room_id, on_invalidate=cache_context.invalidate
+        app_service_users_in_room = await store.get_app_service_users_in_room(
+            room_id, self, on_invalidate=cache_context.invalidate
         )
 
-        # check joined member events
-        for user_id in member_list:
-            if self.is_interested_in_user(user_id):
-                return True
-        return False
+        return len(app_service_users_in_room) > 0
 
     def is_interested_in_user(
         self,
diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py
index 88158822e009..96cc6eae71be 100644
--- a/synapse/handlers/room_member.py
+++ b/synapse/handlers/room_member.py
@@ -1150,7 +1150,7 @@ async def transfer_room_state_on_room_upgrade(
         logger.info("Transferring room state from %s to %s", old_room_id, room_id)
 
         # Find all local users that were in the old room and copy over each user's state
-        users = await self.store.get_users_in_room(old_room_id)
+        users = await self.store.get_local_users_in_room(old_room_id)
         await self.copy_user_state_on_room_upgrade(old_room_id, room_id, users)
 
         # Add new room to the room directory if the old room was there
diff --git a/synapse/handlers/user_directory.py b/synapse/handlers/user_directory.py
index 8c3c52e1caa6..19df4de43930 100644
--- a/synapse/handlers/user_directory.py
+++ b/synapse/handlers/user_directory.py
@@ -392,6 +392,7 @@ async def _track_user_joined_room(self, room_id: str, user_id: str) -> None:
         if is_public:
             await self.store.add_users_in_public_rooms(room_id, (user_id,))
         else:
+            # TODO: get_local_users_in_room here
             users_in_room = await self.store.get_users_in_room(room_id)
             other_users_in_room = [
                 other
diff --git a/synapse/storage/databases/main/appservice.py b/synapse/storage/databases/main/appservice.py
index 64b70a7b28ee..26fd4fa31a85 100644
--- a/synapse/storage/databases/main/appservice.py
+++ b/synapse/storage/databases/main/appservice.py
@@ -157,7 +157,19 @@ async def get_app_service_users_in_room(
         app_service: "ApplicationService",
         cache_context: _CacheContext,
     ) -> List[str]:
-        users_in_room = await self.get_users_in_room(
+        """
+        Get all users in a room that the appservice controls.
+
+        Args:
+            room_id: The room to check in.
+            app_service: The application service to check interest/control against
+
+        Returns:
+            List of user IDs that the appservice controls.
+        """
+        # 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.
+        users_in_room = await self.get_local_users_in_room(
             room_id, on_invalidate=cache_context.invalidate
         )
         return list(filter(app_service.is_interested_in_user, users_in_room))
diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py
index 982e1f08e30b..8a9e553397b4 100644
--- a/synapse/storage/databases/main/roommember.py
+++ b/synapse/storage/databases/main/roommember.py
@@ -152,13 +152,18 @@ async def get_users_in_room(self, room_id: str) -> List[str]:
         `get_current_hosts_in_room()` and so we can re-use the cache but it's
         not horrible to have here either.
 
-        Uses `m.room.member`s in the room state at the current forward extremities to
-        determine which users are in the room.
-
-        Will return inaccurate results for rooms with partial state, since the state for
-        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.
+        Uses `m.room.member`s in the room state at the current forward
+        extremities to determine which users are in the room.
+
+        Will return inaccurate results for rooms with partial state, since the
+        state for 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

From 806b25512ebb6f80d80b5b9ebfe68749b40f09b4 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Thu, 29 Sep 2022 18:49:09 -0500
Subject: [PATCH 02/20] Remove non-appservice usages split out to other PRs

---
 synapse/handlers/room_member.py    | 2 +-
 synapse/handlers/user_directory.py | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py
index 96cc6eae71be..88158822e009 100644
--- a/synapse/handlers/room_member.py
+++ b/synapse/handlers/room_member.py
@@ -1150,7 +1150,7 @@ async def transfer_room_state_on_room_upgrade(
         logger.info("Transferring room state from %s to %s", old_room_id, room_id)
 
         # Find all local users that were in the old room and copy over each user's state
-        users = await self.store.get_local_users_in_room(old_room_id)
+        users = await self.store.get_users_in_room(old_room_id)
         await self.copy_user_state_on_room_upgrade(old_room_id, room_id, users)
 
         # Add new room to the room directory if the old room was there
diff --git a/synapse/handlers/user_directory.py b/synapse/handlers/user_directory.py
index 19df4de43930..8c3c52e1caa6 100644
--- a/synapse/handlers/user_directory.py
+++ b/synapse/handlers/user_directory.py
@@ -392,7 +392,6 @@ async def _track_user_joined_room(self, room_id: str, user_id: str) -> None:
         if is_public:
             await self.store.add_users_in_public_rooms(room_id, (user_id,))
         else:
-            # TODO: get_local_users_in_room here
             users_in_room = await self.store.get_users_in_room(room_id)
             other_users_in_room = [
                 other

From 5f0f81591b43c6748c1c833504698d3677ee8151 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Thu, 29 Sep 2022 18:52:23 -0500
Subject: [PATCH 03/20] Add changelog

---
 changelog.d/13958.bugfix | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 changelog.d/13958.bugfix

diff --git a/changelog.d/13958.bugfix b/changelog.d/13958.bugfix
new file mode 100644
index 000000000000..090bdf16ece3
--- /dev/null
+++ b/changelog.d/13958.bugfix
@@ -0,0 +1 @@
+Fix performance bottleneck with heavy appservice and bridged users in Synapse by checking appservice user interest against the local users instead of all users.

From a8be41bb80e6a165eccd60b7400c08361e23130f Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Thu, 29 Sep 2022 18:57:07 -0500
Subject: [PATCH 04/20] Revert back to using our own
 `_matches_user_in_member_list` thing

See https://github.com/matrix-org/synapse/pull/13958#discussion_r984074247
---
 synapse/appservice/__init__.py | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/synapse/appservice/__init__.py b/synapse/appservice/__init__.py
index 71b534b0397f..c28bfff0f408 100644
--- a/synapse/appservice/__init__.py
+++ b/synapse/appservice/__init__.py
@@ -172,11 +172,26 @@ async def _matches_user_in_member_list(
         Returns:
             True if this service would like to know about this room.
         """
-        app_service_users_in_room = await store.get_app_service_users_in_room(
-            room_id, self, on_invalidate=cache_context.invalidate
+        # 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.
+        member_list = await store.get_local_users_in_room(
+            room_id, on_invalidate=cache_context.invalidate
         )
 
-        return len(app_service_users_in_room) > 0
+        # check joined member events
+        for user_id in member_list:
+            if self.is_interested_in_user(user_id):
+                return True
+        return False
 
     def is_interested_in_user(
         self,

From 72985dffdbea21d0671673c4459986fb6cdce480 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Thu, 29 Sep 2022 18:59:41 -0500
Subject: [PATCH 05/20] Rename variables to 'local' to be obvious our intention

---
 synapse/appservice/__init__.py               | 4 ++--
 synapse/storage/databases/main/appservice.py | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/synapse/appservice/__init__.py b/synapse/appservice/__init__.py
index c28bfff0f408..a1d328cd2940 100644
--- a/synapse/appservice/__init__.py
+++ b/synapse/appservice/__init__.py
@@ -183,12 +183,12 @@ async def _matches_user_in_member_list(
         # `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.
-        member_list = await store.get_local_users_in_room(
+        local_user_ids = await store.get_local_users_in_room(
             room_id, on_invalidate=cache_context.invalidate
         )
 
         # check joined member events
-        for user_id in member_list:
+        for user_id in local_user_ids:
             if self.is_interested_in_user(user_id):
                 return True
         return False
diff --git a/synapse/storage/databases/main/appservice.py b/synapse/storage/databases/main/appservice.py
index 26fd4fa31a85..726f2547dcc7 100644
--- a/synapse/storage/databases/main/appservice.py
+++ b/synapse/storage/databases/main/appservice.py
@@ -169,10 +169,10 @@ async def get_app_service_users_in_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.
-        users_in_room = await self.get_local_users_in_room(
+        local_users_in_room = await self.get_local_users_in_room(
             room_id, on_invalidate=cache_context.invalidate
         )
-        return list(filter(app_service.is_interested_in_user, users_in_room))
+        return list(filter(app_service.is_interested_in_user, local_users_in_room))
 
 
 class ApplicationServiceStore(ApplicationServiceWorkerStore):

From 92b9da2f177bfb4913d99a8ab49e27d90a9d912b Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Thu, 29 Sep 2022 19:22:47 -0500
Subject: [PATCH 06/20] Fix tests

---
 tests/appservice/test_appservice.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/appservice/test_appservice.py b/tests/appservice/test_appservice.py
index 3018d3fc6f21..d4dccfc2f070 100644
--- a/tests/appservice/test_appservice.py
+++ b/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_users_in_room = simple_async_mock([])
+        self.store.get_local_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_users_in_room = simple_async_mock([])
+        self.store.get_local_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_users_in_room = simple_async_mock([])
+        self.store.get_local_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_users_in_room = simple_async_mock([])
+        self.store.get_local_users_in_room = simple_async_mock([])
         self.assertTrue(
             (
                 yield defer.ensureDeferred(
@@ -236,7 +236,7 @@ def test_interested_in_self(self):
     def test_member_list_match(self):
         self.service.namespaces[ApplicationService.NS_USERS].append(_regex("@irc_.*"))
         # Note that @irc_fo:here is the AS user.
-        self.store.get_users_in_room = simple_async_mock(
+        self.store.get_local_users_in_room = simple_async_mock(
             ["@alice:here", "@irc_fo:here", "@bob:here"]
         )
         self.store.get_aliases_for_room = simple_async_mock([])

From 5d3c6a31dd683391d239b914b2f6908780241e61 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Fri, 30 Sep 2022 12:08:48 -0500
Subject: [PATCH 07/20] Wrapping happens at 88 chars

See https://github.com/matrix-org/synapse/pull/13958#discussion_r984106210
---
 synapse/storage/databases/main/roommember.py | 22 +++++++++-----------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py
index 8a9e553397b4..5230efffb143 100644
--- a/synapse/storage/databases/main/roommember.py
+++ b/synapse/storage/databases/main/roommember.py
@@ -152,18 +152,16 @@ async def get_users_in_room(self, room_id: str) -> List[str]:
         `get_current_hosts_in_room()` and so we can re-use the cache but it's
         not horrible to have here either.
 
-        Uses `m.room.member`s in the room state at the current forward
-        extremities to determine which users are in the room.
-
-        Will return inaccurate results for rooms with partial state, since the
-        state for 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.
+        Uses `m.room.member`s in the room state at the current forward extremities to
+        determine which users are in the room.
+
+        Will return inaccurate results for rooms with partial state, since the state for
+        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

From 76435c7915c3a8d8c5b7ac345573bced20343aaf Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Fri, 30 Sep 2022 13:46:25 -0500
Subject: [PATCH 08/20] Add actual homeserver tests for local/remote
 interesting to appservice users

See https://github.com/matrix-org/synapse/pull/13958#discussion_r984470972
---
 synapse/appservice/__init__.py      |  23 ++---
 tests/appservice/test_appservice.py |   8 +-
 tests/handlers/test_appservice.py   | 139 +++++++++++++++++++++++++++-
 3 files changed, 147 insertions(+), 23 deletions(-)

diff --git a/synapse/appservice/__init__.py b/synapse/appservice/__init__.py
index a1d328cd2940..357ccf109099 100644
--- a/synapse/appservice/__init__.py
+++ b/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.
diff --git a/tests/appservice/test_appservice.py b/tests/appservice/test_appservice.py
index d4dccfc2f070..b3af272944a1 100644
--- a/tests/appservice/test_appservice.py
+++ b/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(
diff --git a/tests/handlers/test_appservice.py b/tests/handlers/test_appservice.py
index af24c4984da5..6a761ac8efd0 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,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.

From 4451998d38920248db5bf13e18fe6d085013cb70 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Fri, 30 Sep 2022 13:51:06 -0500
Subject: [PATCH 09/20] Clarify interested/control and lints

---
 synapse/storage/databases/main/appservice.py | 6 +++---
 tests/handlers/test_appservice.py            | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/synapse/storage/databases/main/appservice.py b/synapse/storage/databases/main/appservice.py
index 726f2547dcc7..0ec18d1baf92 100644
--- a/synapse/storage/databases/main/appservice.py
+++ b/synapse/storage/databases/main/appservice.py
@@ -158,14 +158,14 @@ async def get_app_service_users_in_room(
         cache_context: _CacheContext,
     ) -> List[str]:
         """
-        Get all users in a room that the appservice controls.
+        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/control against
+            app_service: The application service to check interest against
 
         Returns:
-            List of user IDs that the appservice controls.
+            List of user IDs that the appservice is interested in.
         """
         # 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.
diff --git a/tests/handlers/test_appservice.py b/tests/handlers/test_appservice.py
index 6a761ac8efd0..038736b944cb 100644
--- a/tests/handlers/test_appservice.py
+++ b/tests/handlers/test_appservice.py
@@ -391,11 +391,11 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer):
         # 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
         )
 

From 1218f03a8fd2221c38f9fde3f86851ac316cdc42 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Fri, 30 Sep 2022 13:52:36 -0500
Subject: [PATCH 10/20] Revert mock

---
 tests/appservice/test_appservice.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/appservice/test_appservice.py b/tests/appservice/test_appservice.py
index b3af272944a1..3018d3fc6f21 100644
--- a/tests/appservice/test_appservice.py
+++ b/tests/appservice/test_appservice.py
@@ -236,7 +236,7 @@ def test_interested_in_self(self):
     def test_member_list_match(self):
         self.service.namespaces[ApplicationService.NS_USERS].append(_regex("@irc_.*"))
         # Note that @irc_fo:here is the AS user.
-        self.store.get_local_users_in_room = simple_async_mock(
+        self.store.get_users_in_room = simple_async_mock(
             ["@alice:here", "@irc_fo:here", "@bob:here"]
         )
         self.store.get_aliases_for_room = simple_async_mock([])

From 7bd38034f90ef352560c266dc3b6fb883c65c8d8 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Fri, 30 Sep 2022 13:55:02 -0500
Subject: [PATCH 11/20] Add test descriptions

---
 tests/handlers/test_appservice.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tests/handlers/test_appservice.py b/tests/handlers/test_appservice.py
index 038736b944cb..a142be38c5ef 100644
--- a/tests/handlers/test_appservice.py
+++ b/tests/handlers/test_appservice.py
@@ -426,6 +426,10 @@ def _notify_interested_services(self):
         )
 
     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={
@@ -486,6 +490,10 @@ def test_match_local_room_members(self):
         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={

From 33f718c51eea85e1e030a0911eb7a809f9a32c45 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Fri, 30 Sep 2022 13:59:43 -0500
Subject: [PATCH 12/20] Revert "Clarify interested/control and lints"

This reverts commit 4451998d38920248db5bf13e18fe6d085013cb70.
---
 synapse/storage/databases/main/appservice.py | 6 +++---
 tests/handlers/test_appservice.py            | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/synapse/storage/databases/main/appservice.py b/synapse/storage/databases/main/appservice.py
index 0ec18d1baf92..726f2547dcc7 100644
--- a/synapse/storage/databases/main/appservice.py
+++ b/synapse/storage/databases/main/appservice.py
@@ -158,14 +158,14 @@ async def get_app_service_users_in_room(
         cache_context: _CacheContext,
     ) -> List[str]:
         """
-        Get all users in a room that the appservice is interested in.
+        Get all users in a room that the appservice controls.
 
         Args:
             room_id: The room to check in.
-            app_service: The application service to check interest against
+            app_service: The application service to check interest/control against
 
         Returns:
-            List of user IDs that the appservice is interested in.
+            List of user IDs that the appservice controls.
         """
         # 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.
diff --git a/tests/handlers/test_appservice.py b/tests/handlers/test_appservice.py
index a142be38c5ef..474691ff3499 100644
--- a/tests/handlers/test_appservice.py
+++ b/tests/handlers/test_appservice.py
@@ -391,11 +391,11 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer):
         # 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  # type: ignore[assignment]
+        hs.get_application_service_handler().scheduler.txn_ctrl.send = self.send_mock
 
         # Mock out application services, and allow defining our own in tests
         self._services: List[ApplicationService] = []
-        self.hs.get_datastores().main.get_app_services = Mock(  # type: ignore[assignment]
+        self.hs.get_datastores().main.get_app_services = Mock(
             return_value=self._services
         )
 

From cf8299be2c6816a81bfc7617bb0230380e2d56e5 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Fri, 30 Sep 2022 14:00:24 -0500
Subject: [PATCH 13/20] Revert "Add test descriptions"

This reverts commit 7bd38034f90ef352560c266dc3b6fb883c65c8d8.
---
 tests/handlers/test_appservice.py | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/tests/handlers/test_appservice.py b/tests/handlers/test_appservice.py
index 474691ff3499..6a761ac8efd0 100644
--- a/tests/handlers/test_appservice.py
+++ b/tests/handlers/test_appservice.py
@@ -426,10 +426,6 @@ def _notify_interested_services(self):
         )
 
     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={
@@ -490,10 +486,6 @@ def test_match_local_room_members(self):
         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={

From 3de90e6de258e2fde7635222a76063c4ce1c0ab8 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Fri, 30 Sep 2022 14:00:37 -0500
Subject: [PATCH 14/20] Revert "Revert mock"

This reverts commit 1218f03a8fd2221c38f9fde3f86851ac316cdc42.
---
 tests/appservice/test_appservice.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/appservice/test_appservice.py b/tests/appservice/test_appservice.py
index 3018d3fc6f21..b3af272944a1 100644
--- a/tests/appservice/test_appservice.py
+++ b/tests/appservice/test_appservice.py
@@ -236,7 +236,7 @@ def test_interested_in_self(self):
     def test_member_list_match(self):
         self.service.namespaces[ApplicationService.NS_USERS].append(_regex("@irc_.*"))
         # Note that @irc_fo:here is the AS user.
-        self.store.get_users_in_room = simple_async_mock(
+        self.store.get_local_users_in_room = simple_async_mock(
             ["@alice:here", "@irc_fo:here", "@bob:here"]
         )
         self.store.get_aliases_for_room = simple_async_mock([])

From ab33cd60714e319c9fdeb32eb1b7a5d5be8cbd9a Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Fri, 30 Sep 2022 14:01:06 -0500
Subject: [PATCH 15/20] Revert "Add actual homeserver tests for local/remote
 interesting to appservice users"

This reverts commit 76435c7915c3a8d8c5b7ac345573bced20343aaf.
---
 synapse/appservice/__init__.py      |  23 +++--
 tests/appservice/test_appservice.py |   8 +-
 tests/handlers/test_appservice.py   | 139 +---------------------------
 3 files changed, 23 insertions(+), 147 deletions(-)

diff --git a/synapse/appservice/__init__.py b/synapse/appservice/__init__.py
index 357ccf109099..a1d328cd2940 100644
--- a/synapse/appservice/__init__.py
+++ b/synapse/appservice/__init__.py
@@ -172,14 +172,23 @@ 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(
+        # 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(
             room_id, on_invalidate=cache_context.invalidate
         )
 
         # check joined member events
-        for user_id in member_list:
+        for user_id in local_user_ids:
             if self.is_interested_in_user(user_id):
                 return True
         return False
@@ -191,9 +200,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 user can be local or remote.
+        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.
 
         Args:
             user_id: The ID of the user to check.
diff --git a/tests/appservice/test_appservice.py b/tests/appservice/test_appservice.py
index b3af272944a1..d4dccfc2f070 100644
--- a/tests/appservice/test_appservice.py
+++ b/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_users_in_room = simple_async_mock([])
+        self.store.get_local_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_users_in_room = simple_async_mock([])
+        self.store.get_local_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_users_in_room = simple_async_mock([])
+        self.store.get_local_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_users_in_room = simple_async_mock([])
+        self.store.get_local_users_in_room = simple_async_mock([])
         self.assertTrue(
             (
                 yield defer.ensureDeferred(
diff --git a/tests/handlers/test_appservice.py b/tests/handlers/test_appservice.py
index 6a761ac8efd0..af24c4984da5 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, EventTypes
+from synapse.api.constants import EduTypes
 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 event_injection, make_awaitable, simple_async_mock
+from tests.test_utils import make_awaitable, simple_async_mock
 from tests.unittest import override_config
 from tests.utils import MockClock
 
@@ -386,8 +386,7 @@ class ApplicationServicesHandlerSendEventsTestCase(unittest.HomeserverTestCase):
         receipts.register_servlets,
     ]
 
-    def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer):
-        self.hs = hs
+    def prepare(self, reactor, clock, hs):
         # Mock the ApplicationServiceScheduler's _TransactionController's send method so that
         # we can track any outgoing ephemeral events
         self.send_mock = simple_async_mock()
@@ -413,138 +412,6 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer):
             "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.

From 3223512a9b211d15cbe75e850965cf31bf94bbab Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Tue, 4 Oct 2022 17:25:12 -0500
Subject: [PATCH 16/20] Move tests over from #14000

See https://github.com/matrix-org/synapse/pull/14000#discussion_r987127045
---
 synapse/appservice/__init__.py               |   5 +-
 synapse/storage/databases/main/appservice.py |   5 +-
 tests/handlers/test_appservice.py            | 105 ++++++++++++++++++-
 3 files changed, 106 insertions(+), 9 deletions(-)

diff --git a/synapse/appservice/__init__.py b/synapse/appservice/__init__.py
index a1d328cd2940..500bdde3a986 100644
--- a/synapse/appservice/__init__.py
+++ b/synapse/appservice/__init__.py
@@ -172,8 +172,9 @@ 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.
+        # We can use `get_local_users_in_room(...)` here because an application service
+        # can only be interested in local users of the server it's on (ignore any remote
+        # users that might match the user namespace regex).
         #
         # In the future, we can consider re-using
         # `store.get_app_service_users_in_room` which is very similar to this
diff --git a/synapse/storage/databases/main/appservice.py b/synapse/storage/databases/main/appservice.py
index 726f2547dcc7..63046c052771 100644
--- a/synapse/storage/databases/main/appservice.py
+++ b/synapse/storage/databases/main/appservice.py
@@ -167,8 +167,9 @@ async def get_app_service_users_in_room(
         Returns:
             List of user IDs that the appservice controls.
         """
-        # 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.
+        # We can use `get_local_users_in_room(...)` here because an application service
+        # can only be interested in local users of the server it's on (ignore any remote
+        # users that might match the user namespace regex).
         local_users_in_room = await self.get_local_users_in_room(
             room_id, on_invalidate=cache_context.invalidate
         )
diff --git a/tests/handlers/test_appservice.py b/tests/handlers/test_appservice.py
index af24c4984da5..a190a110c8ec 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,100 @@ 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", True),
+            # Defining remote users in an application service user namespace regex is a
+            # footgun since the appservice might assume that it'll receive all events
+            # sent by that remote user, but it will only receive events in rooms that
+            # are shared with a local user. So we just remove this footgun possibility
+            # entirely and we won't notify the application service based on remote
+            # users.
+            ("@remote_as_user:remote", False),
+        ]
+    )
+    def test_match_interesting_room_members(
+        self, interesting_user: str, should_notify: bool
+    ):
+        """
+        Test to make sure that a interesting user (local or remote) in the room is
+        notified as expected 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,
+                    },
+                ],
+            },
+        )
+
+        # Create a room
+        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 the event 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()
+
+        if should_notify:
+            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)
+        else:
+            self.send_mock.assert_not_called()
+
     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.

From 2665aa0a363ec687e9d3d021763265d90c9f3b3f Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Mon, 24 Oct 2022 15:57:33 -0500
Subject: [PATCH 17/20] Update changelog

---
 changelog.d/13958.bugfix | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/changelog.d/13958.bugfix b/changelog.d/13958.bugfix
index 090bdf16ece3..f9f651bfdc07 100644
--- a/changelog.d/13958.bugfix
+++ b/changelog.d/13958.bugfix
@@ -1 +1 @@
-Fix performance bottleneck with heavy appservice and bridged users in Synapse by checking appservice user interest against the local users instead of all users.
+Check appservice user interest against the local users instead of all users in the room to align with [MSC3905](https://github.com/matrix-org/matrix-spec-proposals/pull/3905).

From 39e2ead026fd40da53cd3090e33115b3a94009a2 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Tue, 25 Oct 2022 13:43:23 -0500
Subject: [PATCH 18/20] Add specific test to make sure local interesting user
 events are picked up

See https://github.com/matrix-org/synapse/pull/13958#pullrequestreview-1154474364
---
 tests/handlers/test_appservice.py | 59 ++++++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/tests/handlers/test_appservice.py b/tests/handlers/test_appservice.py
index e688891b9c90..144e49d0fd9c 100644
--- a/tests/handlers/test_appservice.py
+++ b/tests/handlers/test_appservice.py
@@ -504,13 +504,70 @@ def test_match_interesting_room_members(
             ) = 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.
+            # notify us because the interesting user is joined to the room where the
+            # message was sent.
             self.assertEqual(service, interested_appservice)
             self.assertEqual(events[0]["type"], "m.room.message")
             self.assertEqual(events[0]["sender"], alice)
         else:
             self.send_mock.assert_not_called()
 
+    def test_application_services_receive_events_sent_by_interesting_local_user(self):
+        """
+        Test to make sure that a messages sent from a local user can be interesting and
+        picked up by the appservice.
+        """
+        # Register an application service that's interested in all local users
+        interested_appservice = self._register_application_service(
+            namespaces={
+                ApplicationService.NS_USERS: [
+                    {
+                        "regex": ".*",
+                        "exclusive": False,
+                    },
+                ],
+            },
+        )
+
+        # Create a room
+        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)
+
+        # We don't care about interesting events before this (this test is making sure
+        # the next thing works)
+        self.send_mock.reset_mock()
+
+        # Send a message from the interesting local user
+        self.helper.send_event(
+            room_id,
+            type=EventTypes.Message,
+            content={
+                "msgtype": "m.text",
+                "body": "message from interesting local 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]
+
+        # Events sent from an interesting local user should also be picked up as
+        # interesting to the appservice.
+        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.

From 33a5b70ad6ecbde54e1921b6062fcd649e78ff8e Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Tue, 25 Oct 2022 14:07:33 -0500
Subject: [PATCH 19/20] Update upgrade notes

See https://github.com/matrix-org/synapse/pull/13958#discussion_r1004255854
---
 docs/upgrade.md | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/docs/upgrade.md b/docs/upgrade.md
index b81385b19183..f5478228cdd5 100644
--- a/docs/upgrade.md
+++ b/docs/upgrade.md
@@ -88,6 +88,28 @@ process, for example:
     dpkg -i matrix-synapse-py3_1.3.0+stretch1_amd64.deb
     ```
 
+
+# Upgrading to v1.71.0
+
+## Changes to the events received by application services (interest)
+
+To align with spec (changed in
+[MSC3905](https://github.com/matrix-org/matrix-spec-proposals/pull/3905)), Synapse now
+only considers local users to be interesting. In other words, the `users` namespace
+regex is only be applied against local users of the homeserver.
+
+Please note, this probably doesn't affect the expected behavior of your application
+service since an interesting local user in a room, still means all messages in the room
+(from local or remote users) will still be considered interesting. And matching a room
+with the `rooms` or `aliases` namespace regex will still consider all events sent in the
+room to be interesting to the application service.
+
+If one of your application service's `users` regex was intending to match a remote user,
+this will no longer match as you expect. The behavioral mismatch between matching all
+local users and some of the remote user is why the spec was changed/clarified and this
+caveat is no longer supported.
+
+
 # Upgrading to v1.69.0
 
 ## Changes to the receipts replication streams

From 92400ff0b2f96a45d4926df45feb550af4702f1c Mon Sep 17 00:00:00 2001
From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
Date: Thu, 27 Oct 2022 18:46:51 +0100
Subject: [PATCH 20/20] move comma

---
 docs/upgrade.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/upgrade.md b/docs/upgrade.md
index f5478228cdd5..152e1c8d94d4 100644
--- a/docs/upgrade.md
+++ b/docs/upgrade.md
@@ -99,7 +99,7 @@ only considers local users to be interesting. In other words, the `users` namesp
 regex is only be applied against local users of the homeserver.
 
 Please note, this probably doesn't affect the expected behavior of your application
-service since an interesting local user in a room, still means all messages in the room
+service, since an interesting local user in a room still means all messages in the room
 (from local or remote users) will still be considered interesting. And matching a room
 with the `rooms` or `aliases` namespace regex will still consider all events sent in the
 room to be interesting to the application service.