fix(sdk): Mark tracked users as dirty when the SS connection is reset. #3965
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Hywan
requested review from
jmartinesp,
andybalaam and
poljar
and removed request for
a team,
jmartinesp and
andybalaam
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3965 +/- ##
==========================================
+ Coverage 84.27% 84.31% +0.03%
==========================================
Files 267 267
Lines 28296 28323 +27
==========================================
+ Hits 23846 23880 +34
+ Misses 4450 4443 -7 ☔ View full report in Codecov by Sentry. |
poljar
requested changes
Sep 9, 2024
| let account = vodozemac::olm::Account::new(); | ||
|
|
||
| // Put some tracked users | ||
| let damir = user_id!("@damir:localhost"); |
|
It's worth emphasising here that this will negatively impact performance and bandwidth, as the rust SDK will be doing many more |
Hywan
force-pushed
the
fix-sdk-sliding-sync-no-since-must-reset-devices
branch
from
dc004b4
to
62cf64c
Compare
This patch adds the `OldMachine::mark_all_tracked_users_as_dirty`. This patch rewrites a bit `OlmMachine::new_helper` by extracting some piece of it inside `OlmMachine::new_helper_prelude`. With that, we can rewrite `OlmMachine::migration_post_verified_latch_support` to use `IdentityManager::mark_all_tracked_users_as_dirty`. This latter is the shared implementation with `OlmMachine::mark_all_tracked_users_as_dirty`. This patch adds a test for `OlmMachine:mark_all_tracked_users_as_dirty`.
Hywan
force-pushed
the
fix-sdk-sliding-sync-no-since-must-reset-devices
branch
from
62cf64c
to
c0bb278
Compare
There is a non-negligible difference MSC3575 and MSC4186 in how the `e2ee` extension works. When the client sends a request with no `pos`: * MSC3575 returns all device lists updates since the last request from the device that asked for device lists (this works similarly to to-device message handling), * MSC4186 returns no device lists updates, as it only returns changes since the provided `pos` (which is `null` in this case); this is in line with sync v2. Therefore, with MSC4186, the device list cache must be marked as to be re-downloaded if the `since` token is `None`, otherwise it's easy to miss device lists updates that happened between the previous request and the new “initial” request.
Hywan
force-pushed
the
fix-sdk-sliding-sync-no-since-must-reset-devices
branch
from
c0bb278
to
781d3d0
Compare
poljar
approved these changes
Sep 10, 2024
kegsay
added a commit
to matrix-org/matrix-js-sdk
that referenced
this pull request
Sep 18, 2024
See matrix-org/matrix-rust-sdk#3965 for more information. Requires `Extension.onRequest` to be `async`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #3959.
There is a non-negligible difference MSC3575 and MSC4186 in how the
e2eeextension works. When the client sends a request with nopos:MSC3575 returns all device lists updates since the last request
from the device that asked for device lists (this works similarly to
to-device message handling),
MSC4186 returns no device lists updates, as it only returns changes
since the provided
pos(which isnullin this case); this is inline with sync v2.
Therefore, with MSC4186, the device list cache must be marked as to be
re-downloaded if the
sincetoken isNone, otherwise it's easy tomiss device lists updates that happened between the previous request and
the new “initial” request.
This patch first off implements
OlmMachine::mark_all_tracked_users_as_dirty.Next, this patch uses this method inside sliding sync when
posisNoneande2eeextension is enabled.