Skip to content

Commit fe94608

Browse files
committed
Fix bug where we didn't correctly filter lazy members by room
When fetching previously sent lazy members we didn't filter by room, which meant that we didn't send down member events in a room if we'd previously sent that user's member event in another room.
1 parent 1192a4b commit fe94608

File tree

2 files changed

+96
-6
lines changed

2 files changed

+96
-6
lines changed

synapse/storage/databases/main/sliding_sync.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
DatabasePool,
2626
LoggingDatabaseConnection,
2727
LoggingTransaction,
28+
make_in_list_sql_clause,
2829
)
2930
from synapse.storage.engines import PostgresEngine
3031
from synapse.types import MultiWriterStreamToken, RoomStreamToken
@@ -532,14 +533,17 @@ async def get_sliding_sync_connection_lazy_members(
532533
def get_sliding_sync_connection_lazy_members_txn(
533534
txn: LoggingTransaction,
534535
) -> Mapping[str, int]:
535-
sql = """
536-
SELECT user_id, mem.connection_position, last_seen_ts
537-
FROM sliding_sync_connection_positions AS pos
538-
INNER JOIN sliding_sync_connection_lazy_members AS mem USING (connection_key)
539-
WHERE pos.connection_position = ?
536+
user_clause, user_args = make_in_list_sql_clause(
537+
txn.database_engine, "user_id", user_ids
538+
)
539+
540+
sql = f"""
541+
SELECT user_id, connection_position, last_seen_ts
542+
FROM sliding_sync_connection_lazy_members AS pos
543+
WHERE room_id = ? AND {user_clause}
540544
"""
541545

542-
txn.execute(sql, (connection_position,))
546+
txn.execute(sql, (room_id, *user_args))
543547

544548
# Filter out any cache entries that only apply to forked connection
545549
# positions. Entries with `NULL` connection position apply to all

tests/rest/client/sliding_sync/test_rooms_required_state.py

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -932,6 +932,92 @@ def test_lazy_members_limited_sync(self) -> None:
932932
exact=True,
933933
)
934934

935+
def test_lazy_members_across_multiple_rooms(self) -> None:
936+
"""Test that lazy loading room members are tracked per-room correctly.
937+
"""
938+
939+
user1_id = self.register_user("user1", "pass")
940+
user1_tok = self.login(user1_id, "pass")
941+
user2_id = self.register_user("user2", "pass")
942+
user2_tok = self.login(user2_id, "pass")
943+
944+
# Create two rooms with both users in them and send a message in each
945+
room_id1 = self.helper.create_room_as(user2_id, tok=user2_tok)
946+
self.helper.join(room_id1, user1_id, tok=user1_tok)
947+
self.helper.send(room_id1, "room1-msg1", tok=user2_tok)
948+
949+
room_id2 = self.helper.create_room_as(user2_id, tok=user2_tok)
950+
self.helper.join(room_id2, user1_id, tok=user1_tok)
951+
self.helper.send(room_id2, "room2-msg1", tok=user2_tok)
952+
953+
# Make a sync with lazy loading for the room members to establish
954+
# a position
955+
sync_body = {
956+
"lists": {
957+
"foo-list": {
958+
"ranges": [[0, 1]],
959+
"required_state": [
960+
[EventTypes.Member, StateValues.LAZY],
961+
],
962+
"timeline_limit": 1,
963+
}
964+
}
965+
}
966+
response_body, from_token = self.do_sync(sync_body, tok=user1_tok)
967+
968+
# We expect to see only user2's membership in both rooms
969+
state_map = self.get_success(
970+
self.storage_controllers.state.get_current_state(room_id1)
971+
)
972+
self._assertRequiredStateIncludes(
973+
response_body["rooms"][room_id1]["required_state"],
974+
{
975+
state_map[(EventTypes.Member, user2_id)],
976+
},
977+
exact=True,
978+
)
979+
980+
# Send a message in room1 from user1
981+
self.helper.send(room_id1, "room1-msg2", tok=user1_tok)
982+
983+
# Make an incremental Sliding Sync request and check that we get user1's
984+
# membership.
985+
response_body, from_token = self.do_sync(
986+
sync_body, since=from_token, tok=user1_tok
987+
)
988+
989+
state_map = self.get_success(
990+
self.storage_controllers.state.get_current_state(room_id1)
991+
)
992+
self._assertRequiredStateIncludes(
993+
response_body["rooms"][room_id1]["required_state"],
994+
{
995+
state_map[(EventTypes.Member, user1_id)],
996+
},
997+
exact=True,
998+
)
999+
1000+
# Send a message in room2 from user1
1001+
self.helper.send(room_id2, "room2-msg2", tok=user1_tok)
1002+
1003+
# Make an incremental Sliding Sync request and check that we get user1's
1004+
# membership.
1005+
response_body, from_token = self.do_sync(
1006+
sync_body, since=from_token, tok=user1_tok
1007+
)
1008+
state_map = self.get_success(
1009+
self.storage_controllers.state.get_current_state(room_id2)
1010+
)
1011+
self._assertRequiredStateIncludes(
1012+
response_body["rooms"][room_id2]["required_state"],
1013+
{
1014+
state_map[(EventTypes.Member, user1_id)],
1015+
},
1016+
exact=True,
1017+
)
1018+
1019+
1020+
9351021
def test_rooms_required_state_me(self) -> None:
9361022
"""
9371023
Test `rooms.required_state` correctly handles $ME.

0 commit comments

Comments
 (0)