fix(sync): apply senders/contains_url filters, gate historical leaves by include_leave#232
Merged
Merged
Conversation
… by include_leave Three sync-filter gaps: - Room timeline/state filters ignored the `senders` allow-list and `contains_url` — only types/not_types/not_senders applied. Share one RoomEventFilter predicate and add both (not_senders keeps precedence over senders; contains_url matches a string url, Synapse-style). The state filter now also reaches MSC4222 `state_after`, where Element carries its state, so it was previously a no-op there for the primary client. - `include_leave` (spec default false) was ignored: an initial sync returned every room the user had ever left. Historical left rooms now appear only when the filter opts in, while a room left within the sync window is still always surfaced so the client learns of the leave.
…remental sync The first cut gated left rooms on `newly_left || include_leave`, which made a room left long ago reappear in the leave section on every incremental sync when include_leave was set — breaking Complement's TestOlderLeftRoomsNotInLeaveSection. A left room now surfaces on an incremental sync only when the membership changed within the window; an initial or full-state sync includes historical left rooms exactly when include_leave is set.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Three sync-filter compliance gaps, verified against the filter definitions and continuwuity/Synapse behavior.
sendersallow-list +contains_url— the room timeline/state filters only appliedtypes/not_types/not_senders, silently ignoring thesendersallow-list andcontains_url. Factored a sharedRoomEventFilterpredicate and added both:not_sendersis checked beforesenders, so a sender in both is excluded (spec precedence). An emptysenders: []excludes everything (distinct from absent).contains_urlkeeps events whosecontent.urlis a string (matching Synapse:null/non-string count as "no url").state_after(built internally under the stable key, renamed for the client afterwards). Element opts into MSC4222, so filtering only/state/eventswas a no-op there — this fixestypes/not_typestoo, which had the same blind spot.include_leave(spec defaultfalse) was ignored — an initial/syncreturned every room the user had ever left. Corrected semantics:full_statesync — and only wheninclude_leaveis set.include_leavedoes not re-surface an already-reported leave on later incremental syncs (Complement'sTestOlderLeftRoomsNotInLeaveSection).Known limitation (documented in code): the timeline is trimmed to
limitupstream, then filtered, so a highly selective predicate can return fewer thanlimitmatches even when more exist earlier — matching Synapse would require pushing the predicate into the timeline read.Tests: senders allow-list +
not_sendersprecedence + empty-list;contains_urltrue/false/omitted and null/non-string url; state filtering understate_after; andinclude_leavegating (initial excludes historical, include_leave surfaces them on a full view, an in-window leave is always surfaced, and a historical leave is not re-surfaced on later incremental syncs).