Make unread notification count sending work: put the correct count in incremental syncs too, where necessary, and fix silly bugs like only select the event actions for that user...

2 files changed, 49 insertions, 20 deletions

diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py
index 6d193a10c4..44420a063a 100644
--- a/synapse/handlers/sync.py
+++ b/synapse/handlers/sync.py
@@ -65,7 +65,8 @@ class JoinedSyncResult(collections.namedtuple("JoinedSyncResult", [
             or self.state
             or self.ephemeral
             or self.account_data
-            or self.unread_notification_count > 0
+            # nb the notification count does not, er, count: if there's nothing
+            # else in the result, we don't need to send it.
         )
 
 
@@ -279,15 +280,12 @@ class SyncHandler(BaseHandler):
             room_id, sync_config, now_token, since_token=timeline_since_token
         )
 
-        last_unread_event_id = self.last_read_event_id_for_room_and_user(
-            room_id, sync_config.user.to_string(), ephemeral_by_room
+        notifs = yield self.unread_notifs_for_room_id(
+                room_id, sync_config, ephemeral_by_room
         )
-
-        notifs = []
-        if last_unread_event_id:
-            notifs = yield self.store.get_unread_event_actions_by_room(
-                room_id, last_unread_event_id
-            )
+        notif_count = None
+        if notifs is not None:
+            notif_count = len(notifs)
 
         current_state = yield self.get_state_at(room_id, now_token)
 
@@ -299,7 +297,7 @@ class SyncHandler(BaseHandler):
             account_data=self.account_data_for_room(
                 room_id, tags_by_room, account_data_by_room
             ),
-            unread_notification_count=len(notifs)
+            unread_notification_count=notif_count
         ))
 
     def account_data_for_user(self, account_data):
@@ -441,6 +439,10 @@ class SyncHandler(BaseHandler):
         )
         now_token = now_token.copy_and_replace("presence_key", presence_key)
 
+        _, all_ephemeral_by_room = yield self.ephemeral_by_room(
+            sync_config, now_token
+        )
+
         now_token, ephemeral_by_room = yield self.ephemeral_by_room(
             sync_config, now_token, since_token
         )
@@ -514,6 +516,13 @@ class SyncHandler(BaseHandler):
                 else:
                     prev_batch = now_token
 
+                notifs = yield self.unread_notifs_for_room_id(
+                    room_id, sync_config, all_ephemeral_by_room
+                )
+                notif_count = None
+                if notifs is not None:
+                    notif_count = len(notifs)
+
                 just_joined = yield self.check_joined_room(sync_config, state)
                 if just_joined:
                     logger.debug("User has just joined %s: needs full state",
@@ -534,7 +543,7 @@ class SyncHandler(BaseHandler):
                     account_data=self.account_data_for_room(
                         room_id, tags_by_room, account_data_by_room
                     ),
-                    unread_notification_count=0
+                    unread_notification_count=notif_count
                 )
                 logger.debug("Result for room %s: %r", room_id, room_sync)
 
@@ -805,3 +814,20 @@ class SyncHandler(BaseHandler):
             if join_event.content["membership"] == Membership.JOIN:
                 return True
         return False
+
+    @defer.inlineCallbacks
+    def unread_notifs_for_room_id(self, room_id, sync_config, ephemeral_by_room):
+        last_unread_event_id = self.last_read_event_id_for_room_and_user(
+            room_id, sync_config.user.to_string(), ephemeral_by_room
+        )
+
+        notifs = []
+        if last_unread_event_id:
+            notifs = yield self.store.get_unread_event_actions_by_room_for_user(
+                room_id, sync_config.user.to_string(), last_unread_event_id
+            )
+        else:
+            # There is no new information in this period, so your notification
+            # count is whatever it was last time.
+            defer.returnValue(None)
+        defer.returnValue(notifs)
\ No newline at end of file
diff --git a/synapse/storage/event_actions.py b/synapse/storage/event_actions.py
index 40ac8e2d27..f7fe78e554 100644
--- a/synapse/storage/event_actions.py
+++ b/synapse/storage/event_actions.py
@@ -42,12 +42,9 @@ class EventActionsStore(SQLBaseStore):
         defer.returnValue(ret)
 
     @defer.inlineCallbacks
-    def get_unread_event_actions_by_room(self, room_id, last_read_event_id):
-        #events = yield self._get_events(
-        #    [last_read_event_id],
-        #    check_redacted=False
-        #)
-
+    def get_unread_event_actions_by_room_for_user(
+            self, room_id, user_id, last_read_event_id
+    ):
         def _get_unread_event_actions_by_room(txn):
             sql = (
                 "SELECT stream_ordering, topological_ordering"
@@ -65,10 +62,11 @@ class EventActionsStore(SQLBaseStore):
             topological_ordering = results[0][1]
 
             sql = (
-                "SELECT ea.actions"
+                "SELECT ea.event_id, ea.actions"
                 " FROM event_actions ea, events e"
                 " WHERE ea.room_id = e.room_id"
                 " AND ea.event_id = e.event_id"
+                " AND ea.user_id = ?"
                 " AND ea.room_id = ?"
                 " AND ("
                 "       e.topological_ordering > ?"
@@ -76,9 +74,14 @@ class EventActionsStore(SQLBaseStore):
                 ")"
             )
             txn.execute(sql,
-                (room_id, topological_ordering, topological_ordering, stream_ordering)
+                (
+                    user_id, room_id,
+                    topological_ordering, topological_ordering, stream_ordering
+                )
             )
-            return txn.fetchall()
+            return [
+                { "event_id": row[0], "actions": row[1] } for row in txn.fetchall()
+            ]
 
         ret = yield self.runInteraction(
             "get_unread_event_actions_by_room",