threads: Bulk process thread subscription updates from sync and companion enpoint

[razvp]

Fixes #5610

• [ ] Public API changes documented in changelogs (optional)

Signed-off-by: Razvan [email protected]

[codecov[bot]]

Codecov Report

:x: Patch coverage is 94.27313% with 13 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 88.55%. Comparing base (1af22a7) to head (d4d5d6e). :warning: Report is 2 commits behind head on main. :white_check_mark: All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...tes/matrix-sdk-base/src/store/integration_tests.rs	93.42%	0 Missing and 10 partials :warning:
crates/matrix-sdk-sqlite/src/state_store.rs	94.28%	0 Missing and 2 partials :warning:
...ates/matrix-sdk/src/client/thread_subscriptions.rs	97.22%	1 Missing :warning:

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #5848    +/-   ##
========================================
  Coverage   88.54%   88.55%            
========================================
  Files         363      363            
  Lines      103703   103885   +182     
  Branches   103703   103885   +182     
========================================
+ Hits        91821    91992   +171     
- Misses       7517     7519     +2     
- Partials     4365     4374     +9     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[codspeed-hq[bot]]

CodSpeed Performance Report

Merging #5848 will not alter performance

_{Comparing razvp:thread-subs-bulk-upsert (d4d5d6e) with main (1af22a7)}

Summary

✅ 50 untouched

[Hywan]

Thanks! It looks like a solid contribution, thank you. However, I wonder if you could add a bit of context and to explain what you're doing, and why, please?

[razvp]

Thanks! It looks like a solid contribution, thank you. However, I wonder if you could add a bit of context and to explain what you're doing, and why, please?

The sliding sync and back-pagination endpoints provide thread subscription updates in batches, but currently the SDK stores/updates them one by one. (MSC4308) This PR changes that by processing them in bulk, in one database transaction.

The first commit adds the upsert_thread_subscriptions() method to the StateStore trait and implements it for the 3 storage types. The IndexeddbStateStore and MemoryStore implementations are very similar to the existing single update per transaction The second commit updates ThreadSubscriptionCatchup::sync_subscriptions and ThreadSubscriptionCatchup::thread_subscriptions_catchup_task to actually use the bulk api.

Most of the bump_stamp logic is gathered from the existing test. (ex: if an update doesn't have a bump_stamp and the stored subscription has one, then keep the existing one)

Switching to a bulk api was suggested in this comment.

[Hywan]

Can you please fix the broken test?

[razvp]

cargo test --package matrix-sdk-ui --test integration -- room_list_service::test_thread_subscriptions_extension_enabled_only_if_server_advertises_it

test room_list_service::test_thread_subscriptions_extension_enabled_only_if_server_advertises_it ... ok

It passes locally and in the first CI run all tests passed. It does contain thread_subscriptions in the name, but i didn't edit this test or logic around it :D

I don't think i can re-run the pipeline. Can you re-run it or should i do a rebase to trigger it?

[Hywan]

For a reason I ignore, I can't re-run the jobs… Can you push an empty commit please, or rebase on top of main and force-push (better)?

[razvp]

Sure, i'll rebase it

[Hywan]

I think it's worth considering the upsert_thread_subscription (singular, not plural) method now, what do you think?

[razvp]

Thank you for reviewing!

Yes, the old method can be removed. Could you please assign the new issue to me?

[Hywan]

I can't assign it to you, but feel free to comment you're working on it!