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 room_types/not_room_types filtering to Sliding Sync /sync #17337

Merged
1 change: 1 addition & 0 deletions changelog.d/17337.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add `room_types`/`not_room_types` filtering to experimental [MSC3575](https://github.com/matrix-org/matrix-spec-proposals/pull/3575) Sliding Sync `/sync` endpoint.
34 changes: 29 additions & 5 deletions synapse/handlers/sliding_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,12 @@

from immutabledict import immutabledict

from synapse.api.constants import AccountDataTypes, EventTypes, Membership
from synapse.api.constants import (
AccountDataTypes,
EventContentFields,
EventTypes,
Membership,
)
from synapse.events import EventBase
from synapse.storage.roommember import RoomsForUser
from synapse.types import (
Expand Down Expand Up @@ -583,6 +588,10 @@ async def filter_rooms(
state_filter=StateFilter.from_types(
[(EventTypes.RoomEncryption, "")]
),
# Partially stated rooms should have all state events except for the
# membership events so we don't need to wait. Plus we don't want to
# block the whole sync waiting for this one room.
await_full_state=False,
)
is_encrypted = state_at_to_token.get((EventTypes.RoomEncryption, ""))

Expand All @@ -596,11 +605,26 @@ async def filter_rooms(
if filters.is_invite:
raise NotImplementedError()

if filters.room_types:
raise NotImplementedError()
# Filter by room type (space vs room, etc). A room must match one of the types
# provided in the list. `None` is a valid type for rooms which do not have a
# room type.
if filters.room_types is not None or filters.not_room_types is not None:
# 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 list(filtered_room_id_set):
create_event = await self.store.get_create_event_for_room(room_id)
room_type = create_event.content.get(EventContentFields.ROOM_TYPE)
if (
filters.room_types is not None
and room_type not in filters.room_types
):
filtered_room_id_set.remove(room_id)

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

if filters.room_name_like:
raise NotImplementedError()
Expand Down
3 changes: 3 additions & 0 deletions synapse/storage/controllers/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,9 @@ async def get_state_at(
)
)

# FIXME: Do we have to worry about gaps? What happens if we try to get a point
# in the room we haven't backfilled before?
Copy link
Collaborator Author

@MadLittleMods MadLittleMods Jun 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't affect this PR because we're just looking at the create event which we will always have but I'm curious about this for get_state_at(...) in general.

For state in a room, do we have to worry about gaps? What happens if we try to get state a point in the room we haven't backfilled before?

I guess I just don't know if we have all historical state in the room. I know we have the whole auth chain of things which will be a subset of state 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erikjohnston Is this a legit concern?

Copy link
Collaborator Author

@MadLittleMods MadLittleMods Jun 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Talked with @erikjohnston and got some knowledge.

In general, asking for state at a random position is dubious and we should careful about our usages.

It's not a problem in our current Sliding Sync usages because we're using it with the to_token which is at the front of the room so there will always be some latest event before the to_token to get state at that positon.


if last_event_id:
state = await self.get_state_after_event(
last_event_id,
Expand Down
2 changes: 1 addition & 1 deletion synapse/types/rest/client/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ class Filters(RequestBodyModel):
is_encrypted: Optional[StrictBool] = None
is_invite: Optional[StrictBool] = None
room_types: Optional[List[Union[StrictStr, None]]] = None
not_room_types: Optional[List[StrictStr]] = None
not_room_types: Optional[List[Union[StrictStr, None]]] = None
room_name_like: Optional[StrictStr] = None
tags: Optional[List[StrictStr]] = None
not_tags: Optional[List[StrictStr]] = None
Expand Down
214 changes: 213 additions & 1 deletion tests/handlers/test_sliding_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,14 @@

from twisted.test.proto_helpers import MemoryReactor

from synapse.api.constants import AccountDataTypes, EventTypes, JoinRules, Membership
from synapse.api.constants import (
AccountDataTypes,
EventContentFields,
EventTypes,
JoinRules,
Membership,
RoomTypes,
)
from synapse.api.room_versions import RoomVersions
from synapse.handlers.sliding_sync import SlidingSyncConfig
from synapse.rest import admin
Expand Down Expand Up @@ -1319,6 +1326,211 @@ def test_filter_encrypted_rooms(self) -> None:

self.assertEqual(falsy_filtered_room_map.keys(), {room_id})

def test_filter_room_types(self) -> None:
"""
Test `filter.room_types` for different room types
"""
user1_id = self.register_user("user1", "pass")
user1_tok = self.login(user1_id, "pass")

# Create a normal room (no room type)
room_id = self.helper.create_room_as(user1_id, tok=user1_tok)

# Create a space room
space_room_id = self.helper.create_room_as(
user1_id,
tok=user1_tok,
extra_content={
"creation_content": {EventContentFields.ROOM_TYPE: RoomTypes.SPACE}
},
)

# Create an arbitrarily typed room
foo_room_id = self.helper.create_room_as(
user1_id,
tok=user1_tok,
extra_content={
"creation_content": {
EventContentFields.ROOM_TYPE: "org.matrix.foobarbaz"
}
},
)

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,
)
)

# Try finding only normal rooms
filtered_room_map = self.get_success(
self.sliding_sync_handler.filter_rooms(
UserID.from_string(user1_id),
sync_room_map,
SlidingSyncConfig.SlidingSyncList.Filters(room_types=[None]),
after_rooms_token,
)
)

self.assertEqual(filtered_room_map.keys(), {room_id})

# Try finding only spaces
filtered_room_map = self.get_success(
self.sliding_sync_handler.filter_rooms(
UserID.from_string(user1_id),
sync_room_map,
SlidingSyncConfig.SlidingSyncList.Filters(room_types=[RoomTypes.SPACE]),
after_rooms_token,
)
)

self.assertEqual(filtered_room_map.keys(), {space_room_id})

# Try finding normal rooms and spaces
filtered_room_map = self.get_success(
self.sliding_sync_handler.filter_rooms(
UserID.from_string(user1_id),
sync_room_map,
SlidingSyncConfig.SlidingSyncList.Filters(
room_types=[None, RoomTypes.SPACE]
),
after_rooms_token,
)
)

self.assertEqual(filtered_room_map.keys(), {room_id, space_room_id})

# Try finding an arbitrary room type
filtered_room_map = self.get_success(
self.sliding_sync_handler.filter_rooms(
UserID.from_string(user1_id),
sync_room_map,
SlidingSyncConfig.SlidingSyncList.Filters(
room_types=["org.matrix.foobarbaz"]
),
after_rooms_token,
)
)

self.assertEqual(filtered_room_map.keys(), {foo_room_id})

def test_filter_not_room_types(self) -> None:
"""
Test `filter.not_room_types` for different room types
"""
user1_id = self.register_user("user1", "pass")
user1_tok = self.login(user1_id, "pass")

# Create a normal room (no room type)
room_id = self.helper.create_room_as(user1_id, tok=user1_tok)

# Create a space room
space_room_id = self.helper.create_room_as(
user1_id,
tok=user1_tok,
extra_content={
"creation_content": {EventContentFields.ROOM_TYPE: RoomTypes.SPACE}
},
)

# Create an arbitrarily typed room
foo_room_id = self.helper.create_room_as(
user1_id,
tok=user1_tok,
extra_content={
"creation_content": {
EventContentFields.ROOM_TYPE: "org.matrix.foobarbaz"
}
},
)

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,
)
)

