From aaa402e1fdcb1389ce46607f3031792dee1333c5 Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrewm@element.io>
Date: Wed, 2 Mar 2022 17:47:22 +0000
Subject: [PATCH 01/10] Rename various ApplicationServices interested methods

...to better reflect what they actually do.

* ApplicationServices.is_interested -> is_interested_in_event
* ApplicationService.is_interested_in_user -> is_user_in_namespace
* ApplicationService.is_interested_in_alias -> is_room_alias_in_namespace
* ApplicationService.is_interested_in_room_id -> is_room_id_in_namespace

This leaves us with only an is_interested_in_event method. Next commit
we'll fill in the gaps for checking interest of rooms and users.
---
 synapse/appservice/__init__.py | 25 ++++++++++++-------------
 synapse/handlers/appservice.py |  4 ++--
 synapse/handlers/directory.py  |  6 +++---
 3 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/synapse/appservice/__init__.py b/synapse/appservice/__init__.py
index 4d3f8e492384..cc389e51c5f6 100644
--- a/synapse/appservice/__init__.py
+++ b/synapse/appservice/__init__.py
@@ -176,11 +176,11 @@ def _is_exclusive(self, namespace_key: str, test_string: str) -> bool:
         return False
 
     async def _matches_user(self, event: EventBase, store: "DataStore") -> bool:
-        if self.is_interested_in_user(event.sender):
+        if self.is_user_in_namespace(event.sender):
             return True
 
         # also check m.room.member state key
