From 84819804157b1650772f32f5d1864f70f3ea1cb2 Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Fri, 27 Mar 2026 09:46:09 +0000 Subject: [PATCH 1/6] reorder rooms by read receipts to update most important rooms first --- synapse/handlers/profile.py | 22 +++- tests/handlers/test_profile.py | 230 ++++++++++++++++++++++++++++++++- 2 files changed, 249 insertions(+), 3 deletions(-) diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index c3886795b66..cb8ee7392f3 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -25,7 +25,7 @@ from twisted.internet.defer import CancelledError -from synapse.api.constants import ProfileFields +from synapse.api.constants import ProfileFields, ReceiptTypes from synapse.api.errors import ( AuthError, Codes, @@ -702,7 +702,25 @@ async def _update_join_states_task( assert task.params target_user = UserID.from_string(task.resource_id) - room_ids = sorted(await self.store.get_rooms_for_user(target_user.to_string())) + all_room_ids = await self.store.get_rooms_for_user(target_user.to_string()) + + # Get the user's latest read receipts for all rooms + user_receipts = await self.store.get_receipts_for_user_with_orderings( + target_user.to_string(), + [ReceiptTypes.READ, ReceiptTypes.READ_PRIVATE], + ) + + # Sort rooms by most recent read receipt (highest stream_ordering first), + # with fallback to alphabetical ordering for rooms without receipts + def sort_key(room_id: str) -> tuple[int, str]: + if room_id in user_receipts: + # Rooms with receipts: sort by stream_ordering (descending) then by room_id + return (-user_receipts[room_id]["stream_ordering"], room_id) + else: + # Rooms without receipts: sort alphabetically after all rooms with receipts + return (0, room_id) + + room_ids = sorted(all_room_ids, key=sort_key) last_room_id = task.result.get("last_room_id", None) if task.result else None diff --git a/tests/handlers/test_profile.py b/tests/handlers/test_profile.py index 5152e8fc536..4366f8a1e29 100644 --- a/tests/handlers/test_profile.py +++ b/tests/handlers/test_profile.py @@ -26,7 +26,7 @@ from twisted.internet.testing import MemoryReactor import synapse.types -from synapse.api.constants import EventTypes +from synapse.api.constants import EventTypes, ReceiptTypes from synapse.api.errors import AuthError, SynapseError from synapse.rest import admin from synapse.rest.client import login, room @@ -232,6 +232,8 @@ def test_background_update_room_membership_resume_after_restart(self) -> None: if room_id_1 > room_id_2: room_id_1, room_id_2 = room_id_2, room_id_1 + # Without read receipts, both rooms should be processed in alphabetical order + original_update_membership = self.hs.get_room_member_handler().update_membership room_1_updated = False @@ -315,6 +317,232 @@ async def potentially_slow_update_membership( membership[state_tuple].content["displayname"], "Frank Jr." ) + def test_room_update_ordering_by_read_receipt(self) -> None: + """Test that rooms are updated in order of most recent read receipt.""" + self.get_success( + self.handler.set_displayname( + self.frank, synapse.types.create_requester(self.frank), "Frank" + ) + ) + + # Create three rooms + room_id_1 = self.helper.create_room_as( + self.frank.to_string(), tok=self.frank_token + ) + room_id_2 = self.helper.create_room_as( + self.frank.to_string(), tok=self.frank_token + ) + room_id_3 = self.helper.create_room_as( + self.frank.to_string(), tok=self.frank_token + ) + + # Send an event in each room to create something to mark as read + event_1 = self.helper.send(room_id_1, "Hello 1", tok=self.frank_token) + event_2 = self.helper.send(room_id_2, "Hello 2", tok=self.frank_token) + event_3 = self.helper.send(room_id_3, "Hello 3", tok=self.frank_token) + + # Set read receipts with different timestamps (simulate different read times) + # Room 2 should be most recent, then room 3, then room 1 + self.get_success( + self.store.insert_receipt( + room_id_1, + ReceiptTypes.READ, + user_id=self.frank.to_string(), + event_ids=[event_1["event_id"]], + thread_id=None, + data={"ts": 100}, + ) + ) + self.get_success( + self.store.insert_receipt( + room_id_3, + ReceiptTypes.READ, + user_id=self.frank.to_string(), + event_ids=[event_3["event_id"]], + thread_id=None, + data={"ts": 200}, + ) + ) + self.get_success( + self.store.insert_receipt( + room_id_2, + ReceiptTypes.READ, + user_id=self.frank.to_string(), + event_ids=[event_2["event_id"]], + thread_id=None, + data={"ts": 300}, + ) + ) + + # Track the order in which rooms are updated + room_update_order = [] + original_update_membership = self.hs.get_room_member_handler().update_membership + + async def track_update_membership(*args: Any, **kwargs: Any) -> tuple[str, int]: + room_id = args[2] + room_update_order.append(room_id) + return await original_update_membership(*args, **kwargs) + + with patch.object( + self.hs.get_room_member_handler(), + "update_membership", + side_effect=track_update_membership, + ): + self.get_success( + self.handler.set_displayname( + self.frank, + synapse.types.create_requester(self.frank), + "Frank Updated", + ) + ) + + # Wait for background task to complete + self.get_success(self.clock.sleep(Duration(milliseconds=50)), by=1) + + # Get receipts to understand the actual stream ordering + user_receipts = self.get_success( + self.store.get_receipts_for_user_with_orderings( + self.frank.to_string(), + [ReceiptTypes.READ, ReceiptTypes.READ_PRIVATE], + ) + ) + + # Sort rooms by stream_ordering (descending) to get expected order + rooms_by_stream_ordering = sorted( + user_receipts.keys(), + key=lambda room_id: -user_receipts[room_id]["stream_ordering"], + ) + + # Verify rooms were updated in order of most recent read receipt (highest stream_ordering first) + self.assertEqual(len(room_update_order), 3) + self.assertEqual(room_update_order, rooms_by_stream_ordering) + + def test_room_update_ordering_with_no_receipts_fallback(self) -> None: + """Test that rooms without read receipts fall back to alphabetical ordering.""" + self.get_success( + self.handler.set_displayname( + self.frank, synapse.types.create_requester(self.frank), "Frank" + ) + ) + + # Create two rooms - ensure we know their alphabetical order + room_id_a = self.helper.create_room_as( + self.frank.to_string(), tok=self.frank_token + ) + room_id_b = self.helper.create_room_as( + self.frank.to_string(), tok=self.frank_token + ) + + # Ensure room_id_a comes before room_id_b alphabetically + if room_id_a > room_id_b: + room_id_a, room_id_b = room_id_b, room_id_a + + # Don't set any read receipts - should fall back to alphabetical + + # Track the order in which rooms are updated + room_update_order = [] + original_update_membership = self.hs.get_room_member_handler().update_membership + + async def track_update_membership(*args: Any, **kwargs: Any) -> tuple[str, int]: + room_id = args[2] + room_update_order.append(room_id) + return await original_update_membership(*args, **kwargs) + + with patch.object( + self.hs.get_room_member_handler(), + "update_membership", + side_effect=track_update_membership, + ): + self.get_success( + self.handler.set_displayname( + self.frank, + synapse.types.create_requester(self.frank), + "Frank Updated", + ) + ) + + # Wait for background task to complete + self.get_success(self.clock.sleep(Duration(milliseconds=50)), by=1) + + # Verify rooms were updated in alphabetical order + self.assertEqual(len(room_update_order), 2) + self.assertEqual(room_update_order[0], room_id_a) # Alphabetically first + self.assertEqual(room_update_order[1], room_id_b) # Alphabetically second + + def test_room_update_ordering_mixed_receipts_and_no_receipts(self) -> None: + """Test ordering when some rooms have receipts and others don't.""" + self.get_success( + self.handler.set_displayname( + self.frank, synapse.types.create_requester(self.frank), "Frank" + ) + ) + + # Create three rooms + room_with_receipt = self.helper.create_room_as( + self.frank.to_string(), tok=self.frank_token + ) + room_without_receipt_1 = self.helper.create_room_as( + self.frank.to_string(), tok=self.frank_token + ) + room_without_receipt_2 = self.helper.create_room_as( + self.frank.to_string(), tok=self.frank_token + ) + + # Ensure we know the alphabetical order of rooms without receipts + if room_without_receipt_1 > room_without_receipt_2: + room_without_receipt_1, room_without_receipt_2 = ( + room_without_receipt_2, + room_without_receipt_1, + ) + + # Send an event and set a read receipt for one room only + event = self.helper.send(room_with_receipt, "Hello", tok=self.frank_token) + self.get_success( + self.store.insert_receipt( + room_with_receipt, + ReceiptTypes.READ, + user_id=self.frank.to_string(), + event_ids=[event["event_id"]], + thread_id=None, + data={"ts": 100}, + ) + ) + + # Track the order in which rooms are updated + room_update_order = [] + original_update_membership = self.hs.get_room_member_handler().update_membership + + async def track_update_membership(*args: Any, **kwargs: Any) -> tuple[str, int]: + room_id = args[2] + room_update_order.append(room_id) + return await original_update_membership(*args, **kwargs) + + with patch.object( + self.hs.get_room_member_handler(), + "update_membership", + side_effect=track_update_membership, + ): + self.get_success( + self.handler.set_displayname( + self.frank, + synapse.types.create_requester(self.frank), + "Frank Updated", + ) + ) + + # Wait for background task to complete + self.get_success(self.clock.sleep(Duration(milliseconds=50)), by=1) + + # Verify ordering: room with receipt first, then others alphabetically + self.assertEqual(len(room_update_order), 3) + self.assertEqual(room_update_order[0], room_with_receipt) # Has receipt - first + self.assertEqual( + room_update_order[1], room_without_receipt_1 + ) # No receipt - alphabetically first + self.assertEqual( + room_update_order[2], room_without_receipt_2 + ) # No receipt - alphabetically second + @override_config({"enable_set_displayname": False}) def test_set_my_name_if_disabled(self) -> None: # Setting displayname for the first time is allowed From d588a1f9c3ef867491636ce0eb2be0f4cc94e639 Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Fri, 27 Mar 2026 09:51:02 +0000 Subject: [PATCH 2/6] changelog --- changelog.d/19613.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/19613.feature diff --git a/changelog.d/19613.feature b/changelog.d/19613.feature new file mode 100644 index 00000000000..9cb0d9917dd --- /dev/null +++ b/changelog.d/19613.feature @@ -0,0 +1 @@ +Reorder rooms by read receipts to update most important rooms first. From 7b140e402ab988358d7539fb2b7c38ab424b0861 Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Mon, 13 Apr 2026 09:23:58 +0100 Subject: [PATCH 3/6] Store precomputed room ordering in profile update tasks --- synapse/handlers/profile.py | 48 ++++++++++++++++-------------- tests/handlers/test_profile.py | 53 ++++++++++++++++++++++++++++++---- 2 files changed, 74 insertions(+), 27 deletions(-) diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index cb8ee7392f3..983ee8edea3 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -20,7 +20,6 @@ # import logging import random -from bisect import bisect_right from typing import TYPE_CHECKING from twisted.internet.defer import CancelledError @@ -671,6 +670,27 @@ async def _update_join_states( target_user_str = target_user.to_string() + # Compute the ordered list of rooms upfront to ensure consistency across restarts + all_room_ids = await self.store.get_rooms_for_user(target_user_str) + + # Get the user's latest read receipts for all rooms + user_receipts = await self.store.get_receipts_for_user_with_orderings( + target_user_str, + [ReceiptTypes.READ, ReceiptTypes.READ_PRIVATE], + ) + + # Sort rooms by most recent read receipt (highest stream_ordering first), + # with fallback to alphabetical ordering for rooms without receipts + def sort_key(room_id: str) -> tuple[int, str]: + if room_id in user_receipts: + # Rooms with receipts: sort by stream_ordering (descending) then by room_id + return (-user_receipts[room_id]["stream_ordering"], room_id) + else: + # Rooms without receipts: sort alphabetically after all rooms with receipts + return (0, room_id) + + room_ids = sorted(all_room_ids, key=sort_key) + # Cancel any ongoing profile membership updates for this user, # and start a new one. async with self._worker_locks.acquire_lock( @@ -691,6 +711,7 @@ async def _update_join_states( resource_id=target_user_str, params={ "requester_authenticated_entity": requester.authenticated_entity, + "ordered_room_ids": room_ids, }, ) @@ -702,33 +723,16 @@ async def _update_join_states_task( assert task.params target_user = UserID.from_string(task.resource_id) - all_room_ids = await self.store.get_rooms_for_user(target_user.to_string()) - - # Get the user's latest read receipts for all rooms - user_receipts = await self.store.get_receipts_for_user_with_orderings( - target_user.to_string(), - [ReceiptTypes.READ, ReceiptTypes.READ_PRIVATE], - ) - - # Sort rooms by most recent read receipt (highest stream_ordering first), - # with fallback to alphabetical ordering for rooms without receipts - def sort_key(room_id: str) -> tuple[int, str]: - if room_id in user_receipts: - # Rooms with receipts: sort by stream_ordering (descending) then by room_id - return (-user_receipts[room_id]["stream_ordering"], room_id) - else: - # Rooms without receipts: sort alphabetically after all rooms with receipts - return (0, room_id) - room_ids = sorted(all_room_ids, key=sort_key) + # Use the precomputed room ordering from task params to ensure consistency + room_ids = task.params.get("ordered_room_ids", []) last_room_id = task.result.get("last_room_id", None) if task.result else None if last_room_id: # Filter out room IDs that have already been handled - # by finding the first room ID greater than the last handled room ID - # and slicing the list from that point onwards. - room_ids = room_ids[bisect_right(room_ids, last_room_id) :] + last_index = room_ids.index(last_room_id, 0) + room_ids = room_ids[last_index + 1 :] requester = create_requester( user_id=target_user, diff --git a/tests/handlers/test_profile.py b/tests/handlers/test_profile.py index 4366f8a1e29..11ee3661086 100644 --- a/tests/handlers/test_profile.py +++ b/tests/handlers/test_profile.py @@ -227,12 +227,46 @@ def test_background_update_room_membership_resume_after_restart(self) -> None: room_id_2 = self.helper.create_room_as( self.frank.to_string(), tok=self.frank_token ) + room_id_3 = self.helper.create_room_as( + self.frank.to_string(), tok=self.frank_token + ) - # Ensure `room_id_1` comes before `room_id_2` alphabetically - if room_id_1 > room_id_2: - room_id_1, room_id_2 = room_id_2, room_id_1 - # Without read receipts, both rooms should be processed in alphabetical order + # Set read receipts with different timestamps (simulate different read times) + # Room 1 should be most recent, then room 2, then room 3 + event_3 = self.helper.send(room_id_3, "Hello 3", tok=self.frank_token) + event_2 = self.helper.send(room_id_2, "Hello 2", tok=self.frank_token) + event_1 = self.helper.send(room_id_1, "Hello 1", tok=self.frank_token) + self.get_success( + self.store.insert_receipt( + room_id_3, + ReceiptTypes.READ, + user_id=self.frank.to_string(), + event_ids=[event_3["event_id"]], + thread_id=None, + data={"ts": 100}, + ) + ) + self.get_success( + self.store.insert_receipt( + room_id_2, + ReceiptTypes.READ, + user_id=self.frank.to_string(), + event_ids=[event_2["event_id"]], + thread_id=None, + data={"ts": 200}, + ) + ) + self.get_success( + self.store.insert_receipt( + room_id_1, + ReceiptTypes.READ, + user_id=self.frank.to_string(), + event_ids=[event_1["event_id"]], + thread_id=None, + data={"ts": 300}, + ) + ) original_update_membership = self.hs.get_room_member_handler().update_membership @@ -241,7 +275,7 @@ def test_background_update_room_membership_resume_after_restart(self) -> None: async def potentially_slow_update_membership( *args: Any, **kwargs: Any ) -> tuple[str, int]: - if args[2] == room_id_2: + if args[2] == room_id_2 or args[2] == room_id_3: await self.clock.sleep(Duration(milliseconds=10)) if args[2] == room_id_1: nonlocal room_1_updated @@ -316,6 +350,15 @@ async def potentially_slow_update_membership( self.assertEqual( membership[state_tuple].content["displayname"], "Frank Jr." ) + membership = self.get_success( + self.storage_controllers.state.get_current_state( + room_id_3, StateFilter.from_types([state_tuple]) + ) + ) + self.assertEqual( + membership[state_tuple].content["displayname"], "Frank Jr." + ) + def test_room_update_ordering_by_read_receipt(self) -> None: """Test that rooms are updated in order of most recent read receipt.""" From eed642038513602c758c13be0490d1e48c18e1dd Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Mon, 13 Apr 2026 09:39:54 +0100 Subject: [PATCH 4/6] fix confusing message ordering and comments in test_room_update_ordering_by_read_receipt --- tests/handlers/test_profile.py | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/tests/handlers/test_profile.py b/tests/handlers/test_profile.py index 11ee3661086..c335a7e1cb5 100644 --- a/tests/handlers/test_profile.py +++ b/tests/handlers/test_profile.py @@ -213,10 +213,13 @@ async def slow_update_membership(*args: Any, **kwargs: Any) -> tuple[str, int]: def test_background_update_room_membership_resume_after_restart(self) -> None: """Test that room membership updates triggered by changing the avatar or the display name are resumed after a restart.""" - + initial_displayname = "Frank" + updated_displayname = "Frank Jr." self.get_success( self.handler.set_displayname( - self.frank, synapse.types.create_requester(self.frank), "Frank" + self.frank, + synapse.types.create_requester(self.frank), + initial_displayname, ) ) @@ -231,7 +234,6 @@ def test_background_update_room_membership_resume_after_restart(self) -> None: self.frank.to_string(), tok=self.frank_token ) - # Set read receipts with different timestamps (simulate different read times) # Room 1 should be most recent, then room 2, then room 3 event_3 = self.helper.send(room_id_3, "Hello 3", tok=self.frank_token) @@ -290,7 +292,9 @@ async def potentially_slow_update_membership( state_tuple = (EventTypes.Member, self.frank.to_string()) self.get_success( self.handler.set_displayname( - self.frank, synapse.types.create_requester(self.frank), "Frank Jr." + self.frank, + synapse.types.create_requester(self.frank), + updated_displayname, ) ) @@ -301,7 +305,7 @@ async def potentially_slow_update_membership( ) ) self.assertEqual( - membership[state_tuple].content["displayname"], "Frank Jr." + membership[state_tuple].content["displayname"], updated_displayname ) # Simulate a synapse restart by emptying the list of running tasks @@ -321,7 +325,9 @@ async def potentially_slow_update_membership( room_id_2, StateFilter.from_types([state_tuple]) ) ) - self.assertEqual(membership[state_tuple].content["displayname"], "Frank") + self.assertEqual( + membership[state_tuple].content["displayname"], initial_displayname + ) cancelled_task = self.get_success( self.task_scheduler.get_tasks( @@ -348,7 +354,7 @@ async def potentially_slow_update_membership( ) ) self.assertEqual( - membership[state_tuple].content["displayname"], "Frank Jr." + membership[state_tuple].content["displayname"], updated_displayname ) membership = self.get_success( self.storage_controllers.state.get_current_state( @@ -356,10 +362,9 @@ async def potentially_slow_update_membership( ) ) self.assertEqual( - membership[state_tuple].content["displayname"], "Frank Jr." + membership[state_tuple].content["displayname"], updated_displayname ) - def test_room_update_ordering_by_read_receipt(self) -> None: """Test that rooms are updated in order of most recent read receipt.""" self.get_success( @@ -385,7 +390,7 @@ def test_room_update_ordering_by_read_receipt(self) -> None: event_3 = self.helper.send(room_id_3, "Hello 3", tok=self.frank_token) # Set read receipts with different timestamps (simulate different read times) - # Room 2 should be most recent, then room 3, then room 1 + # Room 3 is the most recent, then room 2, then room 1 self.get_success( self.store.insert_receipt( room_id_1, @@ -398,20 +403,20 @@ def test_room_update_ordering_by_read_receipt(self) -> None: ) self.get_success( self.store.insert_receipt( - room_id_3, + room_id_2, ReceiptTypes.READ, user_id=self.frank.to_string(), - event_ids=[event_3["event_id"]], + event_ids=[event_2["event_id"]], thread_id=None, data={"ts": 200}, ) ) self.get_success( self.store.insert_receipt( - room_id_2, + room_id_3, ReceiptTypes.READ, user_id=self.frank.to_string(), - event_ids=[event_2["event_id"]], + event_ids=[event_3["event_id"]], thread_id=None, data={"ts": 300}, ) From b9043d4a9ff58f5df5df47ff67b3df7ff5c3576e Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Mon, 13 Apr 2026 09:44:09 +0100 Subject: [PATCH 5/6] use variables for displaynames test_room_update_ordering_by_read_receipt --- tests/handlers/test_profile.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/handlers/test_profile.py b/tests/handlers/test_profile.py index c335a7e1cb5..a0f98f68ff4 100644 --- a/tests/handlers/test_profile.py +++ b/tests/handlers/test_profile.py @@ -367,9 +367,13 @@ async def potentially_slow_update_membership( def test_room_update_ordering_by_read_receipt(self) -> None: """Test that rooms are updated in order of most recent read receipt.""" + initial_displayname = "Frank" + updated_displayname = "Frank Jr." self.get_success( self.handler.set_displayname( - self.frank, synapse.types.create_requester(self.frank), "Frank" + self.frank, + synapse.types.create_requester(self.frank), + initial_displayname, ) ) @@ -440,7 +444,7 @@ async def track_update_membership(*args: Any, **kwargs: Any) -> tuple[str, int]: self.handler.set_displayname( self.frank, synapse.types.create_requester(self.frank), - "Frank Updated", + updated_displayname, ) ) From 052c11ca68c3bd8f5d9e9ed68b9bdd856d063a2b Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Mon, 13 Apr 2026 10:30:45 +0100 Subject: [PATCH 6/6] extract helper method _setup_displayname_and_rooms to set up room and display name --- tests/handlers/test_profile.py | 169 ++++++++++++--------------------- 1 file changed, 63 insertions(+), 106 deletions(-) diff --git a/tests/handlers/test_profile.py b/tests/handlers/test_profile.py index a0f98f68ff4..266d446a7b1 100644 --- a/tests/handlers/test_profile.py +++ b/tests/handlers/test_profile.py @@ -215,59 +215,9 @@ def test_background_update_room_membership_resume_after_restart(self) -> None: """Test that room membership updates triggered by changing the avatar or the display name are resumed after a restart.""" initial_displayname = "Frank" updated_displayname = "Frank Jr." - self.get_success( - self.handler.set_displayname( - self.frank, - synapse.types.create_requester(self.frank), - initial_displayname, - ) - ) - - room_id_1 = self.helper.create_room_as( - self.frank.to_string(), tok=self.frank_token - ) - room_id_2 = self.helper.create_room_as( - self.frank.to_string(), tok=self.frank_token - ) - room_id_3 = self.helper.create_room_as( - self.frank.to_string(), tok=self.frank_token - ) - - # Set read receipts with different timestamps (simulate different read times) - # Room 1 should be most recent, then room 2, then room 3 - event_3 = self.helper.send(room_id_3, "Hello 3", tok=self.frank_token) - event_2 = self.helper.send(room_id_2, "Hello 2", tok=self.frank_token) - event_1 = self.helper.send(room_id_1, "Hello 1", tok=self.frank_token) - self.get_success( - self.store.insert_receipt( - room_id_3, - ReceiptTypes.READ, - user_id=self.frank.to_string(), - event_ids=[event_3["event_id"]], - thread_id=None, - data={"ts": 100}, - ) - ) - self.get_success( - self.store.insert_receipt( - room_id_2, - ReceiptTypes.READ, - user_id=self.frank.to_string(), - event_ids=[event_2["event_id"]], - thread_id=None, - data={"ts": 200}, - ) - ) - self.get_success( - self.store.insert_receipt( - room_id_1, - ReceiptTypes.READ, - user_id=self.frank.to_string(), - event_ids=[event_1["event_id"]], - thread_id=None, - data={"ts": 300}, - ) + room_id_1, room_id_2, room_id_3 = self._setup_displayname_and_rooms( + initial_displayname ) original_update_membership = self.hs.get_room_member_handler().update_membership @@ -369,6 +319,58 @@ def test_room_update_ordering_by_read_receipt(self) -> None: """Test that rooms are updated in order of most recent read receipt.""" initial_displayname = "Frank" updated_displayname = "Frank Jr." + + self._setup_displayname_and_rooms(initial_displayname) + + # Track the order in which rooms are updated + room_update_order = [] + original_update_membership = self.hs.get_room_member_handler().update_membership + + async def track_update_membership(*args: Any, **kwargs: Any) -> tuple[str, int]: + room_id = args[2] + room_update_order.append(room_id) + return await original_update_membership(*args, **kwargs) + + with patch.object( + self.hs.get_room_member_handler(), + "update_membership", + side_effect=track_update_membership, + ): + self.get_success( + self.handler.set_displayname( + self.frank, + synapse.types.create_requester(self.frank), + updated_displayname, + ) + ) + + # Wait for background task to complete + self.get_success(self.clock.sleep(Duration(milliseconds=50)), by=1) + + # Get receipts to understand the actual stream ordering + user_receipts = self.get_success( + self.store.get_receipts_for_user_with_orderings( + self.frank.to_string(), + [ReceiptTypes.READ, ReceiptTypes.READ_PRIVATE], + ) + ) + + # Sort rooms by stream_ordering (descending) to get expected order + rooms_by_stream_ordering = sorted( + user_receipts.keys(), + key=lambda room_id: -user_receipts[room_id]["stream_ordering"], + ) + + # Verify rooms were updated in order of most recent read receipt (highest stream_ordering first) + self.assertEqual(len(room_update_order), 3) + self.assertEqual(room_update_order, rooms_by_stream_ordering) + + def _setup_displayname_and_rooms( + self, initial_displayname: str + ) -> tuple[str, str, str]: + """Helper to set up initial displayname and create three rooms. + Returns tuples of (room_id_1, room_id_2, room_id_3) where room_id_1 + is the room with the most recent read receipt, then room_id_2, then room_id_3.""" self.get_success( self.handler.set_displayname( self.frank, @@ -387,20 +389,17 @@ def test_room_update_ordering_by_read_receipt(self) -> None: room_id_3 = self.helper.create_room_as( self.frank.to_string(), tok=self.frank_token ) - - # Send an event in each room to create something to mark as read - event_1 = self.helper.send(room_id_1, "Hello 1", tok=self.frank_token) - event_2 = self.helper.send(room_id_2, "Hello 2", tok=self.frank_token) - event_3 = self.helper.send(room_id_3, "Hello 3", tok=self.frank_token) - # Set read receipts with different timestamps (simulate different read times) - # Room 3 is the most recent, then room 2, then room 1 + # Room 1 should be most recent, then room 2, then room 3 + event_3 = self.helper.send(room_id_3, "Hello 3", tok=self.frank_token) + event_2 = self.helper.send(room_id_2, "Hello 2", tok=self.frank_token) + event_1 = self.helper.send(room_id_1, "Hello 1", tok=self.frank_token) self.get_success( self.store.insert_receipt( - room_id_1, + room_id_3, ReceiptTypes.READ, user_id=self.frank.to_string(), - event_ids=[event_1["event_id"]], + event_ids=[event_3["event_id"]], thread_id=None, data={"ts": 100}, ) @@ -417,57 +416,15 @@ def test_room_update_ordering_by_read_receipt(self) -> None: ) self.get_success( self.store.insert_receipt( - room_id_3, + room_id_1, ReceiptTypes.READ, user_id=self.frank.to_string(), - event_ids=[event_3["event_id"]], + event_ids=[event_1["event_id"]], thread_id=None, data={"ts": 300}, ) ) - - # Track the order in which rooms are updated - room_update_order = [] - original_update_membership = self.hs.get_room_member_handler().update_membership - - async def track_update_membership(*args: Any, **kwargs: Any) -> tuple[str, int]: - room_id = args[2] - room_update_order.append(room_id) - return await original_update_membership(*args, **kwargs) - - with patch.object( - self.hs.get_room_member_handler(), - "update_membership", - side_effect=track_update_membership, - ): - self.get_success( - self.handler.set_displayname( - self.frank, - synapse.types.create_requester(self.frank), - updated_displayname, - ) - ) - - # Wait for background task to complete - self.get_success(self.clock.sleep(Duration(milliseconds=50)), by=1) - - # Get receipts to understand the actual stream ordering - user_receipts = self.get_success( - self.store.get_receipts_for_user_with_orderings( - self.frank.to_string(), - [ReceiptTypes.READ, ReceiptTypes.READ_PRIVATE], - ) - ) - - # Sort rooms by stream_ordering (descending) to get expected order - rooms_by_stream_ordering = sorted( - user_receipts.keys(), - key=lambda room_id: -user_receipts[room_id]["stream_ordering"], - ) - - # Verify rooms were updated in order of most recent read receipt (highest stream_ordering first) - self.assertEqual(len(room_update_order), 3) - self.assertEqual(room_update_order, rooms_by_stream_ordering) + return room_id_1, room_id_2, room_id_3 def test_room_update_ordering_with_no_receipts_fallback(self) -> None: """Test that rooms without read receipts fall back to alphabetical ordering."""