From d32cd70abde520fe5e0ba9ff00f7796753a52a5c Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Thu, 28 Aug 2025 14:59:40 +0100 Subject: [PATCH] ensure that the rooms are updated according to most recent read receipt, so that the lag in updating all rooms is less noticable --- synapse/handlers/profile.py | 6 +- synapse/storage/databases/main/roommember.py | 34 ++++++ tests/storage/test_roommember.py | 116 +++++++++++++++++++ 3 files changed, 153 insertions(+), 3 deletions(-) diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index 519be9e800..cbf222e850 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -624,9 +624,9 @@ class ProfileHandler: await self.clock.sleep(random.randint(1, 10)) return TaskStatus.COMPLETE, None, None - room_ids = await self.store.get_rooms_for_user(target_user.to_string()) - # TODO order list based on some sort of recent usage heuristic so that the fact - # the changes are not happening instantly is less obvious. + room_ids = await self.store.get_rooms_for_user_by_read_receipts( + target_user.to_string() + ) for room_id in room_ids: handler = self.hs.get_room_member_handler() try: diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 67e7e99baa..e7556930fa 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -769,6 +769,40 @@ class RoomMemberWorkerStore(EventsWorkerStore, CacheInvalidationWorkerStore): return frozenset(room_ids) + async def get_rooms_for_user_by_read_receipts(self, user_id: str) -> List[str]: + """Returns room_ids ordered by most recent read receipt activity. + + Rooms with recent read receipts appear first (most recent first), + rooms without read receipts appear after in arbitrary order. + + Args: + user_id: The ID of the user. + + Returns: + List of room_ids ordered by read receipt activity. + """ + + def _get_rooms_txn(txn: LoggingTransaction) -> List[str]: + sql = """ + SELECT cse.room_id + FROM current_state_events cse + LEFT JOIN receipts_linearized rl ON ( + rl.room_id = cse.room_id + AND rl.user_id = ? + AND rl.receipt_type = 'm.read' + ) + WHERE cse.type = 'm.room.member' + AND cse.membership = 'join' + AND cse.state_key = ? + ORDER BY rl.event_stream_ordering DESC NULLS LAST, cse.room_id + """ + txn.execute(sql, (user_id, user_id)) + return [row[0] for row in txn.fetchall()] + + return await self.db_pool.runInteraction( + "get_rooms_for_user_by_read_receipts", _get_rooms_txn + ) + @cachedList( cached_method_name="get_rooms_for_user", list_name="user_ids", diff --git a/tests/storage/test_roommember.py b/tests/storage/test_roommember.py index fd489022a8..51554af282 100644 --- a/tests/storage/test_roommember.py +++ b/tests/storage/test_roommember.py @@ -810,3 +810,119 @@ class CurrentStateMembershipUpdateTestCase(unittest.HomeserverTestCase): # Now let's actually drive the updates to completion self.wait_for_background_updates() + + +class RoomMemberReadReceiptOrderingTestCase(unittest.HomeserverTestCase): + """Tests for ordering rooms by read receipt activity in get_rooms_for_user_by_read_receipts""" + + def prepare( + self, reactor: MemoryReactor, clock: Clock, homeserver: HomeServer + ) -> None: + super().prepare(reactor, clock, homeserver) + self.store = homeserver.get_datastores().main + self.room_creator = homeserver.get_room_creation_handler() + + persist_event_storage_controller = self.hs.get_storage_controllers().persistence + assert persist_event_storage_controller is not None + self.persist_event_storage_controller = persist_event_storage_controller + # Create test users and rooms in prepare() + self.alice = UserID("alice", "test") + self.alice_id = self.alice.to_string() + self.alice_requester = create_requester(self.alice) + + # Create test rooms + self.room1, _, _ = self.get_success( + self.room_creator.create_room(self.alice_requester, {}), by=1.0 + ) + self.room2, _, _ = self.get_success( + self.room_creator.create_room(self.alice_requester, {}), by=1.0 + ) + self.room3, _, _ = self.get_success( + self.room_creator.create_room(self.alice_requester, {}), by=1.0 + ) + + def test_get_rooms_for_user_by_read_receipts_ordering(self) -> None: + """Test that rooms are ordered by read receipt recency.""" + # Create events in each room to have something to point read receipts at + event1 = self.get_success( + create_event( + self.hs, + room_id=self.room1, + type="m.room.message", + sender=self.alice_id, + content={"msgtype": "m.text", "body": "Message 1"}, + ) + ) + self.get_success( + self.persist_event_storage_controller.persist_event(event1[0], event1[1]) + ) + + event2 = self.get_success( + create_event( + self.hs, + room_id=self.room2, + type="m.room.message", + sender=self.alice_id, + content={"msgtype": "m.text", "body": "Message 2"}, + ) + ) + self.get_success( + self.persist_event_storage_controller.persist_event(event2[0], event2[1]) + ) + + event3 = self.get_success( + create_event( + self.hs, + room_id=self.room3, + type="m.room.message", + sender=self.alice_id, + content={"msgtype": "m.text", "body": "Message 3"}, + ) + ) + self.get_success( + self.persist_event_storage_controller.persist_event(event3[0], event3[1]) + ) + + # Insert read receipts with different timestamps + # room2 should be first (most recent: stream_ordering ~300) + self.get_success( + self.store.insert_receipt( + self.room2, "m.read", self.alice_id, [event2[0].event_id], None, {} + ) + ) + + # room1 should be second (older: stream_ordering ~100) + # Advance clock to ensure different timestamp + self.reactor.advance(1000) + self.get_success( + self.store.insert_receipt( + self.room1, "m.read", self.alice_id, [event1[0].event_id], None, {} + ) + ) + + # room3 has no read receipt, should be last + + # Test the ordering + room_ids = self.get_success( + self.store.get_rooms_for_user_by_read_receipts(self.alice_id) + ) + + # room2 should be first (first receipt, higher stream_ordering) + # room1 should be second (second receipt, lower stream_ordering) + # room3 should be last (no receipt) + self.assertEqual(len(room_ids), 3) + self.assertEqual(room_ids[0], self.room2) # Most recent receipt + self.assertEqual(room_ids[1], self.room1) # Older receipt + self.assertEqual(room_ids[2], self.room3) # No receipt + + def test_get_rooms_for_user_by_read_receipts_no_receipts(self) -> None: + """Test that rooms without read receipts are returned in deterministic order.""" + # Use alice's existing rooms but don't add any read receipts + room_ids = self.get_success( + self.store.get_rooms_for_user_by_read_receipts(self.alice_id) + ) + + # Should return all rooms in deterministic order (by room_id since no receipts) + self.assertEqual(len(room_ids), 3) + expected_order = sorted([self.room1, self.room2, self.room3]) + self.assertEqual(room_ids, expected_order)