diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index 5f2cec2366..2c2eb6c753 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -25,7 +25,7 @@ from typing import TYPE_CHECKING 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, @@ -906,7 +906,25 @@ class ProfileHandler: 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 c53c04fbc8..e439f5a0c7 100644 --- a/tests/handlers/test_profile.py +++ b/tests/handlers/test_profile.py @@ -30,6 +30,7 @@ from synapse.api.constants import ( EventTypes, ProfileFields, ProfileUpdateAction, + ReceiptTypes, ) from synapse.api.errors import AuthError, SynapseError from synapse.rest import admin @@ -768,6 +769,8 @@ class ProfileTestCase(unittest.HomeserverTestCase): 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 @@ -856,6 +859,232 @@ class ProfileTestCase(unittest.HomeserverTestCase): 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