Skip to content

feat(ffi): expose required_state configuration on SyncServiceBuilder#6431

Draft
lucasfeijo wants to merge 5 commits intomatrix-org:mainfrom
lucasfeijo:feat/ffi-sync-service-required-state
Draft

feat(ffi): expose required_state configuration on SyncServiceBuilder#6431
lucasfeijo wants to merge 5 commits intomatrix-org:mainfrom
lucasfeijo:feat/ffi-sync-service-required-state

Conversation

@lucasfeijo
Copy link
Copy Markdown

Add SyncServiceBuilder::with_required_state() so FFI consumers can request additional state events during sliding sync beyond the hardcoded defaults.

Motivation

The RoomListService hardcodes DEFAULT_REQUIRED_STATE and DEFAULT_ROOM_SUBSCRIPTION_EXTRA_REQUIRED_STATE, which cover common Matrix state events. However, applications that use custom state events have no way to include them in the sliding sync request through the FFI layer.

Tested by integrating a build into our iOS application with custom state event types.

  • I've documented the public API Changes in the appropriate CHANGELOG.md files.
  • This PR was made with the help of AI.

Signed-off-by:

Add SyncServiceBuilder.withRequiredState() so FFI consumers can request
additional state events during sliding sync beyond the hardcoded defaults.
The extra entries are merged with DEFAULT_REQUIRED_STATE, ensuring
essential events (encryption, tombstone, etc.) are always present.

Made-with: Cursor
@lucasfeijo lucasfeijo requested a review from a team as a code owner April 9, 2026 17:39
@lucasfeijo lucasfeijo requested review from poljar and removed request for a team April 9, 2026 17:39
@lucasfeijo lucasfeijo marked this pull request as draft April 9, 2026 17:48
@lucasfeijo lucasfeijo marked this pull request as ready for review April 9, 2026 17:51
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 9, 2026

Codecov Report

❌ Patch coverage is 84.00000% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.83%. Comparing base (3d29db5) to head (7c4f2b8).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
crates/matrix-sdk-ui/src/sync_service.rs 69.23% 7 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6431   +/-   ##
=======================================
  Coverage   89.83%   89.83%           
=======================================
  Files         378      378           
  Lines      104214   104256   +42     
  Branches   104214   104256   +42     
=======================================
+ Hits        93616    93658   +42     
- Misses       7008     7010    +2     
+ Partials     3590     3588    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Apr 9, 2026

Merging this PR will not alter performance

✅ 50 untouched benchmarks


Comparing lucasfeijo:feat/ffi-sync-service-required-state (7c4f2b8) with main (3d29db5)

Open in CodSpeed

@lucasfeijo lucasfeijo marked this pull request as draft April 9, 2026 18:10
Collapse function signatures and destructures that fit on one line,
merge duplicate ruma imports, and reorder iterator chains per rustfmt.

Made-with: Cursor
@lucasfeijo lucasfeijo marked this pull request as ready for review April 10, 2026 01:28
Copy link
Copy Markdown
Member

@bnjbvr bnjbvr left a comment

Choose a reason for hiding this comment

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

The feature makes sense to me, but at this point it's badly implemented (by the AI):

  • the introduced test doesn't really test the new feature, as it doesn't check the request received by the server. Look at other tests which do that, and make sure to assert on the extra requested states in the mock server's received requests.
  • it isn't clear if the extra required states applies to all rooms or only rooms that one is subscribed to. Make it clearer in the function name, and make sure to add a test that the extra requested state events aren't requested in the other mode.
  • it isn't clear if a state event type that was already present in the required list will be duplicated, or overridden (and which one will be overridden; the user-provided one, or the default). I think one reasonable behavior would be that the user-provided ones should override the predefined ones. Please add a test asserting this.

@bnjbvr bnjbvr removed the request for review from poljar April 13, 2026 10:23
@lucasfeijo lucasfeijo marked this pull request as draft April 13, 2026 16:22
Make the sync service required_state configuration apply only to ALL_ROOMS and add request-level tests for scope and override behavior.

Made-with: Cursor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants