-
Notifications
You must be signed in to change notification settings - Fork 161
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
Fix filtering room types on remote rooms #17434
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Fix bug in experimental [MSC3575](https://github.com/matrix-org/matrix-spec-proposals/pull/3575) Sliding Sync `/sync` endpoint when using room type filters and the user has one or more remote invites. |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -35,6 +35,8 @@ | |||||||||
RoomTypes, | ||||||||||
) | ||||||||||
from synapse.api.room_versions import RoomVersions | ||||||||||
from synapse.events import make_event_from_dict | ||||||||||
from synapse.events.snapshot import EventContext | ||||||||||
from synapse.handlers.sliding_sync import RoomSyncConfig, StateValues | ||||||||||
from synapse.rest import admin | ||||||||||
from synapse.rest.client import knock, login, room | ||||||||||
|
@@ -2791,6 +2793,72 @@ def test_filter_not_room_types(self) -> None: | |||||||||
|
||||||||||
self.assertEqual(filtered_room_map.keys(), {space_room_id}) | ||||||||||
|
||||||||||
def test_filter_room_types_with_invite_remote_room(self) -> None: | ||||||||||
"""Test that we can apply a room type filter, even if we have an invite | ||||||||||
for a remote room. | ||||||||||
|
||||||||||
This is a regression test. | ||||||||||
""" | ||||||||||
|
||||||||||
user1_id = self.register_user("user1", "pass") | ||||||||||
user1_tok = self.login(user1_id, "pass") | ||||||||||
|
||||||||||
# Create a fake remote invite and persist it. | ||||||||||
invite_room_id = "!some:room" | ||||||||||
invite_event = make_event_from_dict( | ||||||||||
{ | ||||||||||
"room_id": invite_room_id, | ||||||||||
"sender": "@user:test.serv", | ||||||||||
"state_key": user1_id, | ||||||||||
"depth": 1, | ||||||||||
"origin_server_ts": 1, | ||||||||||
"type": EventTypes.Member, | ||||||||||
"content": {"membership": Membership.INVITE}, | ||||||||||
"auth_events": [], | ||||||||||
"prev_events": [], | ||||||||||
}, | ||||||||||
room_version=RoomVersions.V10, | ||||||||||
) | ||||||||||
invite_event.internal_metadata.outlier = True | ||||||||||
invite_event.internal_metadata.out_of_band_membership = True | ||||||||||
|
||||||||||
self.get_success( | ||||||||||
self.store.maybe_store_room_on_outlier_membership( | ||||||||||
room_id=invite_room_id, room_version=invite_event.room_version | ||||||||||
) | ||||||||||
) | ||||||||||
context = EventContext.for_outlier(self.hs.get_storage_controllers()) | ||||||||||
persist_controller = self.hs.get_storage_controllers().persistence | ||||||||||
assert persist_controller is not None | ||||||||||
self.get_success(persist_controller.persist_event(invite_event, context)) | ||||||||||
|
||||||||||
# Create a normal room (no room type) | ||||||||||
room_id = self.helper.create_room_as(user1_id, 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, | ||||||||||
) | ||||||||||
) | ||||||||||
|
||||||||||
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], | ||||||||||
anoadragon453 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
), | ||||||||||
after_rooms_token, | ||||||||||
) | ||||||||||
) | ||||||||||
|
||||||||||
self.assertEqual(filtered_room_map.keys(), {room_id, invite_room_id}) | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this comment is a bit misleading, as we really just care that it worked in general There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some comment to explain the expectation here would be nice. For example, it feels weird to me that we don't filter out rooms where we don't know the type. (I'm working on a PR to update this) -> #17450 |
||||||||||
|
||||||||||
|
||||||||||
class SortRoomsTestCase(HomeserverTestCase): | ||||||||||
""" | ||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to apply this same remote-invite friendly treatment to
filters.is_encrypted
(I'm working on a PR to update this)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-> #17450