From 6bca1ab49f2e8475ffe14cf2d00d7ecbe2814b53 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 22 Oct 2024 13:49:58 +0200 Subject: [PATCH] feat(ui): `RoomListService::subscribe_to_rooms` no longer has a `settings` argument. This patch removes the `settings` argument of `RoomListService::subscribe_to_rooms`. The settings were mostly composed of: * `required_state`: now shared with `all_rooms`, so that we are sure they are synced; except that `m.room.create` is added for subscriptions. * `timeline_limit`: now defaults to 20. This patch thus creates the `DEFAULT_REQUIRED_STATE` and `DEFAULT_ROOM_SUBSCRIPTION_TIMELINE_LIMIT` constants. Finally, this patch updates the tests, and updates all usages of `subscribe_to_rooms`. --- bindings/matrix-sdk-ffi/src/room_list.rs | 36 +---------- .../src/room_list_service/mod.rs | 61 ++++++++++--------- .../tests/integration/room_list_service.rs | 57 ++++++----------- labs/multiverse/src/main.rs | 5 +- 4 files changed, 52 insertions(+), 107 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/room_list.rs b/bindings/matrix-sdk-ffi/src/room_list.rs index d8700571580..1dd5d68437e 100644 --- a/bindings/matrix-sdk-ffi/src/room_list.rs +++ b/bindings/matrix-sdk-ffi/src/room_list.rs @@ -135,11 +135,7 @@ impl RoomListService { }))) } - fn subscribe_to_rooms( - &self, - room_ids: Vec, - settings: Option, - ) -> Result<(), RoomListError> { + fn subscribe_to_rooms(&self, room_ids: Vec) -> Result<(), RoomListError> { let room_ids = room_ids .into_iter() .map(|room_id| { @@ -147,10 +143,7 @@ impl RoomListService { }) .collect::, _>>()?; - self.inner.subscribe_to_rooms( - &room_ids.iter().map(AsRef::as_ref).collect::>(), - settings.map(Into::into), - ); + self.inner.subscribe_to_rooms(&room_ids.iter().map(AsRef::as_ref).collect::>()); Ok(()) } @@ -680,31 +673,6 @@ impl RoomListItem { } } -#[derive(uniffi::Record)] -pub struct RequiredState { - pub key: String, - pub value: String, -} - -#[derive(uniffi::Record)] -pub struct RoomSubscription { - pub required_state: Option>, - pub timeline_limit: u32, - pub include_heroes: Option, -} - -impl From for http::request::RoomSubscription { - fn from(val: RoomSubscription) -> Self { - assign!(http::request::RoomSubscription::default(), { - required_state: val.required_state.map(|r| - r.into_iter().map(|s| (s.key.into(), s.value)).collect() - ).unwrap_or_default(), - timeline_limit: val.timeline_limit.into(), - include_heroes: val.include_heroes, - }) - } -} - #[derive(uniffi::Object)] pub struct UnreadNotificationsCount { highlight_count: u32, diff --git a/crates/matrix-sdk-ui/src/room_list_service/mod.rs b/crates/matrix-sdk-ui/src/room_list_service/mod.rs index 08237d6bdaa..0bd8c29ae5a 100644 --- a/crates/matrix-sdk-ui/src/room_list_service/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list_service/mod.rs @@ -57,7 +57,7 @@ mod room_list; pub mod sorters; mod state; -use std::{sync::Arc, time::Duration}; +use std::{iter::once, sync::Arc, time::Duration}; use async_stream::stream; use eyeball::Subscriber; @@ -69,7 +69,7 @@ use matrix_sdk::{ use matrix_sdk_base::sliding_sync::http; pub use room::*; pub use room_list::*; -use ruma::{assign, directory::RoomTypeFilter, events::StateEventType, OwnedRoomId, RoomId}; +use ruma::{assign, directory::RoomTypeFilter, events::StateEventType, OwnedRoomId, RoomId, UInt}; pub use state::*; use thiserror::Error; use tokio::time::timeout; @@ -77,6 +77,23 @@ use tracing::debug; use crate::timeline; +/// The default `required_state` constant value for sliding sync lists and +/// sliding sync room subscriptions. +const DEFAULT_REQUIRED_STATE: &[(StateEventType, &str)] = &[ + (StateEventType::RoomName, ""), + (StateEventType::RoomEncryption, ""), + (StateEventType::RoomMember, "$LAZY"), + (StateEventType::RoomMember, "$ME"), + (StateEventType::RoomTopic, ""), + (StateEventType::RoomCanonicalAlias, ""), + (StateEventType::RoomPowerLevels, ""), + (StateEventType::RoomPinnedEvents, ""), + (StateEventType::CallMember, ""), +]; + +/// The default `timeline_limit` value when used with room subscriptions. +const DEFAULT_ROOM_SUBSCRIPTION_TIMELINE_LIMIT: u32 = 20; + /// The [`RoomListService`] type. See the module's documentation to learn more. #[derive(Debug)] pub struct RoomListService { @@ -145,17 +162,12 @@ impl RoomListService { .add_range(ALL_ROOMS_DEFAULT_SELECTIVE_RANGE), ) .timeline_limit(1) - .required_state(vec![ - (StateEventType::RoomName, "".to_owned()), - (StateEventType::RoomEncryption, "".to_owned()), - (StateEventType::RoomMember, "$LAZY".to_owned()), - (StateEventType::RoomMember, "$ME".to_owned()), - (StateEventType::RoomTopic, "".to_owned()), - (StateEventType::RoomCanonicalAlias, "".to_owned()), - (StateEventType::RoomPowerLevels, "".to_owned()), - (StateEventType::RoomPinnedEvents, "".to_owned()), - (StateEventType::CallMember, "".to_owned()), - ]) + .required_state( + DEFAULT_REQUIRED_STATE + .iter() + .map(|(state_event, value)| (state_event.clone(), (*value).to_owned())) + .collect(), + ) .include_heroes(Some(true)) .filters(Some(assign!(http::request::ListFilters::default(), { // As defined in the [SlidingSync MSC](https://github.com/matrix-org/matrix-spec-proposals/blob/9450ced7fb9cf5ea9077d029b3adf36aebfa8709/proposals/3575-sync.md?plain=1#L444) @@ -382,22 +394,13 @@ impl RoomListService { /// /// It means that all events from these rooms will be received every time, /// no matter how the `RoomList` is configured. - pub fn subscribe_to_rooms( - &self, - room_ids: &[&RoomId], - settings: Option, - ) { - let mut settings = settings.unwrap_or_default(); - - // Make sure to always include the room creation event in the required state - // events, to know what the room version is. - if !settings - .required_state - .iter() - .any(|(event_type, _state_key)| *event_type == StateEventType::RoomCreate) - { - settings.required_state.push((StateEventType::RoomCreate, "".to_owned())); - } + pub fn subscribe_to_rooms(&self, room_ids: &[&RoomId]) { + let settings = assign!(http::request::RoomSubscription::default(), { + required_state: DEFAULT_REQUIRED_STATE.iter().map(|(state_event, value)| { + (state_event.clone(), (*value).to_owned()) + }).chain(once((StateEventType::RoomCreate, "".to_owned()))).collect(), + timeline_limit: UInt::from(DEFAULT_ROOM_SUBSCRIPTION_TIMELINE_LIMIT), + }); let cancel_in_flight_request = match self.state_machine.get() { State::Init | State::Recovering | State::Error { .. } | State::Terminated { .. } => { diff --git a/crates/matrix-sdk-ui/tests/integration/room_list_service.rs b/crates/matrix-sdk-ui/tests/integration/room_list_service.rs index efde3a3bf50..a6d99824463 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list_service.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list_service.rs @@ -2083,18 +2083,7 @@ async fn test_room_subscription() -> Result<(), Error> { // Subscribe. - room_list.subscribe_to_rooms( - &[room_id_1], - Some(assign!(RoomSubscription::default(), { - required_state: vec![ - (StateEventType::RoomName, "".to_owned()), - (StateEventType::RoomTopic, "".to_owned()), - (StateEventType::RoomAvatar, "".to_owned()), - (StateEventType::RoomCanonicalAlias, "".to_owned()), - ], - timeline_limit: uint!(30), - })), - ); + room_list.subscribe_to_rooms(&[room_id_1]); sync_then_assert_request_and_fake_response! { [server, room_list, sync] @@ -2109,12 +2098,17 @@ async fn test_room_subscription() -> Result<(), Error> { room_id_1: { "required_state": [ ["m.room.name", ""], + ["m.room.encryption", ""], + ["m.room.member", "$LAZY"], + ["m.room.member", "$ME"], ["m.room.topic", ""], - ["m.room.avatar", ""], ["m.room.canonical_alias", ""], + ["m.room.power_levels", ""], + ["m.room.pinned_events", ""], + ["org.matrix.msc3401.call.member", ""], ["m.room.create", ""], ], - "timeline_limit": 30, + "timeline_limit": 20, }, }, }, @@ -2127,18 +2121,7 @@ async fn test_room_subscription() -> Result<(), Error> { // Subscribe to another room. - room_list.subscribe_to_rooms( - &[room_id_2], - Some(assign!(RoomSubscription::default(), { - required_state: vec![ - (StateEventType::RoomName, "".to_owned()), - (StateEventType::RoomTopic, "".to_owned()), - (StateEventType::RoomAvatar, "".to_owned()), - (StateEventType::RoomCanonicalAlias, "".to_owned()), - ], - timeline_limit: uint!(30), - })), - ); + room_list.subscribe_to_rooms(&[room_id_2]); sync_then_assert_request_and_fake_response! { [server, room_list, sync] @@ -2153,12 +2136,17 @@ async fn test_room_subscription() -> Result<(), Error> { room_id_2: { "required_state": [ ["m.room.name", ""], + ["m.room.encryption", ""], + ["m.room.member", "$LAZY"], + ["m.room.member", "$ME"], ["m.room.topic", ""], - ["m.room.avatar", ""], ["m.room.canonical_alias", ""], + ["m.room.power_levels", ""], + ["m.room.pinned_events", ""], + ["org.matrix.msc3401.call.member", ""], ["m.room.create", ""], ], - "timeline_limit": 30, + "timeline_limit": 20, }, }, }, @@ -2171,18 +2159,7 @@ async fn test_room_subscription() -> Result<(), Error> { // Subscribe to an already subscribed room. Nothing happens. - room_list.subscribe_to_rooms( - &[room_id_1], - Some(assign!(RoomSubscription::default(), { - required_state: vec![ - (StateEventType::RoomName, "".to_owned()), - (StateEventType::RoomTopic, "".to_owned()), - (StateEventType::RoomAvatar, "".to_owned()), - (StateEventType::RoomCanonicalAlias, "".to_owned()), - ], - timeline_limit: uint!(30), - })), - ); + room_list.subscribe_to_rooms(&[room_id_1]); sync_then_assert_request_and_fake_response! { [server, room_list, sync] diff --git a/labs/multiverse/src/main.rs b/labs/multiverse/src/main.rs index e844449cec7..ab8f4e84f7f 100644 --- a/labs/multiverse/src/main.rs +++ b/labs/multiverse/src/main.rs @@ -445,10 +445,7 @@ impl App { .get_selected_room_id(Some(selected)) .and_then(|room_id| self.ui_rooms.lock().unwrap().get(&room_id).cloned()) { - let mut sub = RoomSubscription::default(); - sub.timeline_limit = uint!(30); - - self.sync_service.room_list_service().subscribe_to_rooms(&[room.room_id()], Some(sub)); + self.sync_service.room_list_service().subscribe_to_rooms(&[room.room_id()]); self.current_room_subscription = Some(room); } }