Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rooms.bump_stamp to Sliding Sync /sync for easier client-side sorting #17395

Merged
merged 16 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/17395.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Sort by and add `rooms.bump_stamp` for easier client-side sorting in experimental [MSC3575](https://github.com/matrix-org/matrix-spec-proposals/pull/3575) Sliding Sync `/sync` endpoint.
4 changes: 4 additions & 0 deletions synapse/api/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,13 @@ class EventTypes:
SpaceParent: Final = "m.space.parent"

Reaction: Final = "m.reaction"
Sticker: Final = "m.sticker"
LocationShare: Final = "m.location"
MadLittleMods marked this conversation as resolved.
Show resolved Hide resolved

CallInvite: Final = "m.call.invite"

PollStart: Final = "m.poll.start"


class ToDeviceEventTypes:
RoomKeyRequest: Final = "m.room_key_request"
Expand Down
187 changes: 143 additions & 44 deletions synapse/handlers/sliding_sync.py

Large diffs are not rendered by default.

35 changes: 24 additions & 11 deletions synapse/storage/databases/main/stream.py
Original file line number Diff line number Diff line change
Expand Up @@ -1178,6 +1178,7 @@ async def get_last_event_pos_in_room_before_stream_ordering(
self,
room_id: str,
end_token: RoomStreamToken,
event_types: Optional[Collection[str]] = None,
) -> Optional[Tuple[str, PersistedEventPosition]]:
"""
Returns the ID and event position of the last event in a room at or before a
Expand All @@ -1186,6 +1187,7 @@ async def get_last_event_pos_in_room_before_stream_ordering(
Args:
room_id
end_token: The token used to stream from
event_types: Optional allowlist of event types to filter by

Returns:
The ID of the most recent event and it's position, or None if there are no
Expand All @@ -1207,9 +1209,17 @@ def get_last_event_pos_in_room_before_stream_ordering_txn(
min_stream = end_token.stream
max_stream = end_token.get_max_stream_pos()

# We use `union all` because we don't need any of the deduplication logic
# (`union` is really a union + distinct). `UNION ALL` does preserve the
# ordering of the operand queries but there is no actual gurantee that it
event_type_clause = ""
event_type_args: List[str] = []
if event_types is not None and len(event_types) > 0:
event_type_clause, event_type_args = make_in_list_sql_clause(
txn.database_engine, "type", event_types
)
event_type_clause = f"AND {event_type_clause}"

# We use `UNION ALL` because we don't need any of the deduplication logic
# (`UNION` is really a `UNION` + `DISTINCT`). `UNION ALL` does preserve the
# ordering of the operand queries but there is no actual guarantee that it
# has this behavior in all scenarios so we need the extra `ORDER BY` at the
# bottom.
sql = """
Expand All @@ -1218,6 +1228,7 @@ def get_last_event_pos_in_room_before_stream_ordering_txn(
FROM events
LEFT JOIN rejections USING (event_id)
WHERE room_id = ?
%s
AND ? < stream_ordering AND stream_ordering <= ?
AND NOT outlier
AND rejections.event_id IS NULL
Expand All @@ -1229,23 +1240,25 @@ def get_last_event_pos_in_room_before_stream_ordering_txn(
FROM events
LEFT JOIN rejections USING (event_id)
WHERE room_id = ?
%s
AND stream_ordering <= ?
AND NOT outlier
AND rejections.event_id IS NULL
ORDER BY stream_ordering DESC
LIMIT 1
) AS b
ORDER BY stream_ordering DESC
"""
""" % (
event_type_clause,
event_type_clause,
)
txn.execute(
sql,
(
room_id,
min_stream,
max_stream,
room_id,
min_stream,
),
[room_id]
+ event_type_args
+ [min_stream, max_stream, room_id]
+ event_type_args
+ [min_stream],
)

for instance_name, stream_ordering, topological_ordering, event_id in txn:
Expand Down
8 changes: 8 additions & 0 deletions synapse/types/handlers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,13 @@ class RoomResult:
`/rooms/<room_id>/messages` API to retrieve earlier messages.
limited: True if their are more events than fit between the given position and now.
Sync again to get more.
bump_stamp: The `stream_ordering` of the last event according to the
`bump_event_types`. This helps clients sort more readily without them
needing to pull in a bunch of the timeline to determine the last activity.
`bump_event_types` is a thing because for example, we don't want display
name changes to mark the room as unread and bump it to the top. For
encrypted rooms, we just have to consider any activity as a bump because we
can't see the content and the client has to figure it out for themselves.
joined_count: The number of users with membership of join, including the client's
own user ID. (same as sync `v2 m.joined_member_count`)
invited_count: The number of users with membership of invite. (same as sync v2
Expand Down Expand Up @@ -209,6 +216,7 @@ class RoomResult:
prev_batch: Optional[StreamToken]
# Only optional because it won't be included for invite/knock rooms with `stripped_state`
limited: Optional[bool]
bump_stamp: int
joined_count: int
invited_count: int
notification_count: int
Expand Down
66 changes: 62 additions & 4 deletions tests/handlers/test_sliding_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -2312,15 +2312,15 @@ def test_sort_activity_basic(self) -> None:
)

# Sort the rooms (what we're testing)
sorted_room_info = self.get_success(
sorted_sync_rooms = self.get_success(
self.sliding_sync_handler.sort_rooms(
sync_room_map=sync_room_map,
to_token=after_rooms_token,
)
)

self.assertEqual(
[room_id for room_id, _ in sorted_room_info],
[room_membership.room_id for room_membership in sorted_sync_rooms],
[room_id2, room_id1],
)

Expand Down Expand Up @@ -2395,15 +2395,15 @@ def test_activity_after_xxx(self, room1_membership: str) -> None:
)

# Sort the rooms (what we're testing)
sorted_room_info = self.get_success(
sorted_sync_rooms = self.get_success(
self.sliding_sync_handler.sort_rooms(
sync_room_map=sync_room_map,
to_token=after_rooms_token,
)
)

self.assertEqual(
[room_id for room_id, _ in sorted_room_info],
[room_membership.room_id for room_membership in sorted_sync_rooms],
[room_id2, room_id1, room_id3],
"Corresponding map to disambiguate the opaque room IDs: "
+ str(
Expand All @@ -2414,3 +2414,61 @@ def test_activity_after_xxx(self, room1_membership: str) -> None:
}
),
)

def test_default_bump_event_types(self) -> None:
"""
Test that we only consider `bump_event_types` when sorting rooms.
"""
user1_id = self.register_user("user1", "pass")
user1_tok = self.login(user1_id, "pass")

room_id1 = self.helper.create_room_as(
user1_id,
tok=user1_tok,
)
message_response = self.helper.send(room_id1, "message in room1", tok=user1_tok)
room_id2 = self.helper.create_room_as(
user1_id,
tok=user1_tok,
)
self.helper.send(room_id2, "message in room2", tok=user1_tok)

# Send a reaction in room1 but it shouldn't affect the sort order
# because reactions are not part of the `DEFAULT_BUMP_EVENT_TYPES`
self.helper.send_event(
room_id1,
type=EventTypes.Reaction,
content={
"m.relates_to": {
"event_id": message_response["event_id"],
"key": "👍",
"rel_type": "m.annotation",
}
},
tok=user1_tok,
)

after_rooms_token = self.event_sources.get_current_token()

# Get the rooms the user should be syncing with
sync_room_map = self.get_success(
self.sliding_sync_handler.get_sync_room_ids_for_user(
UserID.from_string(user1_id),
from_token=None,
to_token=after_rooms_token,
)
)

# Sort the rooms (what we're testing)
sorted_sync_rooms = self.get_success(
self.sliding_sync_handler.sort_rooms(
sync_room_map=sync_room_map,
to_token=after_rooms_token,
)
)

self.assertEqual(
[room_membership.room_id for room_membership in sorted_sync_rooms],
# room2 sorts before room1 because reactions don't bump the room
[room_id2, room_id1],
)
41 changes: 41 additions & 0 deletions tests/storage/test_stream.py
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,47 @@ def test_last_event_before_sharded_token(self) -> None:
),
)

