From aecf8433faa8f61e35741c8b7f1880cf5a25dffe Mon Sep 17 00:00:00 2001 From: SpiritCroc Date: Sun, 10 Mar 2024 13:21:50 +0100 Subject: [PATCH 1/2] Do not refuse to set read_marker if previous is in wrong room When the old read marker location happens to be an event_id in a different room, we should not let this prevent a new read marker from being set. --- changelog.d/16990.bugfix | 1 + synapse/handlers/read_marker.py | 4 ++-- synapse/storage/databases/main/events_worker.py | 8 +++++--- 3 files changed, 8 insertions(+), 5 deletions(-) create mode 100644 changelog.d/16990.bugfix diff --git a/changelog.d/16990.bugfix b/changelog.d/16990.bugfix new file mode 100644 index 00000000000..76f9dd2e36a --- /dev/null +++ b/changelog.d/16990.bugfix @@ -0,0 +1 @@ +Fix case in which `m.fully_read` marker would not get updated. Contributed by @SpiritCroc. diff --git a/synapse/handlers/read_marker.py b/synapse/handlers/read_marker.py index 135a662267c..fb39c8e04b5 100644 --- a/synapse/handlers/read_marker.py +++ b/synapse/handlers/read_marker.py @@ -55,12 +55,12 @@ async def received_client_read_marker( should_update = True # Get event ordering, this also ensures we know about the event - event_ordering = await self.store.get_event_ordering(event_id) + event_ordering = await self.store.get_event_ordering(event_id, room_id) if existing_read_marker: try: old_event_ordering = await self.store.get_event_ordering( - existing_read_marker["event_id"] + existing_read_marker["event_id"], room_id ) except SynapseError: # Old event no longer exists, assume new is ahead. This may diff --git a/synapse/storage/databases/main/events_worker.py b/synapse/storage/databases/main/events_worker.py index 9c3775bb7c4..72b97d50526 100644 --- a/synapse/storage/databases/main/events_worker.py +++ b/synapse/storage/databases/main/events_worker.py @@ -1997,16 +1997,18 @@ def get_deltas_for_stream_id_txn( return rows, to_token, True @cached(max_entries=5000) - async def get_event_ordering(self, event_id: str) -> Tuple[int, int]: + async def get_event_ordering(self, event_id: str, room_id: str) -> Tuple[int, int]: res = await self.db_pool.simple_select_one( table="events", retcols=["topological_ordering", "stream_ordering"], - keyvalues={"event_id": event_id}, + keyvalues={"event_id": event_id, "room_id": room_id}, allow_none=True, ) if not res: - raise SynapseError(404, "Could not find event %s" % (event_id,)) + raise SynapseError( + 404, "Could not find event %s in room %s" % (event_id, room_id) + ) return int(res[0]), int(res[1]) From fb26d59ab64c901b5c1705dc6b53404e5b37395a Mon Sep 17 00:00:00 2001 From: SpiritCroc Date: Sun, 10 Mar 2024 14:18:14 +0100 Subject: [PATCH 2/2] Fix read marker test cases using wrong room_id --- tests/rest/client/test_read_marker.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/rest/client/test_read_marker.py b/tests/rest/client/test_read_marker.py index 2fe350e1e85..0b4ad685b3c 100644 --- a/tests/rest/client/test_read_marker.py +++ b/tests/rest/client/test_read_marker.py @@ -78,7 +78,7 @@ def send_message() -> str: channel = self.make_request( "POST", - "/rooms/!abc:beep/read_markers", + f"/rooms/{room_id}/read_markers", content={ "m.fully_read": event_id_1, }, @@ -90,7 +90,7 @@ def send_message() -> str: event_id_2 = send_message() channel = self.make_request( "POST", - "/rooms/!abc:beep/read_markers", + f"/rooms/{room_id}/read_markers", content={ "m.fully_read": event_id_2, }, @@ -123,7 +123,7 @@ def send_message() -> str: channel = self.make_request( "POST", - "/rooms/!abc:beep/read_markers", + f"/rooms/{room_id}/read_markers", content={ "m.fully_read": event_id_1, }, @@ -142,7 +142,7 @@ def send_message() -> str: event_id_2 = send_message() channel = self.make_request( "POST", - "/rooms/!abc:beep/read_markers", + f"/rooms/{room_id}/read_markers", content={ "m.fully_read": event_id_2, },