Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Fix skipping items when paginating /relations forward. #13840

Merged
merged 11 commits into from
Sep 22, 2022
3 changes: 3 additions & 0 deletions changelog.d/13840.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix a bug introduced in Synapse v1.53.0 where the experimental implementation of
[MSC3715](https://github.com/matrix-org/matrix-spec-proposals/pull/3715) would give incorrect results when paginating
forward.
6 changes: 6 additions & 0 deletions synapse/storage/databases/main/relations.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,12 @@ def _get_recent_references_for_event_txn(
# If there are more events, generate the next pagination key.
next_token = None
if len(events) > limit and last_topo_id and last_stream_id:
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved
# Due to how the pagination clause is generated, the stream ID is
# handled as an exclusive range when paginating forward. Correct
# for that here.
clokep marked this conversation as resolved.
Show resolved Hide resolved
if direction == "f":
last_stream_id -= 1

next_key = RoomStreamToken(last_topo_id, last_stream_id)
if from_token:
next_token = from_token.copy_and_replace(
Expand Down
29 changes: 28 additions & 1 deletion tests/rest/client/test_relations.py
Original file line number Diff line number Diff line change
Expand Up @@ -788,6 +788,7 @@ def test_basic_paginate_relations(self) -> None:
channel.json_body["chunk"][0],
)

@unittest.override_config({"experimental_features": {"msc3715_enabled": True}})
def test_repeated_paginate_relations(self) -> None:
"""Test that if we paginate using a limit and tokens then we get the
expected events.
Expand All @@ -809,7 +810,7 @@ def test_repeated_paginate_relations(self) -> None:

channel = self.make_request(
"GET",
f"/_matrix/client/v1/rooms/{self.room}/relations/{self.parent_id}?limit=1{from_token}",
f"/_matrix/client/v1/rooms/{self.room}/relations/{self.parent_id}?limit=3{from_token}",
access_token=self.user_token,
)
self.assertEqual(200, channel.code, channel.json_body)
Expand All @@ -827,6 +828,32 @@ def test_repeated_paginate_relations(self) -> None:
found_event_ids.reverse()
self.assertEqual(found_event_ids, expected_event_ids)

# Test forward pagination.
prev_token = ""
found_event_ids = []
for _ in range(20):
from_token = ""
if prev_token:
from_token = "&from=" + prev_token

channel = self.make_request(
"GET",
f"/_matrix/client/v1/rooms/{self.room}/relations/{self.parent_id}?org.matrix.msc3715.dir=f&limit=3{from_token}",
access_token=self.user_token,
)
self.assertEqual(200, channel.code, channel.json_body)

found_event_ids.extend(e["event_id"] for e in channel.json_body["chunk"])
next_batch = channel.json_body.get("next_batch")

self.assertNotEqual(prev_token, next_batch)
prev_token = next_batch

if not prev_token:
break

self.assertEqual(found_event_ids, expected_event_ids)

def test_pagination_from_sync_and_messages(self) -> None:
"""Pagination tokens from /sync and /messages can be used to paginate /relations."""
channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "A")
Expand Down