Skip to content

Commit

Permalink
Sliding Sync: Move filters tests to rest layer (#17703)
Browse files Browse the repository at this point in the history
Move filters tests to rest layer in order to test the new (with sliding
sync tables) and fallback paths that Sliding Sync can use.

Also found a bug in the new path because it's not being tested which is
also fixed in this PR. We now take into account `has_known_state` when
filtering.

Spawning from
#17662 (comment).
This should have been done when we started using the new sliding sync
tables in #17630
  • Loading branch information
MadLittleMods authored Sep 12, 2024
1 parent c5b4be6 commit 9b83fb7
Show file tree
Hide file tree
Showing 9 changed files with 1,928 additions and 1,795 deletions.
1 change: 1 addition & 0 deletions changelog.d/17703.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Refactor sliding sync filter unit tests so the sliding sync API has better test coverage.
17 changes: 16 additions & 1 deletion synapse/handlers/sliding_sync/room_lists.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ async def _compute_interested_rooms_new_tables(
event_pos=change.event_pos,
room_version_id=change.room_version_id,
# We keep the current state of the room though
has_known_state=existing_room.has_known_state,
room_type=existing_room.room_type,
is_encrypted=existing_room.is_encrypted,
)
Expand All @@ -270,6 +271,7 @@ async def _compute_interested_rooms_new_tables(
event_id=change.event_id,
event_pos=change.event_pos,
room_version_id=change.room_version_id,
has_known_state=True,
room_type=room_type,
is_encrypted=is_encrypted,
)
Expand Down Expand Up @@ -305,6 +307,7 @@ async def _compute_interested_rooms_new_tables(
event_id=None,
event_pos=newly_left_room_map[room_id],
room_version_id=await self.store.get_room_version_id(room_id),
has_known_state=True,
room_type=room_type,
is_encrypted=is_encrypted,
)
Expand Down Expand Up @@ -1630,12 +1633,14 @@ async def filter_rooms(
and room_type not in filters.room_types
):
filtered_room_id_set.remove(room_id)
continue

if (
filters.not_room_types is not None
and room_type in filters.not_room_types
):
filtered_room_id_set.remove(room_id)
continue

if filters.room_name_like is not None:
with start_active_span("filters.room_name_like"):
Expand Down Expand Up @@ -1705,7 +1710,10 @@ async def filter_rooms_using_tables(
filtered_room_id_set = {
room_id
for room_id in filtered_room_id_set
if sync_room_map[room_id].is_encrypted == filters.is_encrypted
# Remove rooms if we can't figure out what the encryption status is
if sync_room_map[room_id].has_known_state
# Or remove if it doesn't match the filter
and sync_room_map[room_id].is_encrypted == filters.is_encrypted
}

# Filter for rooms that the user has been invited to
Expand Down Expand Up @@ -1734,19 +1742,26 @@ async def filter_rooms_using_tables(
# Make a copy so we don't run into an error: `Set changed size during
# iteration`, when we filter out and remove items
for room_id in filtered_room_id_set.copy():
# Remove rooms if we can't figure out what room type it is
if not sync_room_map[room_id].has_known_state:
filtered_room_id_set.remove(room_id)
continue

room_type = sync_room_map[room_id].room_type

if (
filters.room_types is not None
and room_type not in filters.room_types
):
filtered_room_id_set.remove(room_id)
continue

if (
filters.not_room_types is not None
and room_type in filters.not_room_types
):
filtered_room_id_set.remove(room_id)
continue

if filters.room_name_like is not None:
with start_active_span("filters.room_name_like"):
Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/client/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -1044,7 +1044,7 @@ async def encode_rooms(
serialized_rooms[room_id]["heroes"] = serialized_heroes

# We should only include the `initial` key if it's `True` to save bandwidth.
# The absense of this flag means `False`.
# The absence of this flag means `False`.
if room_result.initial:
serialized_rooms[room_id]["initial"] = room_result.initial

Expand Down
6 changes: 4 additions & 2 deletions synapse/storage/databases/main/roommember.py
Original file line number Diff line number Diff line change
Expand Up @@ -1420,6 +1420,7 @@ def get_sliding_sync_rooms_for_user_txn(
SELECT m.room_id, m.sender, m.membership, m.membership_event_id,
r.room_version,
m.event_instance_name, m.event_stream_ordering,
m.has_known_state,
COALESCE(j.room_type, m.room_type),
COALESCE(j.is_encrypted, m.is_encrypted)
FROM sliding_sync_membership_snapshots AS m
Expand All @@ -1437,8 +1438,9 @@ def get_sliding_sync_rooms_for_user_txn(
event_id=row[3],
room_version_id=row[4],
event_pos=PersistedEventPosition(row[5], row[6]),
room_type=row[7],
is_encrypted=bool(row[8]),
has_known_state=bool(row[7]),
room_type=row[8],
is_encrypted=bool(row[9]),
)
for row in txn
}
Expand Down
1 change: 1 addition & 0 deletions synapse/storage/roommember.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ class RoomsForUserSlidingSync:
event_pos: PersistedEventPosition
room_version_id: str

has_known_state: bool
room_type: Optional[str]
is_encrypted: bool

Expand Down
Loading

0 comments on commit 9b83fb7

Please sign in to comment.