Merge pull request #1022 from matrix-org/erikj/as_notify_perf

Make notify_interested_services faster

synapse/appservice/__init__.py

@@ -14,6 +14,8 @@
 # limitations under the License.
 from synapse.api.constants import EventTypes
 
+from twisted.internet import defer
+
 import logging
 import re
 
@@ -138,65 +140,66 @@ class ApplicationService(object):
             return regex_obj["exclusive"]
         return False
 
-    def _matches_user(self, event, member_list):
+    @defer.inlineCallbacks
-        if (hasattr(event, "sender") and
+    def _matches_user(self, event, store):
-                self.is_interested_in_user(event.sender)):
+        if not event:
-            return True
+            defer.returnValue(False)
+
+        if self.is_interested_in_user(event.sender):
+            defer.returnValue(True)
         # also check m.room.member state key
-        if (hasattr(event, "type") and event.type == EventTypes.Member
+        if (event.type == EventTypes.Member and
-                and hasattr(event, "state_key")
+                self.is_interested_in_user(event.state_key)):
-                and self.is_interested_in_user(event.state_key)):
+            defer.returnValue(True)
-            return True
+
+        if not store:
+            defer.returnValue(False)
+
+        member_list = yield store.get_users_in_room(event.room_id)
+
         # check joined member events
         for user_id in member_list:
             if self.is_interested_in_user(user_id):
-                return True
+                defer.returnValue(True)
-        return False
+        defer.returnValue(False)
 
     def _matches_room_id(self, event):
         if hasattr(event, "room_id"):
             return self.is_interested_in_room(event.room_id)
         return False
 
-    def _matches_aliases(self, event, alias_list):
+    @defer.inlineCallbacks
+    def _matches_aliases(self, event, store):
+        if not store or not event:
+            defer.returnValue(False)
+
+        alias_list = yield store.get_aliases_for_room(event.room_id)
         for alias in alias_list:
             if self.is_interested_in_alias(alias):
-                return True
+                defer.returnValue(True)
-        return False
+        defer.returnValue(False)
 
-    def is_interested(self, event, restrict_to=None, aliases_for_event=None,
+    @defer.inlineCallbacks
-                      member_list=None):
+    def is_interested(self, event, store=None):
         """Check if this service is interested in this event.
 
         Args:
             event(Event): The event to check.
-            restrict_to(str): The namespace to restrict regex tests to.
+            store(DataStore)
-            aliases_for_event(list): A list of all the known room aliases for
-            this event.
-            member_list(list): A list of all joined user_ids in this room.
         Returns:
             bool: True if this service would like to know about this event.
         """
-        if aliases_for_event is None:
+        # Do cheap checks first
-            aliases_for_event = []
+        if self._matches_room_id(event):
-        if member_list is None:
+            defer.returnValue(True)
-            member_list = []
 
-        if restrict_to and restrict_to not in ApplicationService.NS_LIST:
+        if (yield self._matches_aliases(event, store)):
-            # this is a programming error, so fail early and raise a general
+            defer.returnValue(True)
-            # exception
-            raise Exception("Unexpected restrict_to value: %s". restrict_to)
 
