From 7789399ba518ed2fdadee6edfb768cad5a3efa4d Mon Sep 17 00:00:00 2001 From: Erwan Leboucher Date: Tue, 28 Jul 2026 15:23:08 +0200 Subject: [PATCH] fix(sync): Apply MSC4186 room filters --- changelog.d/+ee3b3204.bugfix.md | 1 + src/api/client/sync/v5.rs | 273 ++++++++++++++++++++++++++------ 2 files changed, 226 insertions(+), 48 deletions(-) create mode 100644 changelog.d/+ee3b3204.bugfix.md diff --git a/changelog.d/+ee3b3204.bugfix.md b/changelog.d/+ee3b3204.bugfix.md new file mode 100644 index 000000000..ab9f5e68f --- /dev/null +++ b/changelog.d/+ee3b3204.bugfix.md @@ -0,0 +1 @@ +Added MSC4186 direct, encrypted, and room-type list filters, resolving invited rooms from their stripped invite state. Contributed by @eleboucher. diff --git a/src/api/client/sync/v5.rs b/src/api/client/sync/v5.rs index 7dbd5aa4c..3bf6ac693 100644 --- a/src/api/client/sync/v5.rs +++ b/src/api/client/sync/v5.rs @@ -1,7 +1,6 @@ use std::{ cmp::{self, Ordering}, collections::{BTreeMap, BTreeSet, HashMap, HashSet}, - ops::Deref, time::Duration, }; @@ -24,7 +23,7 @@ sync::{SnakeConnectionsKey, into_snake_key}, }; use futures::{ - FutureExt, Stream, StreamExt, TryFutureExt, + FutureExt, StreamExt, TryFutureExt, future::{OptionFuture, join3, try_join4}, pin_mut, }; @@ -36,8 +35,13 @@ assign, directory::RoomTypeFilter, events::{ - AnySyncEphemeralRoomEvent, AnySyncStateEvent, StateEventType, TimelineEventType, - room::member::{MembershipState, RoomMemberEventContent}, + AnyStrippedStateEvent, AnySyncEphemeralRoomEvent, AnySyncStateEvent, + GlobalAccountDataEventType, StateEventType, TimelineEventType, + direct::DirectEvent, + room::{ + create::RoomCreateEventContent, + member::{MembershipState, RoomMemberEventContent}, + }, typing::{SyncTypingEvent, TypingEventContent}, }, serde::Raw, @@ -255,13 +259,32 @@ async fn collect_sync_response( rooms: BTreeMap::new(), extensions, }); + let direct_rooms = if body.lists.values().any(|list| { + list.filters + .as_ref() + .is_some_and(|filters| filters.is_dm.is_some()) + }) { + match services + .account_data + .get_global::(sender_user, GlobalAccountDataEventType::Direct) + .await + { + | Ok(event) => event.content.0.into_values().flatten().collect(), + | Err(error) if error.is_not_found() => HashSet::new(), + | Err(error) => return Err(error), + } + } else { + HashSet::new() + }; let mut known_room_updates = handle_lists( services, + sender_user, body, all_invited_rooms.clone(), all_joined_rooms.clone(), all_rooms, + &direct_rooms, &mut todo_rooms, known_rooms, &mut response, @@ -405,10 +428,12 @@ async fn fetch_subscriptions( #[allow(clippy::too_many_arguments)] async fn handle_lists<'a, Rooms, AllRooms>( services: &Services, + sender_user: &UserId, body: &sync_events::v5::Request, all_invited_rooms: Rooms, all_joined_rooms: Rooms, all_rooms: AllRooms, + direct_rooms: &HashSet, todo_rooms: &'a mut TodoRooms, known_rooms: &'a KnownRooms, response: &'_ mut sync_events::v5::Response, @@ -417,29 +442,75 @@ async fn handle_lists<'a, Rooms, AllRooms>( Rooms: Iterator + Clone + Send + 'a, AllRooms: Iterator + Clone + Send + 'a, { - // TODO MSC4186: Implement remaining list filters: is_dm, is_encrypted, - // room_types. + // TODO MSC4186: ruma's `ListFilters` does not model `spaces`, `tags` or + // `not_tags`, nor the rename of `is_invite` to `is_invited`. + let invited_rooms: HashSet<&RoomId> = all_invited_rooms.clone().collect(); + + // Memoised for the whole request; lists commonly overlap. + let mut encrypted_rooms: HashMap<&RoomId, bool> = HashMap::new(); + let mut room_types: HashMap<&RoomId, Option> = HashMap::new(); + let mut known_room_updates = KnownRoomUpdates::new(); for (list_id, list) in &body.lists { - let active_rooms: Vec<_> = match list.filters.as_ref().and_then(|f| f.is_invite) { + let filters = list.filters.as_ref(); + let is_dm = filters.and_then(|filters| filters.is_dm); + let is_encrypted = filters.and_then(|filters| filters.is_encrypted); + let room_types_filter = filters.map_or(&[][..], |filters| filters.room_types.as_slice()); + let not_room_types = filters.map_or(&[][..], |filters| filters.not_room_types.as_slice()); + let filter_room_types = !room_types_filter.is_empty() || !not_room_types.is_empty(); + + let candidate_rooms: Vec<&RoomId> = match filters.and_then(|filters| filters.is_invite) { | None => all_rooms.clone().collect(), | Some(true) => all_invited_rooms.clone().collect(), | Some(false) => all_joined_rooms.clone().collect(), }; - let active_rooms = match list.filters.as_ref().map(|f| &f.not_room_types) { - | None => active_rooms, - | Some(filter) if filter.is_empty() => active_rooms, - | Some(value) => - filter_rooms( - services, - value, - &true, - active_rooms.iter().stream().map(Deref::deref), - ) - .collect() - .await, - }; + let mut active_rooms: Vec<&RoomId> = Vec::with_capacity(candidate_rooms.len()); + for room_id in candidate_rooms { + if !matches_bool_filter(direct_rooms.contains(room_id), is_dm) { + continue; + } + + let invited = invited_rooms.contains(room_id); + + if is_encrypted.is_some() { + let encrypted = match encrypted_rooms.get(room_id) { + | Some(encrypted) => *encrypted, + | None => { + let encrypted = + room_is_encrypted(services, sender_user, room_id, invited).await; + encrypted_rooms.insert(room_id, encrypted); + encrypted + }, + }; + + if !matches_bool_filter(encrypted, is_encrypted) { + continue; + } + } + + if filter_room_types { + let room_type = match room_types.get(room_id) { + | Some(room_type) => room_type.clone(), + | None => { + let room_type = + room_type_filter(services, sender_user, room_id, invited).await; + room_types.insert(room_id, room_type.clone()); + room_type + }, + }; + + let Some(room_type) = room_type else { + continue; + }; + + if !matches_room_type(&room_type, room_types_filter, not_room_types) { + continue; + } + } + + active_rooms.push(room_id); + } let mut new_known_rooms: BTreeSet = BTreeSet::new(); @@ -502,6 +573,92 @@ async fn handle_lists<'a, Rooms, AllRooms>( known_room_updates } +fn matches_bool_filter(value: bool, filter: Option) -> bool { + filter.is_none_or(|expected| value == expected) +} + +/// Rooms we are only invited to have no state locally, so their stripped +/// invite state answers for them instead. +async fn room_is_encrypted( + services: &Services, + sender_user: &UserId, + room_id: &RoomId, + invited: bool, +) -> bool { + if !invited { + return services + .rooms + .state_accessor + .is_encrypted_room(room_id) + .await; + } + + stripped_state_event(services, sender_user, room_id, &StateEventType::RoomEncryption) + .await + .is_some() +} + +/// Returns `None` if the type could not be determined, in which case the room +/// is excluded from the list. +/// +/// Stripped invite state is only recommended to carry `m.room.create`, so an +/// invite without one is treated as untyped rather than hidden from every list. +async fn room_type_filter( + services: &Services, + sender_user: &UserId, + room_id: &RoomId, + invited: bool, +) -> Option { + if invited { + let content: Option = + stripped_state_event(services, sender_user, room_id, &StateEventType::RoomCreate) + .await + .and_then(|event| event.get_field("content").ok().flatten()); + + return Some(RoomTypeFilter::from(content.and_then(|content| content.room_type))); + } + + match services.rooms.state_accessor.get_room_type(room_id).await { + | Ok(room_type) => Some(RoomTypeFilter::from(Some(room_type))), + | Err(error) if error.is_not_found() => Some(RoomTypeFilter::Default), + | Err(error) => { + warn!(%room_id, %error, "Failed to fetch room type for a sliding sync list filter"); + None + }, + } +} + +async fn stripped_state_event( + services: &Services, + sender_user: &UserId, + room_id: &RoomId, + event_type: &StateEventType, +) -> Option> { + services + .rooms + .state_cache + .invite_state(sender_user, room_id) + .await + .ok()? + .into_iter() + .find(|event| { + event + .get_field::("type") + .ok() + .flatten() + .as_ref() == Some(event_type) + }) +} + +fn matches_room_type( + room_type: &RoomTypeFilter, + room_types: &[RoomTypeFilter], + not_room_types: &[RoomTypeFilter], +) -> bool { + !not_room_types.contains(room_type) + && (room_types.is_empty() || room_types.contains(room_type)) +} + #[allow(clippy::too_many_arguments)] async fn process_rooms<'a, Rooms>( services: &Services, @@ -1232,42 +1389,62 @@ async fn collect_receipts(_services: &Services) -> sync_events::v5::response::Re sync_events::v5::response::Receipts::default() } -fn filter_rooms<'a, Rooms>( - services: &'a Services, - filter: &'a [RoomTypeFilter], - negate: &'a bool, - rooms: Rooms, -) -> impl Stream + Send + 'a -where - Rooms: Stream + Send + 'a, -{ - rooms.filter_map(async |room_id| { - let room_type = services.rooms.state_accessor.get_room_type(room_id).await; - - if room_type.as_ref().is_err_and(|e| !e.is_not_found()) { - return None; - } - - let room_type_filter = RoomTypeFilter::from(room_type.ok()); - - let include = if *negate { - !filter.contains(&room_type_filter) - } else { - filter.is_empty() || filter.contains(&room_type_filter) - }; - - include.then_some(room_id) - }) -} - #[cfg(test)] mod tests { + use std::slice; + use ruma::{owned_room_id, room_id}; use super::*; + fn custom_room_type() -> RoomTypeFilter { RoomTypeFilter::from(Some("com.example")) } + fn response() -> sync_events::v5::Response { sync_events::v5::Response::new("1".to_owned()) } + #[test] + fn absent_bool_filter_matches_either_value() { + assert!(matches_bool_filter(true, None)); + assert!(matches_bool_filter(false, None)); + } + + #[test] + fn bool_filter_matches_only_its_own_value() { + assert!(matches_bool_filter(true, Some(true))); + assert!(!matches_bool_filter(false, Some(true))); + assert!(matches_bool_filter(false, Some(false))); + assert!(!matches_bool_filter(true, Some(false))); + } + + #[test] + fn absent_room_type_filters_match_every_type() { + assert!(matches_room_type(&custom_room_type(), &[], &[])); + assert!(matches_room_type(&RoomTypeFilter::Default, &[], &[])); + } + + #[test] + fn positive_room_type_matches_and_mismatches() { + let custom = custom_room_type(); + assert!(matches_room_type(&custom, slice::from_ref(&custom), &[])); + assert!(!matches_room_type(&custom, &[RoomTypeFilter::Default], &[])); + } + + #[test] + fn default_room_type_matches_untyped_rooms() { + assert!(matches_room_type(&RoomTypeFilter::Default, &[RoomTypeFilter::Default], &[])); + } + + #[test] + fn negative_room_type_excludes_without_a_positive_filter() { + assert!(!matches_room_type(&RoomTypeFilter::Space, &[], &[RoomTypeFilter::Space])); + assert!(matches_room_type(&RoomTypeFilter::Default, &[], &[RoomTypeFilter::Space])); + } + + #[test] + fn negative_room_type_overrides_positive() { + let custom = custom_room_type(); + assert!(!matches_room_type(&custom, slice::from_ref(&custom), slice::from_ref(&custom))); + } + #[test] fn fresh_response_is_empty() { assert!(response_is_empty(&response()));