Do not refuse to set read_marker if previous event_id is in wrong room (#16990)

4 files changed, 12 insertions, 9 deletions

diff --git a/changelog.d/16990.bugfix b/changelog.d/16990.bugfix
new file mode 100644

index 0000000000..76f9dd2e36
--- /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 135a662267..fb39c8e04b 100644
--- a/synapse/handlers/read_marker.py
+++ b/synapse/handlers/read_marker.py
@@ -55,12 +55,12 @@ class ReadMarkerHandler:
 
             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 81fccfbccb..e39d4b9624 100644
--- a/synapse/storage/databases/main/events_worker.py
+++ b/synapse/storage/databases/main/events_worker.py
@@ -1995,16 +1995,18 @@ class EventsWorkerStore(SQLBaseStore):
         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])
 
diff --git a/tests/rest/client/test_read_marker.py b/tests/rest/client/test_read_marker.py

index 2fe350e1e8..0b4ad685b3 100644
--- a/tests/rest/client/test_read_marker.py
+++ b/tests/rest/client/test_read_marker.py
@@ -78,7 +78,7 @@ class ReadMarkerTestCase(unittest.HomeserverTestCase):
 
         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 @@ class ReadMarkerTestCase(unittest.HomeserverTestCase):
         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 @@ class ReadMarkerTestCase(unittest.HomeserverTestCase):
 
         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 @@ class ReadMarkerTestCase(unittest.HomeserverTestCase):
         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,
             },