Review feedback

4 files changed, 19 insertions, 18 deletions

diff --git a/synapse/appservice/__init__.py b/synapse/appservice/__init__.py
index 8052f3bc6d..728cc7849d 100644
--- a/synapse/appservice/__init__.py
+++ b/synapse/appservice/__init__.py
@@ -14,7 +14,7 @@
 # limitations under the License.
 import logging
 import re
-from typing import TYPE_CHECKING, List, Optional
+from typing import TYPE_CHECKING, List, Optional, Match, Iterable
 
 from synapse.api.constants import EventTypes
 from synapse.events import EventBase
@@ -132,7 +132,7 @@ class ApplicationService:
                     raise ValueError("Expected string for 'regex' in ns '%s'" % ns)
         return namespaces
 
-    def _matches_regex(self, test_string: str, namespace_key: str):
+    def _matches_regex(self, test_string: str, namespace_key: str)-> Optional[Match]:
         for regex_obj in self.namespaces[namespace_key]:
             if regex_obj["regex"].match(test_string):
                 return regex_obj
@@ -145,7 +145,7 @@ class ApplicationService:
         return False
 
     async def _matches_user(
-        self, event: EventBase, store: Optional["DataStore"] = None
+        self, event: Optional[EventBase], store: Optional["DataStore"] = None
     ) -> bool:
         if not event:
             return False
@@ -290,14 +290,14 @@ class ApplicationService:
             if regex_obj["exclusive"]
         ]
 
-    def get_groups_for_user(self, user_id: str):
+    def get_groups_for_user(self, user_id: str)-> Iterable[str]:
         """Get the groups that this user is associated with by this AS
 
         Args:
-            user_id (str): The ID of the user.
+            user_id: The ID of the user.
 
         Returns:
-            iterable[str]: an iterable that yields group_id strings.
+            iterable: an iterable that yields group_id strings.
         """
         return (
             regex_obj["group_id"]
diff --git a/synapse/appservice/api.py b/synapse/appservice/api.py
index 88ccdecebd..e16912814c 100644
--- a/synapse/appservice/api.py
+++ b/synapse/appservice/api.py
@@ -222,7 +222,8 @@ class ApplicationServiceApi(SimpleHttpClient):
 
         uri = service.url + ("/transactions/%s" % urllib.parse.quote(str(txn_id)))
 
-        if ephemeral:
+        # Never send ephemeral events to appservices that do not support it
+        if service.supports_ephemeral:
             body = {"events": events, "de.sorunome.msc2409.ephemeral": ephemeral}
         else:
             body = {"events": events}
diff --git a/synapse/appservice/scheduler.py b/synapse/appservice/scheduler.py
index bf294daf1c..7aa534d65d 100644
--- a/synapse/appservice/scheduler.py
+++ b/synapse/appservice/scheduler.py
@@ -86,7 +86,7 @@ class ApplicationServiceScheduler:
             self.txn_ctrl.start_recoverer(service)
 
     def submit_event_for_as(self, service: ApplicationService, event: EventBase):
-        self.queuer.enqueue(service, event)
+        self.queuer.enqueue_event(service, event)
 
     def submit_ephemeral_events_for_as(
         self, service: ApplicationService, events: List[JsonDict]
@@ -120,7 +120,7 @@ class _ServiceQueuer:
             "as-sender-%s" % (service.id,), self._send_request, service
         )
 
-    def enqueue(self, service, event):
+    def enqueue_event(self, service, event):
         self.queued_events.setdefault(service.id, []).append(event)
         self._start_background_request(service)
 
@@ -137,7 +137,7 @@ class _ServiceQueuer:
         try:
             while True:
                 events = self.queued_events.pop(service.id, [])
-                ephemeral = self.queued_ephemeral.pop(service.id, None)
+                ephemeral = self.queued_ephemeral.pop(service.id, [])
                 if not events and not ephemeral:
                     return
                 try:
diff --git a/tests/appservice/test_scheduler.py b/tests/appservice/test_scheduler.py
index fc47134651..f0bb5bb304 100644
--- a/tests/appservice/test_scheduler.py
+++ b/tests/appservice/test_scheduler.py
@@ -202,7 +202,7 @@ class ApplicationServiceSchedulerQueuerTestCase(unittest.TestCase):
         # Expect the event to be sent immediately.
         service = Mock(id=4)
         event = Mock()
-        self.queuer.enqueue(service, event)
+        self.queuer.enqueue_event(service, event)
         self.txn_ctrl.send.assert_called_once_with(service, [event], None)
 
     def test_send_single_event_with_queue(self):
@@ -215,10 +215,10 @@ class ApplicationServiceSchedulerQueuerTestCase(unittest.TestCase):
         event2 = Mock(event_id="second")
         event3 = Mock(event_id="third")
         # Send an event and don't resolve it just yet.
-        self.queuer.enqueue(service, event)
+        self.queuer.enqueue_event(service, event)
         # Send more events: expect send() to NOT be called multiple times.
-        self.queuer.enqueue(service, event2)
-        self.queuer.enqueue(service, event3)
+        self.queuer.enqueue_event(service, event2)
+        self.queuer.enqueue_event(service, event3)
         self.txn_ctrl.send.assert_called_with(service, [event], None)
         self.assertEquals(1, self.txn_ctrl.send.call_count)
         # Resolve the send event: expect the queued events to be sent
@@ -247,11 +247,11 @@ class ApplicationServiceSchedulerQueuerTestCase(unittest.TestCase):
         self.txn_ctrl.send = Mock(side_effect=do_send)
 
         # send events for different ASes and make sure they are sent
-        self.queuer.enqueue(srv1, srv_1_event)
-        self.queuer.enqueue(srv1, srv_1_event2)
+        self.queuer.enqueue_event(srv1, srv_1_event)
+        self.queuer.enqueue_event(srv1, srv_1_event2)
         self.txn_ctrl.send.assert_called_with(srv1, [srv_1_event], None)
-        self.queuer.enqueue(srv2, srv_2_event)
-        self.queuer.enqueue(srv2, srv_2_event2)
+        self.queuer.enqueue_event(srv2, srv_2_event)
+        self.queuer.enqueue_event(srv2, srv_2_event2)
         self.txn_ctrl.send.assert_called_with(srv2, [srv_2_event], None)
 
         # make sure callbacks for a service only send queued events for THAT