Prevent duplicate push notifications for room reads

changelog.d/11835.feature

@@ -0,0 +1 @@
+Make a `POST` to `/rooms/<room_id>/receipt/m.read/<event_id>` only trigger a push notification if the count of unread messages is different to the one in the last successfully sent push.

synapse/push/httppusher.py

@@ -109,6 +109,7 @@ def __init__(self, hs: "HomeServer", pusher_config: PusherConfig):
         self.data_minus_url = {}
         self.data_minus_url.update(self.data)
         del self.data_minus_url["url"]
+        self.badge_count_last_call = -1
 
     def on_started(self, should_check_for_notifs: bool) -> None:
         """Called when this pusher has been started.
@@ -136,7 +137,9 @@ async def _update_badge(self) -> None:
             self.user_id,
             group_by_room=self._group_unread_count_by_room,
         )
-        await self._send_badge(badge)
+        if self.badge_count_last_call is None or self.badge_count_last_call != badge:
+            self.badge_count_last_call = badge
+            await self._send_badge(badge)
 
     def on_timer(self) -> None:
         self._start_processing()
@@ -402,6 +405,8 @@ async def dispatch_push(
         rejected = []
         if "rejected" in resp:
             rejected = resp["rejected"]
+        else:
+            self.badge_count_last_call = badge
         return rejected
 
     async def _send_badge(self, badge: int) -> None:

tests/push/test_http.py

@@ -572,7 +572,7 @@ def test_push_unread_count_group_by_room(self):
         #
         # This push should still only contain an unread count of 1 (for 1 unread room)
         self.assertEqual(
-            self.push_attempts[5][2]["notification"]["counts"]["unread"], 1
+            self.push_attempts[4][2]["notification"]["counts"]["unread"], 1
         )
 
     @override_config({"push": {"group_unread_count_by_room": False}})
@@ -588,7 +588,7 @@ def test_push_unread_count_message_count(self):
         # We're counting every unread message, so there should now be 4 since the
         # last read receipt
         self.assertEqual(
-            self.push_attempts[5][2]["notification"]["counts"]["unread"], 4
+            self.push_attempts[4][2]["notification"]["counts"]["unread"], 4
         )
 
     def _test_push_unread_count(self):
@@ -673,30 +673,20 @@ def _test_push_unread_count(self):
         )
         self.assertEqual(channel.code, 200, channel.json_body)
 
-        # Advance time and make the push succeed
-        self.push_attempts[1][0].callback({})
-        self.pump()
-
-        # Unread count is still zero as we've read the only message in the room
-        self.assertEqual(len(self.push_attempts), 2)
-        self.assertEqual(
-            self.push_attempts[1][2]["notification"]["counts"]["unread"], 0
-        )
-
         # Send another message
         self.helper.send(
             room_id, body="How's the weather today?", tok=other_access_token
         )
 
         # Advance time and make the push succeed
-        self.push_attempts[2][0].callback({})
+        self.push_attempts[1][0].callback({})
         self.pump()
 
         # This push should contain an unread count of 1 as there's now been one
         # message since our last read receipt
-        self.assertEqual(len(self.push_attempts), 3)
+        self.assertEqual(len(self.push_attempts), 2)
         self.assertEqual(
-            self.push_attempts[2][2]["notification"]["counts"]["unread"], 1
+            self.push_attempts[1][2]["notification"]["counts"]["unread"], 1
         )
 
         # Since we're grouping by room, sending more messages shouldn't increase the
@@ -705,18 +695,18 @@ def _test_push_unread_count(self):
 
         # Advance time and make the push succeed
         self.pump()
-        self.push_attempts[3][0].callback({})
+        self.push_attempts[2][0].callback({})
 
         self.helper.send(room_id, body="Hello??", tok=other_access_token)
 
         # Advance time and make the push succeed
         self.pump()
-        self.push_attempts[4][0].callback({})
+        self.push_attempts[3][0].callback({})
 
         self.helper.send(room_id, body="HELLO???", tok=other_access_token)
 
         # Advance time and make the push succeed
         self.pump()
-        self.push_attempts[5][0].callback({})
+        self.push_attempts[4][0].callback({})
 
-        self.assertEqual(len(self.push_attempts), 6)
+        self.assertEqual(len(self.push_attempts), 5)