-
Notifications
You must be signed in to change notification settings - Fork 96
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 Sliding Sync /sync
endpoint (initial implementation)
#17187
Add Sliding Sync /sync
endpoint (initial implementation)
#17187
Conversation
Based on: - MSC3575: Sliding Sync (aka Sync v3): matrix-org/matrix-spec-proposals#3575 - MSC3885: Sliding Sync Extension: To-Device messages: matrix-org/matrix-spec-proposals#3885 - MSC3884: Sliding Sync Extension: E2EE: matrix-org/matrix-spec-proposals#3884
We were using the enum just to distinguish /sync v2 vs Sliding Sync /sync/e2ee so we should just make an enum for that instead of trying to glom onto the existing `sync_type` (overloading it).
Conflicts: synapse/handlers/sync.py tests/handlers/test_sync.py
Conflicts: synapse/storage/databases/main/roommember.py
Previously, we added back newly_left rooms and our second fixup was removing them. We can just switch the order of the fixups to solve this.
…sync_room_ids_for_user` implementation The new implementation catches the problem with an assert but I think it's possible to make it work as well. ``` SYNAPSE_POSTGRES=1 SYNAPSE_POSTGRES_USER=postgres SYNAPSE_TEST_LOG_LEVEL=INFO poetry run trial tests.handlers.test_sliding_sync.GetSyncRoomIdsForUserEventShardTestCase ```
…ltiple event persisters Before, the problem scenario would get caught in one of the assertions because we expect the to_token <= membership_snapshot_token or vice-versa but it's possible the tokens are intertwined and neither is ahead of each other. Especially since the `instance_map` in `membership_snapshot_token` is made up from the `stream_ordering` of membership events at various stream positions and processed on different instances (not current stream positions). We get into trouble when stream positions are lagging between workers and our now/`to_token` doesn't cleanly compare to `membership_snapshot_token`. What we really want to assert is that the `to_token` <= the stream positions at the time we asked for the room membership snapshot. Since `get_rooms_for_local_user_where_membership_is()` doesn't return that information, the closest we can get is to get the stream positions before we ask for the room membership snapshot and consider that good enough to compare against.
Co-authored-by: Erik Johnston <erikj@element.io>
See #17187 (comment) `get_membership_changes_for_user(from_key=xxx, to_key=xxx)` will handle getting out what we need and filter the results based on the tokens (even in cases where the from_key is ahead of the to_key).
if not isinstance(membership, str): | ||
raise SynapseError(400, "Invalid membership (must be a string)") |
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.
To address failing CI:
poetry run trial tests.rest.client.test_rooms.RoomMemberStateTestCase.test_invalid_puts
tests.rest.client.test_rooms
poetry run trial tests.rest.client.test_rooms.RoomMemberStateTestCase.test_invalid_puts
tests.rest.client.test_rooms
RoomMemberStateTestCase
test_invalid_puts ... [FAIL]
===============================================================================
[FAIL]
Traceback (most recent call last):
File "/home/eric/Documents/github/element/synapse/tests/rest/client/test_rooms.py", line 1069, in test_invalid_puts
self.assertEqual(
File "/home/eric/.cache/pypoetry/virtualenvs/matrix-synapse-xCtC9ulO-py3.12/lib/python3.12/site-packages/twisted/trial/_synctest.py", line 444, in assertEqual
super().assertEqual(first, second, msg)
File "/usr/lib/python3.12/unittest/case.py", line 885, in assertEqual
assertion_func(first, second, msg=msg)
File "/usr/lib/python3.12/unittest/case.py", line 878, in _baseAssertEqual
raise self.failureException(msg)
twisted.trial.unittest.FailTest: <HTTPStatus.BAD_REQUEST: 400> != 500 : b'{"errcode":"M_UNKNOWN","error":"Internal server error"}'
tests.rest.client.test_rooms.RoomMemberStateTestCase.test_invalid_puts
-------------------------------------------------------------------------------
Ran 1 tests in 1.203s
_trial_temp/test.log
2024-06-05 11:49:09-0500 [-] synapse.http.server - 146 - ERROR - PUT-7 - Failed handle request via 'RoomStateEventRestServlet': <SynapseRequest at 0x7f408b359a00 method='PUT' uri='/_matrix/client/r0/rooms/!iqYybTUtBNrlPoqnZY:red/state/m.room.member/@sid1:red' clientproto='1.1' site='red'>
Traceback (most recent call last):
File "/home/eric/Documents/github/element/synapse/synapse/http/server.py", line 332, in _async_render_wrapper
callback_return = await self._async_render(request)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/eric/Documents/github/element/synapse/synapse/http/server.py", line 544, in _async_render
callback_return = await raw_callback_return
^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/eric/Documents/github/element/synapse/synapse/rest/client/room.py", line 295, in on_PUT
event_id, _ = await self.room_member_handler.update_membership(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/eric/Documents/github/element/synapse/synapse/handlers/room_member.py", line 666, in update_membership
result = await self.update_membership_locked(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/eric/Documents/github/element/synapse/synapse/handlers/room_member.py", line 832, in update_membership_locked
if effective_membership_state not in Membership.LIST:
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: unhashable type: 'list'
Previously, we would just pass whatever we received from the event content into the handler layer that expected a plain string.
Since I changed Membership.LIST
to a set
, when people sent things like arrays/lists, we got unhashable errors. Whereas previously, the comparison worked fine with a Tuple
.
This just adds some validation to the rest layer so the handler can always expect a plain str
as the type signature says.
Thanks for the reviews @erikjohnston 🐙 |
Add initial implementation of the Sliding Sync
/sync
endpoint (not fully functional).Based on MSC3575: Sliding Sync
This iteration only focuses on returning the list of room IDs in the sliding window API (without sorting/filtering).
Rooms appear in the Sliding sync response based on:
invite
,join
,knock
,ban
membership eventsleave
membership events wheresender
is different from theuser_id
/state_key
)newly_left
(rooms that were left during the given token range, >from_token
and <=to_token
)/forget
those rooms. This doesn't modify the event itself though and only adds theforgotten
flag toroom_memberships
in Synapse. There isn't a way to tell when a room was forgotten at the moment so we can't factor it into the from/to range.Example request
POST http://localhost:8008/_matrix/client/unstable/org.matrix.msc3575/sync
Response:
Dev notes
Avoid shadowing arguments - disallow reassignment - masking existing variables:
state_groups
matrix-org/synapse#10439 (comment)Final
in type arguments to avoid shadowing arguments - disallow reassignment of function parameters python/mypy#11076Pull Request Checklist
EventStore
toEventWorkerStore
.".code blocks
.(run the linters)