mirror of
https://git.anonymousland.org/anonymousland/synapse.git
synced 2024-09-03 01:11:54 +00:00
Prevent duplicate push notifications for room reads (#11835)
This commit is contained in:
parent
73fc488783
commit
4077177390
1
changelog.d/11835.feature
Normal file
1
changelog.d/11835.feature
Normal file
@ -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.
|
@ -109,6 +109,7 @@ class HttpPusher(Pusher):
|
|||||||
self.data_minus_url = {}
|
self.data_minus_url = {}
|
||||||
self.data_minus_url.update(self.data)
|
self.data_minus_url.update(self.data)
|
||||||
del self.data_minus_url["url"]
|
del self.data_minus_url["url"]
|
||||||
|
self.badge_count_last_call: Optional[int] = None
|
||||||
|
|
||||||
def on_started(self, should_check_for_notifs: bool) -> None:
|
def on_started(self, should_check_for_notifs: bool) -> None:
|
||||||
"""Called when this pusher has been started.
|
"""Called when this pusher has been started.
|
||||||
@ -136,6 +137,8 @@ class HttpPusher(Pusher):
|
|||||||
self.user_id,
|
self.user_id,
|
||||||
group_by_room=self._group_unread_count_by_room,
|
group_by_room=self._group_unread_count_by_room,
|
||||||
)
|
)
|
||||||
|
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)
|
await self._send_badge(badge)
|
||||||
|
|
||||||
def on_timer(self) -> None:
|
def on_timer(self) -> None:
|
||||||
@ -402,6 +405,8 @@ class HttpPusher(Pusher):
|
|||||||
rejected = []
|
rejected = []
|
||||||
if "rejected" in resp:
|
if "rejected" in resp:
|
||||||
rejected = resp["rejected"]
|
rejected = resp["rejected"]
|
||||||
|
else:
|
||||||
|
self.badge_count_last_call = badge
|
||||||
return rejected
|
return rejected
|
||||||
|
|
||||||
async def _send_badge(self, badge: int) -> None:
|
async def _send_badge(self, badge: int) -> None:
|
||||||
|
@ -571,9 +571,7 @@ class HTTPPusherTests(HomeserverTestCase):
|
|||||||
# Carry out our option-value specific test
|
# Carry out our option-value specific test
|
||||||
#
|
#
|
||||||
# This push should still only contain an unread count of 1 (for 1 unread room)
|
# This push should still only contain an unread count of 1 (for 1 unread room)
|
||||||
self.assertEqual(
|
self._check_push_attempt(6, 1)
|
||||||
self.push_attempts[5][2]["notification"]["counts"]["unread"], 1
|
|
||||||
)
|
|
||||||
|
|
||||||
@override_config({"push": {"group_unread_count_by_room": False}})
|
@override_config({"push": {"group_unread_count_by_room": False}})
|
||||||
def test_push_unread_count_message_count(self):
|
def test_push_unread_count_message_count(self):
|
||||||
@ -585,11 +583,9 @@ class HTTPPusherTests(HomeserverTestCase):
|
|||||||
|
|
||||||
# Carry out our option-value specific test
|
# Carry out our option-value specific test
|
||||||
#
|
#
|
||||||
# We're counting every unread message, so there should now be 4 since the
|
# We're counting every unread message, so there should now be 3 since the
|
||||||
# last read receipt
|
# last read receipt
|
||||||
self.assertEqual(
|
self._check_push_attempt(6, 3)
|
||||||
self.push_attempts[5][2]["notification"]["counts"]["unread"], 4
|
|
||||||
)
|
|
||||||
|
|
||||||
def _test_push_unread_count(self):
|
def _test_push_unread_count(self):
|
||||||
"""
|
"""
|
||||||
@ -597,8 +593,9 @@ class HTTPPusherTests(HomeserverTestCase):
|
|||||||
|
|
||||||
Note that:
|
Note that:
|
||||||
* Sending messages will cause push notifications to go out to relevant users
|
* Sending messages will cause push notifications to go out to relevant users
|
||||||
* Sending a read receipt will cause a "badge update" notification to go out to
|
* Sending a read receipt will cause the HTTP pusher to check whether the unread
|
||||||
the user that sent the receipt
|
count has changed since the last push notification. If so, a "badge update"
|
||||||
|
notification goes out to the user that sent the receipt
|
||||||
"""
|
"""
|
||||||
# Register the user who gets notified
|
# Register the user who gets notified
|
||||||
user_id = self.register_user("user", "pass")
|
user_id = self.register_user("user", "pass")
|
||||||
@ -642,24 +639,74 @@ class HTTPPusherTests(HomeserverTestCase):
|
|||||||
# position in the room. We'll set the read position to this event in a moment
|
# position in the room. We'll set the read position to this event in a moment
|
||||||
first_message_event_id = response["event_id"]
|
first_message_event_id = response["event_id"]
|
||||||
|
|
||||||
# Advance time a bit (so the pusher will register something has happened) and
|
expected_push_attempts = 1
|
||||||
# make the push succeed
|
self._check_push_attempt(expected_push_attempts, 0)
|
||||||
self.push_attempts[0][0].callback({})
|
|
||||||
self.pump()
|
|
||||||
|
|
||||||
# Check our push made it
|
self._send_read_request(access_token, first_message_event_id, room_id)
|
||||||
|
|
||||||
|
# Unread count has not changed. Therefore, ensure that read request does not
|
||||||
|
# trigger a push notification.
|
||||||
self.assertEqual(len(self.push_attempts), 1)
|
self.assertEqual(len(self.push_attempts), 1)
|
||||||
self.assertEqual(
|
|
||||||
self.push_attempts[0][1], "http://example.com/_matrix/push/v1/notify"
|
|
||||||
)
|
|
||||||
|
|
||||||
|
# Send another message
|
||||||
|
response2 = self.helper.send(
|
||||||
|
room_id, body="How's the weather today?", tok=other_access_token
|
||||||
|
)
|
||||||
|
second_message_event_id = response2["event_id"]
|
||||||
|
|
||||||
|
expected_push_attempts += 1
|
||||||
|
|
||||||
|
self._check_push_attempt(expected_push_attempts, 1)
|
||||||
|
|
||||||
|
self._send_read_request(access_token, second_message_event_id, room_id)
|
||||||
|
expected_push_attempts += 1
|
||||||
|
|
||||||
|
self._check_push_attempt(expected_push_attempts, 0)
|
||||||
|
|
||||||
|
# If we're grouping by room, sending more messages shouldn't increase the
|
||||||
|
# unread count, as they're all being sent in the same room. Otherwise, it
|
||||||
|
# should. Therefore, the last call to _check_push_attempt is done in the
|
||||||
|
# caller method.
|
||||||
|
self.helper.send(room_id, body="Hello?", tok=other_access_token)
|
||||||
|
expected_push_attempts += 1
|
||||||
|
|
||||||
|
self._advance_time_and_make_push_succeed(expected_push_attempts)
|
||||||
|
|
||||||
|
self.helper.send(room_id, body="Hello??", tok=other_access_token)
|
||||||
|
expected_push_attempts += 1
|
||||||
|
|
||||||
|
self._advance_time_and_make_push_succeed(expected_push_attempts)
|
||||||
|
|
||||||
|
self.helper.send(room_id, body="HELLO???", tok=other_access_token)
|
||||||
|
|
||||||
|
def _advance_time_and_make_push_succeed(self, expected_push_attempts):
|
||||||
|
self.pump()
|
||||||
|
self.push_attempts[expected_push_attempts - 1][0].callback({})
|
||||||
|
|
||||||
|
def _check_push_attempt(
|
||||||
|
self, expected_push_attempts: int, expected_unread_count_last_push: int
|
||||||
|
) -> None:
|
||||||
|
"""
|
||||||
|
Makes sure that the last expected push attempt succeeds and checks whether
|
||||||
|
it contains the expected unread count.
|
||||||
|
"""
|
||||||
|
self._advance_time_and_make_push_succeed(expected_push_attempts)
|
||||||
|
# Check our push made it
|
||||||
|
self.assertEqual(len(self.push_attempts), expected_push_attempts)
|
||||||
|
_, push_url, push_body = self.push_attempts[expected_push_attempts - 1]
|
||||||
|
self.assertEqual(
|
||||||
|
push_url,
|
||||||
|
"http://example.com/_matrix/push/v1/notify",
|
||||||
|
)
|
||||||
# Check that the unread count for the room is 0
|
# Check that the unread count for the room is 0
|
||||||
#
|
#
|
||||||
# The unread count is zero as the user has no read receipt in the room yet
|
# The unread count is zero as the user has no read receipt in the room yet
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
self.push_attempts[0][2]["notification"]["counts"]["unread"], 0
|
push_body["notification"]["counts"]["unread"],
|
||||||
|
expected_unread_count_last_push,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def _send_read_request(self, access_token, message_event_id, room_id):
|
||||||
# Now set the user's read receipt position to the first event
|
# Now set the user's read receipt position to the first event
|
||||||
#
|
#
|
||||||
# This will actually trigger a new notification to be sent out so that
|
# This will actually trigger a new notification to be sent out so that
|
||||||
@ -667,56 +714,8 @@ class HTTPPusherTests(HomeserverTestCase):
|
|||||||
# count goes down
|
# count goes down
|
||||||
channel = self.make_request(
|
channel = self.make_request(
|
||||||
"POST",
|
"POST",
|
||||||
"/rooms/%s/receipt/m.read/%s" % (room_id, first_message_event_id),
|
"/rooms/%s/receipt/m.read/%s" % (room_id, message_event_id),
|
||||||
{},
|
{},
|
||||||
access_token=access_token,
|
access_token=access_token,
|
||||||
)
|
)
|
||||||
self.assertEqual(channel.code, 200, channel.json_body)
|
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.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(
|
|
||||||
self.push_attempts[2][2]["notification"]["counts"]["unread"], 1
|
|
||||||
)
|
|
||||||
|
|
||||||
# Since we're grouping by room, sending more messages shouldn't increase the
|
|
||||||
# unread count, as they're all being sent in the same room
|
|
||||||
self.helper.send(room_id, body="Hello?", tok=other_access_token)
|
|
||||||
|
|
||||||
# Advance time and make the push succeed
|
|
||||||
self.pump()
|
|
||||||
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[4][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.assertEqual(len(self.push_attempts), 6)
|
|
||||||
|
Loading…
Reference in New Issue
Block a user