def test_restrict_event_types(self) -> None:
"""
Test that we only consider given `event_types` when finding the last event
before a token.
"""
user1_id = self.register_user("user1", "pass")
user1_tok = self.login(user1_id, "pass")

room_id1 = self.helper.create_room_as(user1_id, tok=user1_tok, is_public=True)
event_response = self.helper.send_event(
room_id1,
type="org.matrix.special_message",
content={"body": "before1, target!"},
tok=user1_tok,
)
self.helper.send(room_id1, "before2", tok=user1_tok)

after_room_token = self.event_sources.get_current_token()

# Send some events after the token
self.helper.send_event(
room_id1,
type="org.matrix.special_message",
content={"body": "after1"},
tok=user1_tok,
)
self.helper.send(room_id1, "after2", tok=user1_tok)

last_event_result = self.get_success(
self.store.get_last_event_pos_in_room_before_stream_ordering(
room_id=room_id1,
end_token=after_room_token.room_key,
event_types=["org.matrix.special_message"],
)
)
assert last_event_result is not None
last_event_id, _ = last_event_result

# Make sure it's the last event before the token
self.assertEqual(last_event_id, event_response["event_id"])


class GetCurrentStateDeltaMembershipChangesForUserTestCase(HomeserverTestCase):
"""
Expand Down
Loading