-        if event.type == EventTypes.Member and self.is_interested_in_user(
+        if event.type == EventTypes.Member and self.is_user_in_namespace(
             event.state_key
         ):
             return True
@@ -195,7 +195,7 @@ async def matches_user_in_member_list(
         store: "DataStore",
         cache_context: _CacheContext,
     ) -> bool:
-        """Check if this service is interested a room based upon it's membership
+        """Check if this service is interested a room based upon its membership
 
         Args:
             room_id: The room to check.
@@ -216,18 +216,20 @@ async def matches_user_in_member_list(
 
     def _matches_room_id(self, event: EventBase) -> bool:
         if hasattr(event, "room_id"):
-            return self.is_interested_in_room(event.room_id)
+            return self.is_room_id_in_namespace(event.room_id)
         return False
 
     async def _matches_aliases(self, event: EventBase, store: "DataStore") -> bool:
         alias_list = await store.get_aliases_for_room(event.room_id)
         for alias in alias_list:
-            if self.is_interested_in_alias(alias):
+            if self.is_room_alias_in_namespace(alias):
                 return True
 
         return False
 
-    async def is_interested(self, event: EventBase, store: "DataStore") -> bool:
+    async def is_interested_in_event(
+        self, event: EventBase, store: "DataStore"
+    ) -> bool:
         """Check if this service is interested in this event.
 
         Args:
@@ -276,16 +278,13 @@ async def is_interested_in_presence(
                 return True
         return False
 
-    def is_interested_in_user(self, user_id: str) -> bool:
-        return (
-            bool(self._matches_regex(ApplicationService.NS_USERS, user_id))
-            or user_id == self.sender
-        )
+    def is_user_in_namespace(self, user_id: str) -> bool:
+        return bool(self._matches_regex(ApplicationService.NS_USERS, user_id))
 
-    def is_interested_in_alias(self, alias: str) -> bool:
+    def is_room_alias_in_namespace(self, alias: str) -> bool:
         return bool(self._matches_regex(ApplicationService.NS_ALIASES, alias))
 
-    def is_interested_in_room(self, room_id: str) -> bool:
+    def is_room_id_in_namespace(self, room_id: str) -> bool:
         return bool(self._matches_regex(ApplicationService.NS_ROOMS, room_id))
 
     def is_exclusive_user(self, user_id: str) -> bool:
diff --git a/synapse/handlers/appservice.py b/synapse/handlers/appservice.py
index e6461cc3c980..3c10651fd351 100644
--- a/synapse/handlers/appservice.py
+++ b/synapse/handlers/appservice.py
@@ -571,7 +571,7 @@ async def query_room_alias_exists(
         room_alias_str = room_alias.to_string()
         services = self.store.get_app_services()
         alias_query_services = [
-            s for s in services if (s.is_interested_in_alias(room_alias_str))
+            s for s in services if (s.is_room_alias_in_namespace(room_alias_str))
         ]
         for alias_service in alias_query_services:
             is_known_alias = await self.appservice_api.query_alias(
@@ -660,7 +660,7 @@ async def _get_services_for_event(
         # inside of a list comprehension anymore.
         interested_list = []
         for s in services:
-            if await s.is_interested(event, self.store):
+            if await s.is_interested_in_event(event, self.store):
                 interested_list.append(s)
 
         return interested_list
diff --git a/synapse/handlers/directory.py b/synapse/handlers/directory.py
index b7064c6624b7..33d827a45b33 100644
--- a/synapse/handlers/directory.py
+++ b/synapse/handlers/directory.py
@@ -119,7 +119,7 @@ async def create_association(
 
         service = requester.app_service
         if service:
-            if not service.is_interested_in_alias(room_alias_str):
+            if not service.is_room_alias_in_namespace(room_alias_str):
                 raise SynapseError(
                     400,
                     "This application service has not reserved this kind of alias.",
@@ -221,7 +221,7 @@ async def delete_association(
     async def delete_appservice_association(
         self, service: ApplicationService, room_alias: RoomAlias
     ) -> None:
-        if not service.is_interested_in_alias(room_alias.to_string()):
+        if not service.is_room_alias_in_namespace(room_alias.to_string()):
             raise SynapseError(
                 400,
                 "This application service has not reserved this kind of alias",
@@ -376,7 +376,7 @@ def can_modify_alias(self, alias: RoomAlias, user_id: Optional[str] = None) -> b
         # non-exclusive locks on the alias (or there are no interested services)
         services = self.store.get_app_services()
         interested_services = [
-            s for s in services if s.is_interested_in_alias(alias.to_string())
+            s for s in services if s.is_room_alias_in_namespace(alias.to_string())
         ]
 
         for service in interested_services:

From fe178639c088e841f5cd937a225a2eb379bdadc7 Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrewm@element.io>
Date: Thu, 3 Mar 2022 13:06:54 +0000
Subject: [PATCH 02/10] Add 'is_interested_in_user' and 'is_interested_in_room'

...that make use of the simpler helper methods we just renamed.

This commit adds the equivalent methods of is_interested_in_event for
both users and rooms. These implementations include more comprehensive
checks then the older methods with the same names.
---
 synapse/appservice/__init__.py | 79 +++++++++++++++++++++++++++++++---
 1 file changed, 72 insertions(+), 7 deletions(-)

diff --git a/synapse/appservice/__init__.py b/synapse/appservice/__init__.py
index cc389e51c5f6..9d3d1b6a3012 100644
--- a/synapse/appservice/__init__.py
+++ b/synapse/appservice/__init__.py
@@ -214,6 +214,68 @@ async def matches_user_in_member_list(
                 return True
         return False
 
+    def is_interested_in_user(
+        self,
+        user_id: str,
+    ) -> bool:
+        """
+        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.
+
+        Args:
+            user_id: The ID of the user to check.
+
+        Returns:
+            True if the application service is interested in the user, False if not.
+        """
+        return (
+            # User is the appservice's sender_localpart user
+            user_id == self.sender
+            # User is in the appservice's user namespace
+            or self.is_user_in_namespace(user_id)
+        )
+
+    @cached(num_args=1, cache_context=True)
+    async def is_interested_in_room(
+        self,
+        room_id: str,
+        store: "DataStore",
+        cache_context: _CacheContext,
+    ) -> bool:
+        """
+        Returns whether the application service is interested in a given room ID.
+
+        The appservice is considered to be interested in the room if either: the ID or one
+        of the aliases of the room is in the appservice's room ID or alias namespace
+        respectively, or if one of the members of the room fall into the appservice's user
+        namespace.
+
+        Args:
+            room_id: The ID of the room to check.
+            store: The homeserver's datastore class.
+
+        Returns:
+            True if the application service is interested in the room, False if not.
+        """
+        # Check if this room ID matches the appservice's room ID namespace
+        if self.is_room_id_in_namespace(room_id):
+            return True
+
+        # likewise with the room's aliases (if it has any)
+        alias_list = await store.get_aliases_for_room(room_id)
+        for alias in alias_list:
+            if self.is_room_alias_in_namespace(alias):
+                return True
+
+        # And finally, perform an expensive check on whether any of the
+        # users in the room match the appservice's user namespace
+        return await self.matches_user_in_member_list(
+            room_id, store, on_invalidate=cache_context.invalidate
+        )
+
     def _matches_room_id(self, event: EventBase) -> bool:
         if hasattr(event, "room_id"):
             return self.is_room_id_in_namespace(event.room_id)
@@ -239,17 +301,20 @@ async def is_interested_in_event(
         Returns:
             True if this service would like to know about this event.
         """
-        # Do cheap checks first
-        if self._matches_room_id(event):
+        # Check if we're interested in this event's sender by namespace (or if they're the
+        # sender_localpart user)
+        if self.is_interested_in_user(event.sender):
             return True
 
-        # This will check the namespaces first before
-        # checking the store, so should be run before _matches_aliases
-        if await self._matches_user(event, store):
+        # additionally, if this is a membership event, perform the same checks on
+        # the user it references
+        if event.type == EventTypes.Member and self.is_interested_in_user(
+            event.state_key
+        ):
             return True
 
-        # This will check the store, so should be run last
-        if await self._matches_aliases(event, store):
+        # This will check the datastore, so should be run last
+        if await self.is_interested_in_room(event.room_id, store):
             return True
 
         return False

From a493a16bd6c1588e644c89e4f93c06ed1b03b22e Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrewm@element.io>
Date: Wed, 2 Mar 2022 17:47:55 +0000
Subject: [PATCH 03/10] Update tests to use the new methods

---
 tests/appservice/test_appservice.py | 23 +++++++++++++----------
 tests/handlers/test_appservice.py   | 17 +++++++++--------
 2 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/tests/appservice/test_appservice.py b/tests/appservice/test_appservice.py
index 9bd6275e92db..6e53dbf4c3b8 100644
--- a/tests/appservice/test_appservice.py
+++ b/tests/appservice/test_appservice.py
@@ -50,7 +50,7 @@ def test_regex_user_id_prefix_match(self):
         self.assertTrue(
             (
                 yield defer.ensureDeferred(
-                    self.service.is_interested(self.event, self.store)
+                    self.service.is_interested_in_event(self.event, self.store)
                 )
             )
         )
@@ -62,7 +62,7 @@ def test_regex_user_id_prefix_no_match(self):
         self.assertFalse(
             (
                 yield defer.ensureDeferred(
-                    self.service.is_interested(self.event, self.store)
+                    self.service.is_interested_in_event(self.event, self.store)
                 )
             )
         )
@@ -76,7 +76,7 @@ def test_regex_room_member_is_checked(self):
         self.assertTrue(
             (
                 yield defer.ensureDeferred(
-                    self.service.is_interested(self.event, self.store)
+                    self.service.is_interested_in_event(self.event, self.store)
                 )
             )
         )
@@ -90,7 +90,8 @@ def test_regex_room_id_match(self):
         self.assertTrue(
             (
                 yield defer.ensureDeferred(
-                    self.service.is_interested(self.event, self.store)
+                    # We need to provide the store here in order to carry out room checks
+                    self.service.is_interested_in_event(self.event, self.store)
                 )
             )
         )
@@ -104,7 +105,7 @@ def test_regex_room_id_no_match(self):
         self.assertFalse(
             (
                 yield defer.ensureDeferred(
-                    self.service.is_interested(self.event, self.store)
+                    self.service.is_interested_in_event(self.event, self.store)
                 )
             )
         )
@@ -121,7 +122,7 @@ def test_regex_alias_match(self):
         self.assertTrue(
             (
                 yield defer.ensureDeferred(
-                    self.service.is_interested(self.event, self.store)
+                    self.service.is_interested_in_event(self.event, self.store)
                 )
             )
         )
@@ -174,7 +175,7 @@ def test_regex_alias_no_match(self):
         self.assertFalse(
             (
                 yield defer.ensureDeferred(
-                    self.service.is_interested(self.event, self.store)
+                    self.service.is_interested_in_event(self.event, self.store)
                 )
             )
         )
@@ -191,7 +192,7 @@ def test_regex_multiple_matches(self):
         self.assertTrue(
             (
                 yield defer.ensureDeferred(
-                    self.service.is_interested(self.event, self.store)
+                    self.service.is_interested_in_event(self.event, self.store)
                 )
             )
         )
@@ -207,7 +208,7 @@ def test_interested_in_self(self):
         self.assertTrue(
             (
                 yield defer.ensureDeferred(
-                    self.service.is_interested(self.event, self.store)
+                    self.service.is_interested_in_event(self.event, self.store)
                 )
             )
         )
@@ -225,7 +226,9 @@ def test_member_list_match(self):
         self.assertTrue(
             (
                 yield defer.ensureDeferred(
-                    self.service.is_interested(event=self.event, store=self.store)
+                    self.service.is_interested_in_event(
+                        event=self.event, store=self.store
+                    )
                 )
             )
         )
diff --git a/tests/handlers/test_appservice.py b/tests/handlers/test_appservice.py
index 072e6bbcdd6e..c73e50e37271 100644
--- a/tests/handlers/test_appservice.py
+++ b/tests/handlers/test_appservice.py
@@ -86,7 +86,6 @@ def test_notify_interested_services(self):
     def test_query_user_exists_unknown_user(self):
         user_id = "@someone:anywhere"
         services = [self._mkservice(is_interested=True)]
-        services[0].is_interested_in_user.return_value = True
         self.mock_store.get_app_services.return_value = services
         self.mock_store.get_user_by_id.return_value = make_awaitable(None)
 
@@ -127,11 +126,11 @@ def test_query_room_alias_exists(self):
 
         room_id = "!alpha:bet"
         servers = ["aperture"]
-        interested_service = self._mkservice_alias(is_interested_in_alias=True)
+        interested_service = self._mkservice_alias(is_room_alias_in_namespace=True)
         services = [
-            self._mkservice_alias(is_interested_in_alias=False),
+            self._mkservice_alias(is_room_alias_in_namespace=False),
             interested_service,
-            self._mkservice_alias(is_interested_in_alias=False),
+            self._mkservice_alias(is_room_alias_in_namespace=False),
         ]
 
         self.mock_as_api.query_alias.return_value = make_awaitable(True)
@@ -325,17 +324,19 @@ def test_notify_interested_services_ephemeral_out_of_order(self):
             interested_service, ephemeral=[]
         )
 
-    def _mkservice(self, is_interested, protocols=None):
+    def _mkservice(self, is_interested: bool, protocols=None):
         service = Mock()
-        service.is_interested.return_value = make_awaitable(is_interested)
+        service.is_interested_in_event.return_value = make_awaitable(is_interested)
+        service.is_interested_in_user.return_value = make_awaitable(is_interested)
+        service.is_interested_in_room.return_value = make_awaitable(is_interested)
         service.token = "mock_service_token"
         service.url = "mock_service_url"
         service.protocols = protocols
         return service
 
-    def _mkservice_alias(self, is_interested_in_alias):
+    def _mkservice_alias(self, is_room_alias_in_namespace):
         service = Mock()
-        service.is_interested_in_alias.return_value = is_interested_in_alias
+        service.is_room_alias_in_namespace.return_value = is_room_alias_in_namespace
         service.token = "mock_service_token"
         service.url = "mock_service_url"
         return service

From 48662270e57f4411a1d2b4838f12977e2bf70bb3 Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrewm@element.io>
Date: Wed, 2 Mar 2022 17:48:13 +0000
Subject: [PATCH 04/10] Remove _matches_user

This function was no longer used.
---
 synapse/appservice/__init__.py | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/synapse/appservice/__init__.py b/synapse/appservice/__init__.py
index 9d3d1b6a3012..9004265b5158 100644
--- a/synapse/appservice/__init__.py
+++ b/synapse/appservice/__init__.py
@@ -175,19 +175,6 @@ def _is_exclusive(self, namespace_key: str, test_string: str) -> bool:
             return namespace.exclusive
         return False
 
-    async def _matches_user(self, event: EventBase, store: "DataStore") -> bool:
-        if self.is_user_in_namespace(event.sender):
-            return True
-
-        # also check m.room.member state key
-        if event.type == EventTypes.Member and self.is_user_in_namespace(
-            event.state_key
-        ):
-            return True
-
-        does_match = await self.matches_user_in_member_list(event.room_id, store)
-        return does_match
-
     @cached(num_args=1, cache_context=True)
     async def matches_user_in_member_list(
         self,

From bcae63e4fa8e122c99f55091352f5bf3df294267 Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrewm@element.io>
Date: Thu, 3 Mar 2022 11:37:24 +0000
Subject: [PATCH 05/10] Remove inline'd _matches_room_id and _matches_aliases
 methods

---
 synapse/appservice/__init__.py | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/synapse/appservice/__init__.py b/synapse/appservice/__init__.py
index 9004265b5158..71a10d64d4eb 100644
--- a/synapse/appservice/__init__.py
+++ b/synapse/appservice/__init__.py
@@ -263,19 +263,6 @@ async def is_interested_in_room(
             room_id, store, on_invalidate=cache_context.invalidate
         )
 
-    def _matches_room_id(self, event: EventBase) -> bool:
-        if hasattr(event, "room_id"):
-            return self.is_room_id_in_namespace(event.room_id)
-        return False
-
-    async def _matches_aliases(self, event: EventBase, store: "DataStore") -> bool:
-        alias_list = await store.get_aliases_for_room(event.room_id)
-        for alias in alias_list:
-            if self.is_room_alias_in_namespace(alias):
-                return True
-
-        return False
-
     async def is_interested_in_event(
         self, event: EventBase, store: "DataStore"
     ) -> bool:

From 381b56081067da1a3d76fac3bc744a570b6f056c Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrew@amorgan.xyz>
Date: Mon, 29 Nov 2021 18:32:05 +0000
Subject: [PATCH 06/10] Add caching to
 ApplicationService.is_interested_in_event

Which will be invalidated when RoomMemberWorkerStore.get_users_in_room
is.

is_interested_in_room already has caching, and is_interested_in_user
does not arguably need it (as the check is so cheap and regex
comparisons are cached anyways).
---
 synapse/appservice/__init__.py | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/synapse/appservice/__init__.py b/synapse/appservice/__init__.py
index 71a10d64d4eb..3f54d936db62 100644
--- a/synapse/appservice/__init__.py
+++ b/synapse/appservice/__init__.py
@@ -247,7 +247,7 @@ async def is_interested_in_room(
         Returns:
             True if the application service is interested in the room, False if not.
         """
-        # Check if this room ID matches the appservice's room ID namespace
+        # Check if we have interest in this room ID
         if self.is_room_id_in_namespace(room_id):
             return True
 
@@ -263,8 +263,9 @@ async def is_interested_in_room(
             room_id, store, on_invalidate=cache_context.invalidate
         )
 
+    @cached(num_args=1, cache_context=True)
     async def is_interested_in_event(
-        self, event: EventBase, store: "DataStore"
+        self, event: EventBase, store: "DataStore", cache_context: _CacheContext
     ) -> bool:
         """Check if this service is interested in this event.
 
@@ -288,14 +289,16 @@ async def is_interested_in_event(
             return True
 
         # This will check the datastore, so should be run last
-        if await self.is_interested_in_room(event.room_id, store):
+        if await self.is_interested_in_room(
+            event.room_id, store, on_invalidate=cache_context.invalidate
+        ):
             return True
 
         return False
 
-    @cached(num_args=1)
+    @cached(num_args=1, cache_context=True)
     async def is_interested_in_presence(
-        self, user_id: UserID, store: "DataStore"
+        self, user_id: UserID, store: "DataStore", cache_context: _CacheContext
     ) -> bool:
         """Check if this service is interested a user's presence
 
@@ -313,7 +316,9 @@ async def is_interested_in_presence(
 
         # Then find out if the appservice is interested in any of those rooms
         for room_id in room_ids:
-            if await self.matches_user_in_member_list(room_id, store):
+            if await self.matches_user_in_member_list(
+                room_id, store, on_invalidate=cache_context.invalidate
+            ):
                 return True
         return False
 

From bf00539f2652b7f32c361ee407347b975e00c170 Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrewm@element.io>
Date: Thu, 3 Mar 2022 12:34:11 +0000
Subject: [PATCH 07/10] Make matches_user_in_member_list method private; update
 refs

References to this method wanted to check if an AS should receive events
from a room. They were only checking to see if the AS had a user in the
room, whereas the AS should also see those events if it has registered
the room's ID or one of its aliases in the AS' relevant namespace.
---
 synapse/appservice/__init__.py | 6 +++---
 synapse/handlers/receipts.py   | 2 +-
 synapse/handlers/typing.py     | 4 +---
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/synapse/appservice/__init__.py b/synapse/appservice/__init__.py
index 3f54d936db62..5072650fcf3e 100644
--- a/synapse/appservice/__init__.py
+++ b/synapse/appservice/__init__.py
@@ -176,7 +176,7 @@ def _is_exclusive(self, namespace_key: str, test_string: str) -> bool:
         return False
 
     @cached(num_args=1, cache_context=True)
-    async def matches_user_in_member_list(
+    async def _matches_user_in_member_list(
         self,
         room_id: str,
         store: "DataStore",
@@ -259,7 +259,7 @@ async def is_interested_in_room(
 
         # And finally, perform an expensive check on whether any of the
         # users in the room match the appservice's user namespace
-        return await self.matches_user_in_member_list(
+        return await self._matches_user_in_member_list(
             room_id, store, on_invalidate=cache_context.invalidate
         )
 
@@ -316,7 +316,7 @@ async def is_interested_in_presence(
 
         # Then find out if the appservice is interested in any of those rooms
         for room_id in room_ids:
-            if await self.matches_user_in_member_list(
+            if await self.is_interested_in_room(
                 room_id, store, on_invalidate=cache_context.invalidate
             ):
                 return True
diff --git a/synapse/handlers/receipts.py b/synapse/handlers/receipts.py
index b4132c353ae2..6250bb3bdf2b 100644
--- a/synapse/handlers/receipts.py
+++ b/synapse/handlers/receipts.py
@@ -269,7 +269,7 @@ async def get_new_events_as(
         # Then filter down to rooms that the AS can read
         events = []
         for room_id, event in rooms_to_events.items():
-            if not await service.matches_user_in_member_list(room_id, self.store):
+            if not await service.is_interested_in_room(room_id, self.store):
                 continue
 
             events.append(event)
diff --git a/synapse/handlers/typing.py b/synapse/handlers/typing.py
index 843c68eb0fdf..3b8912652856 100644
--- a/synapse/handlers/typing.py
+++ b/synapse/handlers/typing.py
@@ -486,9 +486,7 @@ async def get_new_events_as(
                 if handler._room_serials[room_id] <= from_key:
                     continue
 
-                if not await service.matches_user_in_member_list(
-                    room_id, self._main_store
-                ):
+                if not await service.is_interested_in_room(room_id, self._main_store):
                     continue
 
                 events.append(self._make_event_for(room_id))

From 27ffd6dddbbaf50151e2ee3089b0c796ef16f6ff Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrewm@element.io>
Date: Thu, 3 Mar 2022 11:00:55 +0000
Subject: [PATCH 08/10] Changelog

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

diff --git a/changelog.d/11915.misc b/changelog.d/11915.misc
new file mode 100644
index 000000000000..e3cef1511eb6
--- /dev/null
+++ b/changelog.d/11915.misc
@@ -0,0 +1 @@
+Simplify the `ApplicationService` class' set of public methods related to interest checking.
\ No newline at end of file

From eb779e03042f326dad48b740e48970de7ad79072 Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrewm@element.io>
Date: Thu, 3 Mar 2022 14:45:42 +0000
Subject: [PATCH 09/10] Type hints and docstring for _mkservice,
 _mkservice_alias

This commit also returns _mkservice to only modifying whether an AS is
interested in an event (and shifts other interest modifications back to
specific test methods).

Modifying _mkservice to enable further interest wasn't necessary for these
tests to pass.
---
 tests/handlers/test_appservice.py | 51 +++++++++++++++++++++++--------
 1 file changed, 39 insertions(+), 12 deletions(-)

diff --git a/tests/handlers/test_appservice.py b/tests/handlers/test_appservice.py
index c73e50e37271..cead9f90df56 100644
--- a/tests/handlers/test_appservice.py
+++ b/tests/handlers/test_appservice.py
@@ -59,11 +59,11 @@ def setUp(self):
         self.event_source = hs.get_event_sources()
 
     def test_notify_interested_services(self):
-        interested_service = self._mkservice(is_interested=True)
+        interested_service = self._mkservice(is_interested_in_event=True)
         services = [
-            self._mkservice(is_interested=False),
+            self._mkservice(is_interested_in_event=False),
             interested_service,
-            self._mkservice(is_interested=False),
+            self._mkservice(is_interested_in_event=False),
         ]
 
         self.mock_as_api.query_user.return_value = make_awaitable(True)
@@ -85,7 +85,8 @@ def test_notify_interested_services(self):
 
     def test_query_user_exists_unknown_user(self):
         user_id = "@someone:anywhere"
-        services = [self._mkservice(is_interested=True)]
+        services = [self._mkservice(is_interested_in_event=True)]
+        services[0].is_interested_in_user.return_value = True
         self.mock_store.get_app_services.return_value = services
         self.mock_store.get_user_by_id.return_value = make_awaitable(None)
 
@@ -101,7 +102,7 @@ def test_query_user_exists_unknown_user(self):
 
     def test_query_user_exists_known_user(self):
         user_id = "@someone:anywhere"
-        services = [self._mkservice(is_interested=True)]
+        services = [self._mkservice(is_interested_in_event=True)]
         services[0].is_interested_in_user.return_value = True
         self.mock_store.get_app_services.return_value = services
         self.mock_store.get_user_by_id.return_value = make_awaitable({"name": user_id})
@@ -274,7 +275,7 @@ def test_notify_interested_services_ephemeral(self):
         to be pushed out to interested appservices, and that the stream ID is
         updated accordingly.
         """
-        interested_service = self._mkservice(is_interested=True)
+        interested_service = self._mkservice(is_interested_in_event=True)
         services = [interested_service]
         self.mock_store.get_app_services.return_value = services
         self.mock_store.get_type_stream_id_for_appservice.return_value = make_awaitable(
@@ -303,7 +304,7 @@ def test_notify_interested_services_ephemeral_out_of_order(self):
         Test sending out of order ephemeral events to the appservice handler
         are ignored.
         """
-        interested_service = self._mkservice(is_interested=True)
+        interested_service = self._mkservice(is_interested_in_event=True)
         services = [interested_service]
 
         self.mock_store.get_app_services.return_value = services
@@ -324,17 +325,43 @@ def test_notify_interested_services_ephemeral_out_of_order(self):
             interested_service, ephemeral=[]
         )
 
-    def _mkservice(self, is_interested: bool, protocols=None):
+    def _mkservice(
+        self, is_interested_in_event: bool, protocols: Optional[Iterable] = None
+    ) -> Mock:
+        """
+        Create a new mock representing an ApplicationService.
+
+        Args:
+            is_interested_in_event: Whether this application service will be considered
+                interested in all events.
+            protocols: The third-party protocols that this application service claims to
+                support.
+
+        Returns:
+            A mock representing the ApplicationService.
+        """
         service = Mock()
-        service.is_interested_in_event.return_value = make_awaitable(is_interested)
-        service.is_interested_in_user.return_value = make_awaitable(is_interested)
-        service.is_interested_in_room.return_value = make_awaitable(is_interested)
+        service.is_interested_in_event.return_value = make_awaitable(
+            is_interested_in_event
+        )
         service.token = "mock_service_token"
         service.url = "mock_service_url"
         service.protocols = protocols
         return service
 
-    def _mkservice_alias(self, is_room_alias_in_namespace):
+    def _mkservice_alias(self, is_room_alias_in_namespace: bool) -> Mock:
+        """
+        Create a new mock representing an ApplicationService that is or is not interested
+        any given room aliase.
+
+        Args:
+            is_room_alias_in_namespace: If true, the application service will be interested
+                in all room aliases that are queried against it. If false, the application
+                service will not be interested in any room aliases.
+
+        Returns:
+            A mock representing the ApplicationService.
+        """
         service = Mock()
         service.is_room_alias_in_namespace.return_value = is_room_alias_in_namespace
         service.token = "mock_service_token"

From 453fdacb7af6860630cc4f53be5ac18db4ac56ff Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrewm@element.io>
Date: Thu, 3 Mar 2022 17:46:52 +0000
Subject: [PATCH 10/10] Add an event ID arg to is_interested_in_event to cache
 with

Caching using an EventBase is not quite sufficient while we have no defined way of
checking whether two instances are equal other than if they are the equivalent object.

Instead, we use the event ID here for now, which should be good enough in this case.
---
 synapse/appservice/__init__.py      | 10 +++++--
 synapse/handlers/appservice.py      |  2 +-
 tests/appservice/test_appservice.py | 44 +++++++++++++++++++++--------
 3 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/synapse/appservice/__init__.py b/synapse/appservice/__init__.py
index 5072650fcf3e..07ec95f1d67e 100644
--- a/synapse/appservice/__init__.py
+++ b/synapse/appservice/__init__.py
@@ -265,16 +265,22 @@ async def is_interested_in_room(
 
     @cached(num_args=1, cache_context=True)
     async def is_interested_in_event(
-        self, event: EventBase, store: "DataStore", cache_context: _CacheContext
+        self,
+        event_id: str,
+        event: EventBase,
+        store: "DataStore",
+        cache_context: _CacheContext,
     ) -> bool:
         """Check if this service is interested in this event.
 
         Args:
+            event_id: The ID of the event to check. This is purely used for simplifying the
+                caching of calls to this method.
             event: The event to check.
             store: The datastore to query.
 
         Returns:
-            True if this service would like to know about this event.
+            True if this service would like to know about this event, otherwise False.
         """
         # Check if we're interested in this event's sender by namespace (or if they're the
         # sender_localpart user)
diff --git a/synapse/handlers/appservice.py b/synapse/handlers/appservice.py
index 3c10651fd351..bd913e524e7b 100644
--- a/synapse/handlers/appservice.py
+++ b/synapse/handlers/appservice.py
@@ -660,7 +660,7 @@ async def _get_services_for_event(
         # inside of a list comprehension anymore.
         interested_list = []
         for s in services:
-            if await s.is_interested_in_event(event, self.store):
+            if await s.is_interested_in_event(event.event_id, event, self.store):
                 interested_list.append(s)
 
         return interested_list
diff --git a/tests/appservice/test_appservice.py b/tests/appservice/test_appservice.py
index 6e53dbf4c3b8..edc584d0cf50 100644
--- a/tests/appservice/test_appservice.py
+++ b/tests/appservice/test_appservice.py
@@ -36,7 +36,10 @@ def setUp(self):
             hostname="matrix.org",  # only used by get_groups_for_user
         )
         self.event = Mock(
-            type="m.something", room_id="!foo:bar", sender="@someone:somewhere"
+            event_id="$abc:xyz",
+            type="m.something",
+            room_id="!foo:bar",
+            sender="@someone:somewhere",
         )
 
         self.store = Mock()
@@ -50,7 +53,9 @@ def test_regex_user_id_prefix_match(self):
         self.assertTrue(
             (
                 yield defer.ensureDeferred(
-                    self.service.is_interested_in_event(self.event, self.store)
+                    self.service.is_interested_in_event(
+                        self.event.event_id, self.event, self.store
+                    )
                 )
             )
         )
@@ -62,7 +67,9 @@ def test_regex_user_id_prefix_no_match(self):
         self.assertFalse(
             (
                 yield defer.ensureDeferred(
-                    self.service.is_interested_in_event(self.event, self.store)
+                    self.service.is_interested_in_event(
+                        self.event.event_id, self.event, self.store
+                    )
                 )
             )
         )
@@ -76,7 +83,9 @@ def test_regex_room_member_is_checked(self):
         self.assertTrue(
             (
                 yield defer.ensureDeferred(
-                    self.service.is_interested_in_event(self.event, self.store)
+                    self.service.is_interested_in_event(
+                        self.event.event_id, self.event, self.store
+                    )
                 )
             )
         )
@@ -90,8 +99,9 @@ def test_regex_room_id_match(self):
         self.assertTrue(
             (
                 yield defer.ensureDeferred(
-                    # We need to provide the store here in order to carry out room checks
-                    self.service.is_interested_in_event(self.event, self.store)
+                    self.service.is_interested_in_event(
+                        self.event.event_id, self.event, self.store
+                    )
                 )
             )
         )
@@ -105,7 +115,9 @@ def test_regex_room_id_no_match(self):
         self.assertFalse(
             (
                 yield defer.ensureDeferred(
-                    self.service.is_interested_in_event(self.event, self.store)
+                    self.service.is_interested_in_event(
+                        self.event.event_id, self.event, self.store
+                    )
                 )
             )
         )
@@ -122,7 +134,9 @@ def test_regex_alias_match(self):
         self.assertTrue(
             (
                 yield defer.ensureDeferred(
-                    self.service.is_interested_in_event(self.event, self.store)
+                    self.service.is_interested_in_event(
+                        self.event.event_id, self.event, self.store
+                    )
                 )
             )
         )
@@ -175,7 +189,9 @@ def test_regex_alias_no_match(self):
         self.assertFalse(
             (
                 yield defer.ensureDeferred(
-                    self.service.is_interested_in_event(self.event, self.store)
+                    self.service.is_interested_in_event(
+                        self.event.event_id, self.event, self.store
+                    )
                 )
             )
         )
@@ -192,7 +208,9 @@ def test_regex_multiple_matches(self):
         self.assertTrue(
             (
                 yield defer.ensureDeferred(
-                    self.service.is_interested_in_event(self.event, self.store)
+                    self.service.is_interested_in_event(
+                        self.event.event_id, self.event, self.store
+                    )
                 )
             )
         )
@@ -208,7 +226,9 @@ def test_interested_in_self(self):
         self.assertTrue(
             (
                 yield defer.ensureDeferred(
-                    self.service.is_interested_in_event(self.event, self.store)
+                    self.service.is_interested_in_event(
+                        self.event.event_id, self.event, self.store
+                    )
                 )
             )
         )
@@ -227,7 +247,7 @@ def test_member_list_match(self):
             (
                 yield defer.ensureDeferred(
                     self.service.is_interested_in_event(
-                        event=self.event, store=self.store
+                        self.event.event_id, self.event, self.store
                     )
                 )
             )