-        if not restrict_to:
+        if (yield self._matches_user(event, store)):
-            return (self._matches_user(event, member_list)
+            defer.returnValue(True)
-                    or self._matches_aliases(event, aliases_for_event)
+
-                    or self._matches_room_id(event))
+        defer.returnValue(False)
-        elif restrict_to == ApplicationService.NS_ALIASES:
-            return self._matches_aliases(event, aliases_for_event)
-        elif restrict_to == ApplicationService.NS_ROOMS:
-            return self._matches_room_id(event)
-        elif restrict_to == ApplicationService.NS_USERS:
-            return self._matches_user(event, member_list)
 
     def is_interested_in_user(self, user_id):
         return (

synapse/handlers/appservice.py

@@ -16,7 +16,6 @@
 from twisted.internet import defer
 
 from synapse.api.constants import EventTypes
-from synapse.appservice import ApplicationService
 from synapse.util.metrics import Measure
 
 import logging
@@ -107,11 +106,12 @@ class ApplicationServicesHandler(object):
             association can be found.
         """
         room_alias_str = room_alias.to_string()
-        alias_query_services = yield self._get_services_for_event(
+        services = yield self.store.get_app_services()
-            event=None,
+        alias_query_services = [
-            restrict_to=ApplicationService.NS_ALIASES,
+            s for s in services if (
-            alias_list=[room_alias_str]
+                s.is_interested_in_alias(room_alias_str)
-        )
+            )
+        ]
         for alias_service in alias_query_services:
             is_known_alias = yield self.appservice_api.query_alias(
                 alias_service, room_alias_str
@@ -124,34 +124,19 @@ class ApplicationServicesHandler(object):
                 defer.returnValue(result)
 
     @defer.inlineCallbacks
-    def _get_services_for_event(self, event, restrict_to="", alias_list=None):
+    def _get_services_for_event(self, event):
         """Retrieve a list of application services interested in this event.
 
         Args:
             event(Event): The event to check. Can be None if alias_list is not.
-            restrict_to(str): The namespace to restrict regex tests to.
-            alias_list: A list of aliases to get services for. If None, this
-            list is obtained from the database.
         Returns:
             list<ApplicationService>: A list of services interested in this
             event based on the service regex.
         """
-        member_list = None
-        if hasattr(event, "room_id"):
-            # We need to know the aliases associated with this event.room_id,
-            # if any.
-            if not alias_list:
-                alias_list = yield self.store.get_aliases_for_room(
-                    event.room_id
-                )
-            # We need to know the members associated with this event.room_id,
-            # if any.
-            member_list = yield self.store.get_users_in_room(event.room_id)
-
         services = yield self.store.get_app_services()
         interested_list = [
             s for s in services if (
-                s.is_interested(event, restrict_to, alias_list, member_list)
+                yield s.is_interested(event, self.store)
             )
         ]
         defer.returnValue(interested_list)

tests/appservice/test_appservice.py

@@ -14,6 +14,8 @@
 # limitations under the License.
 from synapse.appservice import ApplicationService
 
+from twisted.internet import defer
+
 from mock import Mock
 from tests import unittest
 
@@ -42,20 +44,25 @@ class ApplicationServiceTestCase(unittest.TestCase):
             type="m.something", room_id="!foo:bar", sender="@someone:somewhere"
         )
 
+        self.store = Mock()
+
+    @defer.inlineCallbacks
     def test_regex_user_id_prefix_match(self):
         self.service.namespaces[ApplicationService.NS_USERS].append(
             _regex("@irc_.*")
         )
         self.event.sender = "@irc_foobar:matrix.org"
-        self.assertTrue(self.service.is_interested(self.event))
+        self.assertTrue((yield self.service.is_interested(self.event)))
 
+    @defer.inlineCallbacks
     def test_regex_user_id_prefix_no_match(self):
         self.service.namespaces[ApplicationService.NS_USERS].append(
             _regex("@irc_.*")
         )
         self.event.sender = "@someone_else:matrix.org"
-        self.assertFalse(self.service.is_interested(self.event))
+        self.assertFalse((yield self.service.is_interested(self.event)))
 
+    @defer.inlineCallbacks
     def test_regex_room_member_is_checked(self):
         self.service.namespaces[ApplicationService.NS_USERS].append(
             _regex("@irc_.*")
@@ -63,30 +70,36 @@ class ApplicationServiceTestCase(unittest.TestCase):
         self.event.sender = "@someone_else:matrix.org"
         self.event.type = "m.room.member"
         self.event.state_key = "@irc_foobar:matrix.org"
-        self.assertTrue(self.service.is_interested(self.event))
+        self.assertTrue((yield self.service.is_interested(self.event)))
 
+    @defer.inlineCallbacks
     def test_regex_room_id_match(self):
         self.service.namespaces[ApplicationService.NS_ROOMS].append(
             _regex("!some_prefix.*some_suffix:matrix.org")
         )
         self.event.room_id = "!some_prefixs0m3th1nGsome_suffix:matrix.org"
-        self.assertTrue(self.service.is_interested(self.event))
+        self.assertTrue((yield self.service.is_interested(self.event)))
 
+    @defer.inlineCallbacks
     def test_regex_room_id_no_match(self):
         self.service.namespaces[ApplicationService.NS_ROOMS].append(
             _regex("!some_prefix.*some_suffix:matrix.org")
         )
         self.event.room_id = "!XqBunHwQIXUiqCaoxq:matrix.org"
-        self.assertFalse(self.service.is_interested(self.event))
+        self.assertFalse((yield self.service.is_interested(self.event)))
 
+    @defer.inlineCallbacks
     def test_regex_alias_match(self):
         self.service.namespaces[ApplicationService.NS_ALIASES].append(
             _regex("#irc_.*:matrix.org")
         )
-        self.assertTrue(self.service.is_interested(
+        self.store.get_aliases_for_room.return_value = [
-            self.event,
+            "#irc_foobar:matrix.org", "#athing:matrix.org"
-            aliases_for_event=["#irc_foobar:matrix.org", "#athing:matrix.org"]
+        ]
-        ))
+        self.store.get_users_in_room.return_value = []
+        self.assertTrue((yield self.service.is_interested(
+            self.event, self.store
+        )))
 
     def test_non_exclusive_alias(self):
         self.service.namespaces[ApplicationService.NS_ALIASES].append(
@@ -136,15 +149,20 @@ class ApplicationServiceTestCase(unittest.TestCase):
             "!irc_foobar:matrix.org"
         ))
 
+    @defer.inlineCallbacks
     def test_regex_alias_no_match(self):
         self.service.namespaces[ApplicationService.NS_ALIASES].append(
             _regex("#irc_.*:matrix.org")
         )
-        self.assertFalse(self.service.is_interested(
+        self.store.get_aliases_for_room.return_value = [
-            self.event,
+            "#xmpp_foobar:matrix.org", "#athing:matrix.org"
-            aliases_for_event=["#xmpp_foobar:matrix.org", "#athing:matrix.org"]
+        ]
-        ))
+        self.store.get_users_in_room.return_value = []
+        self.assertFalse((yield self.service.is_interested(
+            self.event, self.store
+        )))
 
+    @defer.inlineCallbacks
     def test_regex_multiple_matches(self):
         self.service.namespaces[ApplicationService.NS_ALIASES].append(
             _regex("#irc_.*:matrix.org")
@@ -153,53 +171,13 @@ class ApplicationServiceTestCase(unittest.TestCase):
             _regex("@irc_.*")
         )
         self.event.sender = "@irc_foobar:matrix.org"
-        self.assertTrue(self.service.is_interested(
+        self.store.get_aliases_for_room.return_value = ["#irc_barfoo:matrix.org"]
-            self.event,
+        self.store.get_users_in_room.return_value = []
-            aliases_for_event=["#irc_barfoo:matrix.org"]
+        self.assertTrue((yield self.service.is_interested(
-        ))
+            self.event, self.store
-
+        )))
-    def test_restrict_to_rooms(self):
-        self.service.namespaces[ApplicationService.NS_ROOMS].append(
-            _regex("!flibble_.*:matrix.org")
-        )
-        self.service.namespaces[ApplicationService.NS_USERS].append(
-            _regex("@irc_.*")
-        )
-        self.event.sender = "@irc_foobar:matrix.org"
-        self.event.room_id = "!wibblewoo:matrix.org"
-        self.assertFalse(self.service.is_interested(
-            self.event,
-            restrict_to=ApplicationService.NS_ROOMS
-        ))
-
-    def test_restrict_to_aliases(self):
-        self.service.namespaces[ApplicationService.NS_ALIASES].append(
-            _regex("#xmpp_.*:matrix.org")
-        )
-        self.service.namespaces[ApplicationService.NS_USERS].append(
-            _regex("@irc_.*")
-        )
-        self.event.sender = "@irc_foobar:matrix.org"
-        self.assertFalse(self.service.is_interested(
-            self.event,
-            restrict_to=ApplicationService.NS_ALIASES,
-            aliases_for_event=["#irc_barfoo:matrix.org"]
-        ))
-
-    def test_restrict_to_senders(self):
-        self.service.namespaces[ApplicationService.NS_ALIASES].append(
-            _regex("#xmpp_.*:matrix.org")
-        )
-        self.service.namespaces[ApplicationService.NS_USERS].append(
-            _regex("@irc_.*")
-        )
-        self.event.sender = "@xmpp_foobar:matrix.org"
-        self.assertFalse(self.service.is_interested(
-            self.event,
-            restrict_to=ApplicationService.NS_USERS,
-            aliases_for_event=["#xmpp_barfoo:matrix.org"]
-        ))
 
+    @defer.inlineCallbacks
     def test_interested_in_self(self):
         # make sure invites get through
         self.service.sender = "@appservice:name"
@@ -211,20 +189,21 @@ class ApplicationServiceTestCase(unittest.TestCase):
             "membership": "invite"
         }
         self.event.state_key = self.service.sender
-        self.assertTrue(self.service.is_interested(self.event))
+        self.assertTrue((yield self.service.is_interested(self.event)))
 
+    @defer.inlineCallbacks
     def test_member_list_match(self):
         self.service.namespaces[ApplicationService.NS_USERS].append(
             _regex("@irc_.*")
         )
-        join_list = [
+        self.store.get_users_in_room.return_value = [
             "@alice:here",
             "@irc_fo:here",  # AS user
             "@bob:here",
         ]
+        self.store.get_aliases_for_room.return_value = []
 
         self.event.sender = "@xmpp_foobar:matrix.org"
-        self.assertTrue(self.service.is_interested(
+        self.assertTrue((yield self.service.is_interested(
-            event=self.event,
+            event=self.event, store=self.store
-            member_list=join_list
+        )))
-        ))

tests/handlers/test_appservice.py

@@ -110,11 +110,11 @@ class AppServiceHandlerTestCase(unittest.TestCase):
 
         room_id = "!alpha:bet"
         servers = ["aperture"]
-        interested_service = self._mkservice(is_interested=True)
+        interested_service = self._mkservice_alias(is_interested_in_alias=True)
         services = [
-            self._mkservice(is_interested=False),
+            self._mkservice_alias(is_interested_in_alias=False),
             interested_service,
-            self._mkservice(is_interested=False)
+            self._mkservice_alias(is_interested_in_alias=False)
         ]
 
         self.mock_store.get_app_services = Mock(return_value=services)
@@ -137,3 +137,10 @@ class AppServiceHandlerTestCase(unittest.TestCase):
         service.token = "mock_service_token"
         service.url = "mock_service_url"
         return service
+
+    def _mkservice_alias(self, is_interested_in_alias):
+        service = Mock()
+        service.is_interested_in_alias = Mock(return_value=is_interested_in_alias)
+        service.token = "mock_service_token"
+        service.url = "mock_service_url"
+        return service