# Try finding *NOT* normal rooms
filtered_room_map = self.get_success(
self.sliding_sync_handler.filter_rooms(
UserID.from_string(user1_id),
sync_room_map,
SlidingSyncConfig.SlidingSyncList.Filters(not_room_types=[None]),
after_rooms_token,
)
)

self.assertEqual(filtered_room_map.keys(), {space_room_id, foo_room_id})

# Try finding *NOT* spaces
filtered_room_map = self.get_success(
self.sliding_sync_handler.filter_rooms(
UserID.from_string(user1_id),
sync_room_map,
SlidingSyncConfig.SlidingSyncList.Filters(
not_room_types=[RoomTypes.SPACE]
),
after_rooms_token,
)
)

self.assertEqual(filtered_room_map.keys(), {room_id, foo_room_id})

# Try finding *NOT* normal rooms or spaces
filtered_room_map = self.get_success(
self.sliding_sync_handler.filter_rooms(
UserID.from_string(user1_id),
sync_room_map,
SlidingSyncConfig.SlidingSyncList.Filters(
not_room_types=[None, RoomTypes.SPACE]
),
after_rooms_token,
)
)

self.assertEqual(filtered_room_map.keys(), {foo_room_id})

# Test how it behaves when we have both `room_types` and `not_room_types`.
# `not_room_types` should win.
filtered_room_map = self.get_success(
self.sliding_sync_handler.filter_rooms(
UserID.from_string(user1_id),
sync_room_map,
SlidingSyncConfig.SlidingSyncList.Filters(
room_types=[None], not_room_types=[None]
),
after_rooms_token,
)
)

# Nothing matches because nothing is both a normal room and not a normal room
self.assertEqual(filtered_room_map.keys(), set())

# Test how it behaves when we have both `room_types` and `not_room_types`.
# `not_room_types` should win.
filtered_room_map = self.get_success(
self.sliding_sync_handler.filter_rooms(
UserID.from_string(user1_id),
sync_room_map,
SlidingSyncConfig.SlidingSyncList.Filters(
room_types=[None, RoomTypes.SPACE], not_room_types=[None]
),
after_rooms_token,
)
)

self.assertEqual(filtered_room_map.keys(), {space_room_id})


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