From 5f58438389da356498661b78ee6a08ee049c440b Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 22 May 2023 15:19:40 +0200 Subject: [PATCH 01/48] feat(ui): Oh, a `roomlist` module. --- crates/matrix-sdk-ui/Cargo.toml | 1 + crates/matrix-sdk-ui/src/lib.rs | 3 +++ crates/matrix-sdk-ui/src/roomlist/mod.rs | 0 3 files changed, 4 insertions(+) create mode 100644 crates/matrix-sdk-ui/src/roomlist/mod.rs diff --git a/crates/matrix-sdk-ui/Cargo.toml b/crates/matrix-sdk-ui/Cargo.toml index 232e35192..bbd20f7a5 100644 --- a/crates/matrix-sdk-ui/Cargo.toml +++ b/crates/matrix-sdk-ui/Cargo.toml @@ -11,6 +11,7 @@ e2e-encryption = ["matrix-sdk/e2e-encryption"] native-tls = ["matrix-sdk/native-tls"] rustls-tls = ["matrix-sdk/rustls-tls"] +experimental-roomlist = ["experimental-sliding-sync"] experimental-sliding-sync = ["matrix-sdk/experimental-sliding-sync"] [dependencies] diff --git a/crates/matrix-sdk-ui/src/lib.rs b/crates/matrix-sdk-ui/src/lib.rs index 6635ce881..94ded1e70 100644 --- a/crates/matrix-sdk-ui/src/lib.rs +++ b/crates/matrix-sdk-ui/src/lib.rs @@ -13,6 +13,9 @@ // limitations under the License. mod events; + +#[cfg(feature = "experimental-roomlist")] +pub mod roomlist; pub mod timeline; pub use self::timeline::Timeline; diff --git a/crates/matrix-sdk-ui/src/roomlist/mod.rs b/crates/matrix-sdk-ui/src/roomlist/mod.rs new file mode 100644 index 000000000..e69de29bb From 7706b0096b5aa63e34ed3de13c91bb5165a057bc Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 24 May 2023 11:15:39 +0200 Subject: [PATCH 02/48] !foo --- crates/matrix-sdk-ui/Cargo.toml | 2 +- crates/matrix-sdk-ui/src/roomlist/mod.rs | 33 ++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/crates/matrix-sdk-ui/Cargo.toml b/crates/matrix-sdk-ui/Cargo.toml index bbd20f7a5..af767afe6 100644 --- a/crates/matrix-sdk-ui/Cargo.toml +++ b/crates/matrix-sdk-ui/Cargo.toml @@ -4,7 +4,7 @@ version = "0.6.0" edition = "2021" [features] -default = ["e2e-encryption", "native-tls"] +default = ["e2e-encryption", "native-tls", "experimental-roomlist"] e2e-encryption = ["matrix-sdk/e2e-encryption"] diff --git a/crates/matrix-sdk-ui/src/roomlist/mod.rs b/crates/matrix-sdk-ui/src/roomlist/mod.rs index e69de29bb..7dd95a3cd 100644 --- a/crates/matrix-sdk-ui/src/roomlist/mod.rs +++ b/crates/matrix-sdk-ui/src/roomlist/mod.rs @@ -0,0 +1,33 @@ +//! `RoomList` API. + +use matrix_sdk::{Client, Result, SlidingSync, SlidingSyncList}; + +#[derive(Debug)] +pub struct RoomList { + sliding_sync: SlidingSync, +} + +impl RoomList { + pub async fn new(client: Client) -> Result { + Ok(Self { + sliding_sync: client + .sliding_sync() + .storage_key(Some("matrix-sdk-ui-roomlist".to_string())) + .add_cached_list(SlidingSyncList::builder("all_rooms")) + .await? + .add_list(SlidingSyncList::builder("visible_rooms")) + .build() + .await?, + }) + } + + pub fn sync(&self) {} +} + +#[cfg(test)] +mod tests { + #[tokio::test] + async fn test_has_two_lists() { + // let + } +} From f47c2ba1252e6312a44bc817d49b24067ab42e75 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 24 May 2023 17:53:58 +0200 Subject: [PATCH 03/48] !bar --- crates/matrix-sdk-ui/Cargo.toml | 13 +- crates/matrix-sdk-ui/src/lib.rs | 6 +- crates/matrix-sdk-ui/src/room_list/mod.rs | 169 ++++++++++++++++++ crates/matrix-sdk-ui/src/roomlist/mod.rs | 33 ---- .../matrix-sdk-ui/tests/integration/main.rs | 2 + .../tests/integration/room_list.rs | 107 +++++++++++ .../integration/timeline/sliding_sync.rs | 2 +- 7 files changed, 294 insertions(+), 38 deletions(-) create mode 100644 crates/matrix-sdk-ui/src/room_list/mod.rs delete mode 100644 crates/matrix-sdk-ui/src/roomlist/mod.rs create mode 100644 crates/matrix-sdk-ui/tests/integration/room_list.rs diff --git a/crates/matrix-sdk-ui/Cargo.toml b/crates/matrix-sdk-ui/Cargo.toml index af767afe6..b3abd3ad7 100644 --- a/crates/matrix-sdk-ui/Cargo.toml +++ b/crates/matrix-sdk-ui/Cargo.toml @@ -4,17 +4,20 @@ version = "0.6.0" edition = "2021" [features] -default = ["e2e-encryption", "native-tls", "experimental-roomlist"] +default = ["e2e-encryption", "native-tls", "experimental-room-list"] e2e-encryption = ["matrix-sdk/e2e-encryption"] native-tls = ["matrix-sdk/native-tls"] rustls-tls = ["matrix-sdk/rustls-tls"] -experimental-roomlist = ["experimental-sliding-sync"] +experimental-room-list = ["experimental-sliding-sync", "dep:async-stream"] experimental-sliding-sync = ["matrix-sdk/experimental-sliding-sync"] +testing = [] + [dependencies] +async-stream = { workspace = true, optional = true } async-trait = { workspace = true } chrono = "0.4.23" eyeball-im = { workspace = true } @@ -23,6 +26,7 @@ futures-util = { workspace = true } imbl = { version = "2.0.0", features = ["serde"] } indexmap = "1.9.1" matrix-sdk = { version = "0.6.2", path = "../matrix-sdk", default-features = false } +matrix-sdk-base = { version = "0.6.1", path = "../matrix-sdk-base" } mime = "0.3.16" once_cell = { workspace = true } pin-project-lite = "0.2.9" @@ -35,8 +39,13 @@ tracing = { workspace = true, features = ["attributes"] } [dev-dependencies] anyhow = { workspace = true } +assert-json-diff = "2.0" assert_matches = { workspace = true } ctor = { workspace = true } matrix-sdk-test = { version = "0.6.0", path = "../../testing/matrix-sdk-test" } tracing-subscriber = { version = "0.3.11", features = ["env-filter"] } wiremock = "0.5.13" + +[[test]] +name = "integration" +required-features = ["testing"] \ No newline at end of file diff --git a/crates/matrix-sdk-ui/src/lib.rs b/crates/matrix-sdk-ui/src/lib.rs index 94ded1e70..a08debc48 100644 --- a/crates/matrix-sdk-ui/src/lib.rs +++ b/crates/matrix-sdk-ui/src/lib.rs @@ -14,8 +14,10 @@ mod events; -#[cfg(feature = "experimental-roomlist")] -pub mod roomlist; +#[cfg(feature = "experimental-room-list")] +pub mod room_list; pub mod timeline; +#[cfg(feature = "experimental-room-list")] +pub use self::room_list::RoomList; pub use self::timeline::Timeline; diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs new file mode 100644 index 000000000..eb6a4244e --- /dev/null +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -0,0 +1,169 @@ +//! `RoomList` API. + +use async_stream::stream; +use async_trait::async_trait; +use futures_util::{pin_mut, Stream, StreamExt}; +use matrix_sdk::{ + Client, Error, Result, SlidingSync, SlidingSyncList, SlidingSyncMode, UpdateSummary, +}; +use once_cell::sync::Lazy; + +pub const ALL_ROOMS_LIST_NAME: &str = "all_rooms"; +pub const VISIBLE_ROOMS_LIST_NAME: &str = "visible_rooms"; + +#[derive(Debug)] +pub struct RoomList { + sliding_sync: SlidingSync, +} + +impl RoomList { + pub async fn new(client: Client) -> Result { + Ok(Self { + sliding_sync: client + .sliding_sync() + .storage_key(Some("matrix-sdk-ui-roomlist".to_string())) + .add_cached_list( + SlidingSyncList::builder(ALL_ROOMS_LIST_NAME) + .sync_mode(SlidingSyncMode::new_selective()), + ) + .await? + .build() + .await?, + }) + } + + pub fn sync(&self) -> impl Stream> + '_ { + stream! { + let sync = self.sliding_sync.sync(); + pin_mut!(sync); + + let mut state = State::LoadFirstRooms; + + while let Some(update_summary) = sync.next().await { + state = state.next(update_summary, &self.sliding_sync).await?; + + // if let State::Failure = state { + // yield Err(()); + // // transform into a custom `MyError::SyncFailure` + // } else { + yield Ok(()); + // } + } + } + } + + #[cfg(feature = "testing")] + pub fn sliding_sync(&self) -> &SlidingSync { + &self.sliding_sync + } +} + +enum State { + LoadFirstRooms, + LoadAllRooms, + Enjoy, + Failure, +} + +impl State { + async fn next( + self, + update_summary: Result, + sliding_sync: &SlidingSync, + ) -> Result { + use State::*; + + if update_summary.is_err() { + return Ok(Failure); + } + + let (next_state, actions) = match self { + LoadFirstRooms => (LoadAllRooms, Actions::first_rooms_are_loaded()), + LoadAllRooms => (Enjoy, Actions::none()), + Enjoy => (Enjoy, Actions::none()), + Failure => (Failure, Actions::none()), + }; + + for action in actions.iter() { + action.run(sliding_sync).await?; + } + + Ok(next_state) + } +} + +#[async_trait] +trait Action { + async fn run(&self, sliding_sync: &SlidingSync) -> Result<(), Error>; +} + +struct AddVisibleRoomsList; + +#[async_trait] +impl Action for AddVisibleRoomsList { + async fn run(&self, sliding_sync: &SlidingSync) -> Result<(), Error> { + sliding_sync + .add_list( + SlidingSyncList::builder(VISIBLE_ROOMS_LIST_NAME) + .sync_mode(SlidingSyncMode::new_selective()), + ) + .await?; + + Ok(()) + } +} + +struct ChangeAllRoomsListToSelectiveSyncMode; + +#[async_trait] +impl Action for ChangeAllRoomsListToSelectiveSyncMode { + async fn run(&self, sliding_sync: &SlidingSync) -> Result<(), Error> { + sliding_sync + .on_list("all_rooms", |list| list.set_sync_mode(SlidingSyncMode::new_growing(20, None))) + .unwrap() // transform into a custom `MyError::UnknownList` + ?; + + Ok(()) + } +} + +type OneAction = Box; +type ManyActions = Vec; + +struct Actions { + actions: &'static Lazy, +} + +macro_rules! actions { + ( + $( + $action_group_name:ident => [ + $( $action_name:ident ),* $(,)? + ] + ),* + $(,)? + ) => { + $( + fn $action_group_name () -> Self { + static ACTIONS: Lazy = Lazy::new(|| { + vec![ + $( Box::new( $action_name ) ),* + ] + }); + + Self { actions: &ACTIONS } + } + )* + }; +} + +impl Actions { + actions! { + none => [], + first_rooms_are_loaded => [ChangeAllRoomsListToSelectiveSyncMode, AddVisibleRoomsList], + } + + fn iter(&self) -> &[OneAction] { + self.actions.as_slice() + } +} diff --git a/crates/matrix-sdk-ui/src/roomlist/mod.rs b/crates/matrix-sdk-ui/src/roomlist/mod.rs deleted file mode 100644 index 7dd95a3cd..000000000 --- a/crates/matrix-sdk-ui/src/roomlist/mod.rs +++ /dev/null @@ -1,33 +0,0 @@ -//! `RoomList` API. - -use matrix_sdk::{Client, Result, SlidingSync, SlidingSyncList}; - -#[derive(Debug)] -pub struct RoomList { - sliding_sync: SlidingSync, -} - -impl RoomList { - pub async fn new(client: Client) -> Result { - Ok(Self { - sliding_sync: client - .sliding_sync() - .storage_key(Some("matrix-sdk-ui-roomlist".to_string())) - .add_cached_list(SlidingSyncList::builder("all_rooms")) - .await? - .add_list(SlidingSyncList::builder("visible_rooms")) - .build() - .await?, - }) - } - - pub fn sync(&self) {} -} - -#[cfg(test)] -mod tests { - #[tokio::test] - async fn test_has_two_lists() { - // let - } -} diff --git a/crates/matrix-sdk-ui/tests/integration/main.rs b/crates/matrix-sdk-ui/tests/integration/main.rs index 0823b3db8..5d36604b4 100644 --- a/crates/matrix-sdk-ui/tests/integration/main.rs +++ b/crates/matrix-sdk-ui/tests/integration/main.rs @@ -7,6 +7,8 @@ use wiremock::{ Mock, MockServer, ResponseTemplate, }; +#[cfg(feature = "experimental-room-list")] +mod room_list; mod timeline; #[cfg(all(test, not(target_arch = "wasm32")))] diff --git a/crates/matrix-sdk-ui/tests/integration/room_list.rs b/crates/matrix-sdk-ui/tests/integration/room_list.rs new file mode 100644 index 000000000..165e310ef --- /dev/null +++ b/crates/matrix-sdk-ui/tests/integration/room_list.rs @@ -0,0 +1,107 @@ +use anyhow::{Context, Result}; +use futures_util::{pin_mut, StreamExt}; +use matrix_sdk_test::async_test; +use matrix_sdk_ui::{room_list, RoomList}; +use serde_json::json; +use wiremock::{http::Method, Match, Mock, MockServer, Request, ResponseTemplate}; + +use crate::logged_in_client; + +async fn new_room_list() -> Result<(MockServer, RoomList)> { + let (client, server) = logged_in_client().await; + let room_list = RoomList::new(client).await?; + + Ok((server, room_list)) +} + +#[derive(Copy, Clone)] +struct SlidingSyncMatcher; + +impl Match for SlidingSyncMatcher { + fn matches(&self, request: &Request) -> bool { + request.url.path() == "/_matrix/client/unstable/org.matrix.msc3575/sync" + && request.method == Method::Post + } +} + +macro_rules! sync_then_assert_request_and_fake_response { + ( + [$server:ident, $room_list_sync_stream:ident] + request = { $( $request_json:tt )* }, + response = { $( $response_json:tt )* } + $(,)? + ) => { + { + let matcher = SlidingSyncMatcher; + + let _mock_guard = Mock::given(matcher) + .respond_with(ResponseTemplate::new(200).set_body_json( + json!({ $( $response_json )* }) + )) + .mount_as_scoped(&$server) + .await; + + let next = $room_list_sync_stream.next().await.context("`sync` trip")??; + + for request in $server.received_requests().await.expect("Request recording has been disabled").iter().rev() { + if matcher.matches(request) { + let json_value = serde_json::from_slice::(&request.body).unwrap(); + + if let Err(error) = assert_json_diff::assert_json_matches_no_panic( + &json_value, + &json!({ $( $response_json )* }), + assert_json_diff::Config::new(assert_json_diff::CompareMode::Inclusive), + ) { + dbg!(json_value); + panic!("{}", error); + } + } + } + + next + } + }; +} + +#[async_test] +async fn test_one_list() -> Result<()> { + let (_, room_list) = new_room_list().await?; + let sliding_sync = room_list.sliding_sync(); + + assert_eq!(sliding_sync.on_list(room_list::ALL_ROOMS_LIST_NAME, |_list| ()), Some(())); + assert_eq!(sliding_sync.on_list(room_list::VISIBLE_ROOMS_LIST_NAME, |_list| ()), None); + + Ok(()) +} + +#[async_test] +async fn test_foo() -> Result<()> { + let (server, room_list) = new_room_list().await?; + + let sync = room_list.sync(); + pin_mut!(sync); + + sync_then_assert_request_and_fake_response! { + [server, sync] + request = { + "lists": { + "all_rooms": { + "ranges": [], + } + }, + }, + response = { + "pos": "foo", + "lists": {}, + "rooms": {}, + "extensions": {}, + }, + }; + + assert_eq!( + room_list.sliding_sync().on_list(room_list::VISIBLE_ROOMS_LIST_NAME, |_list| ()), + Some(()) + ); + + Ok(()) +} diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/sliding_sync.rs b/crates/matrix-sdk-ui/tests/integration/timeline/sliding_sync.rs index 360075314..b9f650c83 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/sliding_sync.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/sliding_sync.rs @@ -28,7 +28,7 @@ macro_rules! receive_response { .mount_as_scoped(&$server) .await; - let next = $sliding_sync_stream.next().await.context("`stream` trip")??; + let next = $sliding_sync_stream.next().await.context("`sync` trip")??; next } From 92c300353501ef05e5a1d4cd8f85b22f3325599f Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 25 May 2023 15:50:00 +0200 Subject: [PATCH 04/48] !baz --- crates/matrix-sdk-ui/src/room_list/mod.rs | 2 +- .../tests/integration/room_list.rs | 26 ++++++++++++++----- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index eb6a4244e..06cc17fdf 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -119,7 +119,7 @@ struct ChangeAllRoomsListToSelectiveSyncMode; impl Action for ChangeAllRoomsListToSelectiveSyncMode { async fn run(&self, sliding_sync: &SlidingSync) -> Result<(), Error> { sliding_sync - .on_list("all_rooms", |list| list.set_sync_mode(SlidingSyncMode::new_growing(20, None))) + .on_list(ALL_ROOMS_LIST_NAME, |list| list.set_sync_mode(SlidingSyncMode::new_growing(20, None))) .unwrap() // transform into a custom `MyError::UnknownList` ?; diff --git a/crates/matrix-sdk-ui/tests/integration/room_list.rs b/crates/matrix-sdk-ui/tests/integration/room_list.rs index 165e310ef..f2773df0a 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list.rs @@ -26,9 +26,9 @@ impl Match for SlidingSyncMatcher { macro_rules! sync_then_assert_request_and_fake_response { ( - [$server:ident, $room_list_sync_stream:ident] - request = { $( $request_json:tt )* }, - response = { $( $response_json:tt )* } + sync with $server:ident and $room_list_sync_stream:ident, + assert request = { $( $request_json:tt )* }, + respond with = { $( $response_json:tt )* } $(,)? ) => { { @@ -49,7 +49,7 @@ macro_rules! sync_then_assert_request_and_fake_response { if let Err(error) = assert_json_diff::assert_json_matches_no_panic( &json_value, - &json!({ $( $response_json )* }), + &json!({ $( $request_json )* }), assert_json_diff::Config::new(assert_json_diff::CompareMode::Inclusive), ) { dbg!(json_value); @@ -82,15 +82,27 @@ async fn test_foo() -> Result<()> { pin_mut!(sync); sync_then_assert_request_and_fake_response! { - [server, sync] - request = { + sync with server and sync, + assert request = { "lists": { "all_rooms": { "ranges": [], + "required_state": [ + ["m.room.encryption", ""] + ], + "sort": ["by_recency", "by_name"], } }, + "extensions": { + "e2ee": { + "enabled": true, + }, + "to_device": { + "enabled": true, + } + } }, - response = { + respond with = { "pos": "foo", "lists": {}, "rooms": {}, From 5f81d829c4465b08e5e7de5205ede323f50c65b5 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 25 May 2023 16:13:50 +0200 Subject: [PATCH 05/48] feat(ui): Create an `Init` state, and add state observer for `RoomList`. --- crates/matrix-sdk-ui/Cargo.toml | 1 + crates/matrix-sdk-ui/src/room_list/mod.rs | 64 ++++++++++++++++------- 2 files changed, 46 insertions(+), 19 deletions(-) diff --git a/crates/matrix-sdk-ui/Cargo.toml b/crates/matrix-sdk-ui/Cargo.toml index b3abd3ad7..37ac6068a 100644 --- a/crates/matrix-sdk-ui/Cargo.toml +++ b/crates/matrix-sdk-ui/Cargo.toml @@ -20,6 +20,7 @@ testing = [] async-stream = { workspace = true, optional = true } async-trait = { workspace = true } chrono = "0.4.23" +eyeball = { workspace = true } eyeball-im = { workspace = true } futures-core = { workspace = true } futures-util = { workspace = true } diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 06cc17fdf..056d3608e 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -2,6 +2,7 @@ use async_stream::stream; use async_trait::async_trait; +use eyeball::shared::Observable; use futures_util::{pin_mut, Stream, StreamExt}; use matrix_sdk::{ Client, Error, Result, SlidingSync, SlidingSyncList, SlidingSyncMode, UpdateSummary, @@ -14,6 +15,7 @@ pub const VISIBLE_ROOMS_LIST_NAME: &str = "visible_rooms"; #[derive(Debug)] pub struct RoomList { sliding_sync: SlidingSync, + state: Observable, } impl RoomList { @@ -22,13 +24,9 @@ impl RoomList { sliding_sync: client .sliding_sync() .storage_key(Some("matrix-sdk-ui-roomlist".to_string())) - .add_cached_list( - SlidingSyncList::builder(ALL_ROOMS_LIST_NAME) - .sync_mode(SlidingSyncMode::new_selective()), - ) - .await? .build() .await?, + state: Observable::new(State::Init), }) } @@ -37,10 +35,19 @@ impl RoomList { let sync = self.sliding_sync.sync(); pin_mut!(sync); - let mut state = State::LoadFirstRooms; + // Before doing the first sync, let's transition to the next state. + { + let next_state = self.state.read().next(&self.sliding_sync).await?; + + Observable::set(&self.state, next_state); + } while let Some(update_summary) = sync.next().await { - state = state.next(update_summary, &self.sliding_sync).await?; + { + let next_state = self.state.read().next(&self.sliding_sync).await?; + + Observable::set(&self.state, next_state); + } // if let State::Failure = state { // yield Err(()); @@ -52,36 +59,37 @@ impl RoomList { } } + pub fn state(&self) -> State { + self.state.get() + } + + pub fn state_stream(&self) -> impl Stream { + Observable::subscribe(&self.state) + } + #[cfg(feature = "testing")] pub fn sliding_sync(&self) -> &SlidingSync { &self.sliding_sync } } -enum State { +#[derive(Debug, Copy, Clone)] +pub enum State { + Init, LoadFirstRooms, LoadAllRooms, Enjoy, - Failure, } impl State { - async fn next( - self, - update_summary: Result, - sliding_sync: &SlidingSync, - ) -> Result { + async fn next(&self, sliding_sync: &SlidingSync) -> Result { use State::*; - if update_summary.is_err() { - return Ok(Failure); - } - let (next_state, actions) = match self { + Init => (LoadFirstRooms, Actions::start()), LoadFirstRooms => (LoadAllRooms, Actions::first_rooms_are_loaded()), LoadAllRooms => (Enjoy, Actions::none()), Enjoy => (Enjoy, Actions::none()), - Failure => (Failure, Actions::none()), }; for action in actions.iter() { @@ -97,6 +105,23 @@ trait Action { async fn run(&self, sliding_sync: &SlidingSync) -> Result<(), Error>; } +struct AddAllRoomsList; + +#[async_trait] +impl Action for AddAllRoomsList { + async fn run(&self, sliding_sync: &SlidingSync) -> Result<(), Error> { + sliding_sync + .add_cached_list( + SlidingSyncList::builder(ALL_ROOMS_LIST_NAME) + .sync_mode(SlidingSyncMode::new_selective()) + .add_range(0..=20), + ) + .await?; + + Ok(()) + } +} + struct AddVisibleRoomsList; #[async_trait] @@ -160,6 +185,7 @@ macro_rules! actions { impl Actions { actions! { none => [], + start => [AddAllRoomsList], first_rooms_are_loaded => [ChangeAllRoomsListToSelectiveSyncMode, AddVisibleRoomsList], } From 4fa38d23d432160571aeb0041a5c4658bf9a38d3 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Fri, 26 May 2023 09:52:29 +0200 Subject: [PATCH 06/48] feat(sdk): `SlidingSync` has more non-blocking API. This patch does several things. First, `SlidingSync::on_list` is now async, and accept async closures. Second, `SlidingSync::lists` and `::rooms` are behind an `AsyncRwLock` instead of a `StdRwLock`. The rest of the patch updates the consequence of this. --- Cargo.lock | 4 + crates/matrix-sdk/src/sliding_sync/README.md | 5 +- crates/matrix-sdk/src/sliding_sync/builder.rs | 6 +- crates/matrix-sdk/src/sliding_sync/cache.rs | 253 +++++++++--------- .../matrix-sdk/src/sliding_sync/list/mod.rs | 42 +-- crates/matrix-sdk/src/sliding_sync/mod.rs | 47 ++-- 6 files changed, 186 insertions(+), 171 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 02e6efa3d..dffa536e6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2981,16 +2981,20 @@ name = "matrix-sdk-ui" version = "0.6.0" dependencies = [ "anyhow", + "assert-json-diff", "assert_matches", + "async-stream", "async-trait", "chrono", "ctor 0.2.0", + "eyeball", "eyeball-im", "futures-core", "futures-util", "imbl", "indexmap", "matrix-sdk", + "matrix-sdk-base", "matrix-sdk-test", "mime", "once_cell", diff --git a/crates/matrix-sdk/src/sliding_sync/README.md b/crates/matrix-sdk/src/sliding_sync/README.md index 5bb0971ca..89a0678a0 100644 --- a/crates/matrix-sdk/src/sliding_sync/README.md +++ b/crates/matrix-sdk/src/sliding_sync/README.md @@ -404,6 +404,7 @@ use ruma::{assign, api::client::sync::sync_events::v4, events::StateEventType}; use tracing::{warn, error, info, debug}; use futures_util::{pin_mut, StreamExt}; use url::Url; +use std::future::ready; # async { # let homeserver = Url::parse("http://example.com")?; # let client = Client::new(homeserver).await?; @@ -441,8 +442,8 @@ let sliding_sync = sliding_sync_builder // subscribe to the list APIs for updates let (list_state_stream, list_count_stream, list_stream) = sliding_sync.on_list(&active_list_name, |list| { - (list.state_stream(), list.maximum_number_of_rooms_stream(), list.room_list_stream()) -}).unwrap(); + ready((list.state_stream(), list.maximum_number_of_rooms_stream(), list.room_list_stream())) +}).await.unwrap(); tokio::spawn(async move { pin_mut!(list_state_stream); diff --git a/crates/matrix-sdk/src/sliding_sync/builder.rs b/crates/matrix-sdk/src/sliding_sync/builder.rs index 7b44beeea..c1ad56116 100644 --- a/crates/matrix-sdk/src/sliding_sync/builder.rs +++ b/crates/matrix-sdk/src/sliding_sync/builder.rs @@ -8,7 +8,7 @@ use ruma::{ events::TimelineEventType, OwnedRoomId, }; -use tokio::sync::broadcast::channel; +use tokio::sync::{broadcast::channel, RwLock as AsyncRwLock}; use url::Url; use super::{ @@ -248,8 +248,8 @@ impl SlidingSyncBuilder { .await?; } - let rooms = StdRwLock::new(self.rooms); - let lists = StdRwLock::new(lists); + let rooms = AsyncRwLock::new(self.rooms); + let lists = AsyncRwLock::new(lists); Ok(SlidingSync::new(SlidingSyncInner { homeserver: self.homeserver, diff --git a/crates/matrix-sdk/src/sliding_sync/cache.rs b/crates/matrix-sdk/src/sliding_sync/cache.rs index 0b0084767..d109f3736 100644 --- a/crates/matrix-sdk/src/sliding_sync/cache.rs +++ b/crates/matrix-sdk/src/sliding_sync/cache.rs @@ -69,13 +69,13 @@ pub(super) async fn store_sliding_sync_state(sliding_sync: &SlidingSync) -> Resu // Write every `SlidingSyncList` that's configured for caching into the store. let frozen_lists = { - let rooms_lock = sliding_sync.inner.rooms.read().unwrap(); + let rooms_lock = sliding_sync.inner.rooms.read().await; sliding_sync .inner .lists .read() - .unwrap() + .await .iter() .filter_map(|(list_name, list)| { matches!(list.cache_policy(), SlidingSyncListCachePolicy::Enabled).then(|| { @@ -228,151 +228,148 @@ mod tests { } #[allow(clippy::await_holding_lock)] - #[test] - fn test_sliding_sync_can_be_stored_and_restored() -> Result<()> { - block_on(async { - let homeserver = Url::parse("https://foo.bar")?; - let client = Client::new(homeserver).await?; + #[tokio::test] + async fn test_sliding_sync_can_be_stored_and_restored() -> Result<()> { + let homeserver = Url::parse("https://foo.bar")?; + let client = Client::new(homeserver).await?; - let store = client.store(); + let store = client.store(); - // Store entries don't exist. - assert!(store - .get_custom_value(format_storage_key_for_sliding_sync("hello").as_bytes()) + // Store entries don't exist. + assert!(store + .get_custom_value(format_storage_key_for_sliding_sync("hello").as_bytes()) + .await? + .is_none()); + + assert!(store + .get_custom_value( + format_storage_key_for_sliding_sync_list("hello", "list_foo").as_bytes() + ) + .await? + .is_none()); + + assert!(store + .get_custom_value( + format_storage_key_for_sliding_sync_list("hello", "list_bar").as_bytes() + ) + .await? + .is_none()); + + // Create a new `SlidingSync` instance, and store it. + { + let sliding_sync = client + .sliding_sync() + .storage_key(Some("hello".to_owned())) + .add_cached_list(SlidingSyncList::builder("list_foo")) .await? - .is_none()); + .add_list(SlidingSyncList::builder("list_bar")) + .build() + .await?; - assert!(store - .get_custom_value( - format_storage_key_for_sliding_sync_list("hello", "list_foo").as_bytes() - ) - .await? - .is_none()); - - assert!(store - .get_custom_value( - format_storage_key_for_sliding_sync_list("hello", "list_bar").as_bytes() - ) - .await? - .is_none()); - - // Create a new `SlidingSync` instance, and store it. + // Modify both lists, so we can check expected caching behavior later. { - let sliding_sync = client - .sliding_sync() - .storage_key(Some("hello".to_owned())) - .add_cached_list(SlidingSyncList::builder("list_foo")) - .await? - .add_list(SlidingSyncList::builder("list_bar")) - .build() - .await?; + let lists = sliding_sync.inner.lists.write().await; - // Modify both lists, so we can check expected caching behavior later. - { - let lists = sliding_sync.inner.lists.write().unwrap(); + let list_foo = lists.get("list_foo").unwrap(); + list_foo.set_maximum_number_of_rooms(Some(42)); - let list_foo = lists.get("list_foo").unwrap(); - list_foo.set_maximum_number_of_rooms(Some(42)); - - let list_bar = lists.get("list_bar").unwrap(); - list_bar.set_maximum_number_of_rooms(Some(1337)); - } - - assert!(sliding_sync.cache_to_storage().await.is_ok()); + let list_bar = lists.get("list_bar").unwrap(); + list_bar.set_maximum_number_of_rooms(Some(1337)); } - // Store entries now exist for the sliding sync object and list_foo. - assert!(store - .get_custom_value(format_storage_key_for_sliding_sync("hello").as_bytes()) - .await? - .is_some()); + assert!(sliding_sync.cache_to_storage().await.is_ok()); + } - assert!(store - .get_custom_value( - format_storage_key_for_sliding_sync_list("hello", "list_foo").as_bytes() - ) - .await? - .is_some()); + // Store entries now exist for the sliding sync object and list_foo. + assert!(store + .get_custom_value(format_storage_key_for_sliding_sync("hello").as_bytes()) + .await? + .is_some()); - // But not for list_bar. - assert!(store - .get_custom_value( - format_storage_key_for_sliding_sync_list("hello", "list_bar").as_bytes() - ) - .await? - .is_none()); + assert!(store + .get_custom_value( + format_storage_key_for_sliding_sync_list("hello", "list_foo").as_bytes() + ) + .await? + .is_some()); - // Create a new `SlidingSync`, and it should be read from the cache. + // But not for list_bar. + assert!(store + .get_custom_value( + format_storage_key_for_sliding_sync_list("hello", "list_bar").as_bytes() + ) + .await? + .is_none()); + + // Create a new `SlidingSync`, and it should be read from the cache. + { + let max_number_of_room_stream = Arc::new(RwLock::new(None)); + let cloned_stream = max_number_of_room_stream.clone(); + let sliding_sync = client + .sliding_sync() + .storage_key(Some("hello".to_owned())) + .add_cached_list(SlidingSyncList::builder("list_foo").once_built(move |list| { + // In the `once_built()` handler, nothing has been read from the cache yet. + assert_eq!(list.maximum_number_of_rooms(), None); + + let mut stream = cloned_stream.write().unwrap(); + *stream = Some(list.maximum_number_of_rooms_stream()); + list + })) + .await? + .add_list(SlidingSyncList::builder("list_bar")) + .build() + .await?; + + // Check the list' state. { - let max_number_of_room_stream = Arc::new(RwLock::new(None)); - let cloned_stream = max_number_of_room_stream.clone(); - let sliding_sync = client - .sliding_sync() - .storage_key(Some("hello".to_owned())) - .add_cached_list(SlidingSyncList::builder("list_foo").once_built(move |list| { - // In the `once_built()` handler, nothing has been read from the cache yet. - assert_eq!(list.maximum_number_of_rooms(), None); + let lists = sliding_sync.inner.lists.write().await; - let mut stream = cloned_stream.write().unwrap(); - *stream = Some(list.maximum_number_of_rooms_stream()); - list - })) - .await? - .add_list(SlidingSyncList::builder("list_bar")) - .build() - .await?; + // This one was cached. + let list_foo = lists.get("list_foo").unwrap(); + assert_eq!(list_foo.maximum_number_of_rooms(), Some(42)); - // Check the list' state. - { - let lists = sliding_sync.inner.lists.write().unwrap(); - - // This one was cached. - let list_foo = lists.get("list_foo").unwrap(); - assert_eq!(list_foo.maximum_number_of_rooms(), Some(42)); - - // This one wasn't. - let list_bar = lists.get("list_bar").unwrap(); - assert_eq!(list_bar.maximum_number_of_rooms(), None); - } - - // The maximum number of rooms reloaded from the cache should have been - // published. - { - let mut stream = max_number_of_room_stream - .write() - .unwrap() - .take() - .expect("stream must be set"); - let initial_max_number_of_rooms = - stream.next().await.expect("stream must have emitted something"); - assert_eq!(initial_max_number_of_rooms, Some(42)); - } - - // Clean the cache. - clean_storage(&client, "hello", &sliding_sync.inner.lists.read().unwrap()).await; + // This one wasn't. + let list_bar = lists.get("list_bar").unwrap(); + assert_eq!(list_bar.maximum_number_of_rooms(), None); } - // Store entries don't exist. - assert!(store - .get_custom_value(format_storage_key_for_sliding_sync("hello").as_bytes()) - .await? - .is_none()); + // The maximum number of rooms reloaded from the cache should have been + // published. + { + let mut stream = + max_number_of_room_stream.write().unwrap().take().expect("stream must be set"); + let initial_max_number_of_rooms = + stream.next().await.expect("stream must have emitted something"); + assert_eq!(initial_max_number_of_rooms, Some(42)); + } - assert!(store - .get_custom_value( - format_storage_key_for_sliding_sync_list("hello", "list_foo").as_bytes() - ) - .await? - .is_none()); + // Clean the cache. + let lists = sliding_sync.inner.lists.read().await; - assert!(store - .get_custom_value( - format_storage_key_for_sliding_sync_list("hello", "list_bar").as_bytes() - ) - .await? - .is_none()); + clean_storage(&client, "hello", &lists).await; + } - Ok(()) - }) + // Store entries don't exist. + assert!(store + .get_custom_value(format_storage_key_for_sliding_sync("hello").as_bytes()) + .await? + .is_none()); + + assert!(store + .get_custom_value( + format_storage_key_for_sliding_sync_list("hello", "list_foo").as_bytes() + ) + .await? + .is_none()); + + assert!(store + .get_custom_value( + format_storage_key_for_sliding_sync_list("hello", "list_bar").as_bytes() + ) + .await? + .is_none()); + + Ok(()) } } diff --git a/crates/matrix-sdk/src/sliding_sync/list/mod.rs b/crates/matrix-sdk/src/sliding_sync/list/mod.rs index b429a4fca..602104f5f 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/mod.rs @@ -70,7 +70,10 @@ impl SlidingSyncList { /// means that the state is not reset **purposely**. The ranges and the /// state will be updated when the next request will be sent and a /// response will be received. The maximum number of rooms won't change. - pub fn set_sync_mode(&self, sync_mode: impl Into) { + pub fn set_sync_mode(&self, sync_mode: M) + where + M: Into, + { self.inner.set_sync_mode(sync_mode.into()); // When the sync mode is changed, the sync loop must skip over any work in its @@ -837,6 +840,7 @@ mod tests { use futures_util::StreamExt; use imbl::vector; + use matrix_sdk_test::async_test; use ruma::{api::client::sync::sync_events::v4::SlidingOp, room_id, uint}; use serde_json::json; use tokio::{ @@ -859,8 +863,8 @@ mod tests { }; } - #[test] - fn test_sliding_sync_list_selective_mode() { + #[async_test] + async fn test_sliding_sync_list_selective_mode() { let (sender, mut receiver) = channel(1); // Set range on `Selective`. @@ -879,7 +883,7 @@ mod tests { // There shouldn't be any internal request to restart the sync loop yet. assert!(matches!(receiver.try_recv(), Err(TryRecvError::Empty))); - list.set_sync_mode(SlidingSyncSelectiveModeBuilder::new().add_range(4..=5)); + list.set_sync_mode(SlidingSyncMode::new_selective().add_range(4..=5)); { let mut generator = list.inner.request_generator.write().unwrap(); @@ -902,7 +906,7 @@ mod tests { let (sender, _receiver) = channel(1); let list = SlidingSyncList::builder("foo") - .sync_mode(SlidingSyncSelectiveModeBuilder::new().add_range(0..=1)) + .sync_mode(SlidingSyncMode::new_selective().add_range(0..=1)) .timeline_limit(7) .build(sender); @@ -920,7 +924,7 @@ mod tests { let (sender, _receiver) = channel(1); let mut list = SlidingSyncList::builder("foo") - .sync_mode(SlidingSyncSelectiveModeBuilder::new().add_range(0..=1)) + .sync_mode(SlidingSyncMode::new_selective().add_range(0..=1)) .build(sender); let room0 = room_id!("!room0:bar.org"); @@ -1164,7 +1168,7 @@ mod tests { let (sender, _receiver) = channel(1); let mut list = SlidingSyncList::builder("testing") - .sync_mode(SlidingSyncSelectiveModeBuilder::new().add_range(0..=10).add_range(42..=153)) + .sync_mode(SlidingSyncMode::new_selective().add_range(0..=10).add_range(42..=153)) .build(sender); assert_ranges! { @@ -1191,12 +1195,12 @@ mod tests { }; } - #[test] - fn test_generator_selective_with_modifying_ranges_on_the_fly() { + #[tokio::test] + async fn test_generator_selective_with_modifying_ranges_on_the_fly() { let (sender, _receiver) = channel(4); let mut list = SlidingSyncList::builder("testing") - .sync_mode(SlidingSyncSelectiveModeBuilder::new().add_range(0..=10).add_range(42..=153)) + .sync_mode(SlidingSyncMode::new_selective().add_range(0..=10).add_range(42..=153)) .build(sender); assert_ranges! { @@ -1222,7 +1226,7 @@ mod tests { } }; - list.set_sync_mode(SlidingSyncSelectiveModeBuilder::new().add_range(3..=7)); + list.set_sync_mode(SlidingSyncMode::new_selective().add_range(3..=7)); assert_ranges! { list = list, @@ -1235,7 +1239,7 @@ mod tests { }, }; - list.set_sync_mode(SlidingSyncSelectiveModeBuilder::new().add_range(42..=77)); + list.set_sync_mode(SlidingSyncMode::new_selective().add_range(42..=77)); assert_ranges! { list = list, @@ -1248,7 +1252,7 @@ mod tests { }, }; - list.set_sync_mode(SlidingSyncSelectiveModeBuilder::new()); + list.set_sync_mode(SlidingSyncMode::new_selective()); assert_ranges! { list = list, @@ -1262,12 +1266,12 @@ mod tests { }; } - #[test] - fn test_generator_changing_sync_mode_to_various_modes() { + #[async_test] + async fn test_generator_changing_sync_mode_to_various_modes() { let (sender, _receiver) = channel(4); let mut list = SlidingSyncList::builder("testing") - .sync_mode(SlidingSyncSelectiveModeBuilder::new().add_range(0..=10).add_range(42..=153)) + .sync_mode(SlidingSyncMode::new_selective().add_range(0..=10).add_range(42..=153)) .build(sender); assert_ranges! { @@ -1366,14 +1370,14 @@ mod tests { }; // Changing from `Paging` to `Selective`. - list.set_sync_mode(SlidingSyncSelectiveModeBuilder::new()); + list.set_sync_mode(SlidingSyncMode::new_selective()); assert_eq!(list.state(), SlidingSyncState::PartiallyLoaded); // we had some partial state, but we can't be sure it's fully loaded until the // next request // We need to update the ranges, of course, as they are not managed // automatically anymore. - list.set_sync_mode(SlidingSyncSelectiveModeBuilder::new().add_range(0..=100)); + list.set_sync_mode(SlidingSyncMode::new_selective().add_range(0..=100)); assert_ranges! { list = list, @@ -1405,7 +1409,7 @@ mod tests { let (sender, _receiver) = channel(1); let mut list = SlidingSyncList::builder("foo") - .sync_mode(SlidingSyncSelectiveModeBuilder::new().add_range(0..=3)) + .sync_mode(SlidingSyncMode::new_selective().add_range(0..=3)) .build(sender); assert_eq!(**list.inner.maximum_number_of_rooms.read().unwrap(), None); diff --git a/crates/matrix-sdk/src/sliding_sync/mod.rs b/crates/matrix-sdk/src/sliding_sync/mod.rs index 7691bed6c..7e7c55301 100644 --- a/crates/matrix-sdk/src/sliding_sync/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/mod.rs @@ -25,6 +25,7 @@ mod room; use std::{ collections::{BTreeMap, BTreeSet}, fmt::Debug, + future::Future, sync::{ atomic::{AtomicU8, Ordering}, Arc, RwLock as StdRwLock, @@ -51,7 +52,7 @@ use ruma::{ use serde::{Deserialize, Serialize}; use tokio::{ select, spawn, - sync::{broadcast::Sender, Mutex as AsyncMutex}, + sync::{broadcast::Sender, Mutex as AsyncMutex, RwLock as AsyncRwLock}, }; use tracing::{debug, error, instrument, warn, Instrument, Span}; use url::Url; @@ -94,10 +95,10 @@ pub(super) struct SlidingSyncInner { position: StdRwLock, /// The lists of this Sliding Sync instance. - lists: StdRwLock>, + lists: AsyncRwLock>, /// The rooms details - rooms: StdRwLock>, + rooms: AsyncRwLock>, /// The `bump_event_types` field. See /// [`SlidingSyncBuilder::bump_event_types`] to learn more. @@ -164,12 +165,12 @@ impl SlidingSync { /// Lookup a specific room pub fn get_room(&self, room_id: &RoomId) -> Option { - self.inner.rooms.read().unwrap().get(room_id).cloned() + self.inner.rooms.blocking_read().get(room_id).cloned() } /// Check the number of rooms. pub fn get_number_of_rooms(&self) -> usize { - self.inner.rooms.read().unwrap().len() + self.inner.rooms.blocking_read().len() } #[instrument(skip(self))] @@ -178,13 +179,21 @@ impl SlidingSync { } /// Find a list by its name, and do something on it if it exists. - pub fn on_list(&self, list_name: &str, f: F) -> Option + pub async fn on_list( + &self, + list_name: &str, + function: Function, + ) -> Option where - F: FnOnce(&SlidingSyncList) -> R, + Function: FnOnce(&SlidingSyncList) -> FunctionOutput, + FunctionOutput: Future, { - let lists = self.inner.lists.read().unwrap(); + let lists = self.inner.lists.read().await; - lists.get(list_name).map(f) + match lists.get(list_name) { + Some(list) => Some(function(list).await), + None => None, + } } /// Add the list to the list of lists. @@ -198,7 +207,7 @@ impl SlidingSync { ) -> Result> { let list = list_builder.build(self.inner.internal_channel.clone()); - let old_list = self.inner.lists.write().unwrap().insert(list.name().to_owned(), list); + let old_list = self.inner.lists.write().await.insert(list.name().to_owned(), list); self.inner.internal_channel_send_if_possible( SlidingSyncInternalMessage::SyncLoopSkipOverCurrentIteration, @@ -225,7 +234,7 @@ impl SlidingSync { list_builder.set_cached_and_reload(&self.inner.client, storage_key).await?; if !reloaded_rooms.is_empty() { - let mut rooms = self.inner.rooms.write().unwrap(); + let mut rooms = self.inner.rooms.write().await; for (key, frozen) in reloaded_rooms { rooms.entry(key).or_insert_with(|| { @@ -242,14 +251,14 @@ impl SlidingSync { &self, room_ids: I, ) -> Vec> { - let rooms = self.inner.rooms.read().unwrap(); + let rooms = self.inner.rooms.blocking_read(); room_ids.map(|room_id| rooms.get(&room_id).cloned()).collect() } /// Get all rooms. pub fn get_all_rooms(&self) -> Vec { - self.inner.rooms.read().unwrap().values().cloned().collect() + self.inner.rooms.blocking_read().values().cloned().collect() } fn prepare_extension_config(&self, pos: Option<&str>) -> ExtensionsConfig { @@ -272,7 +281,7 @@ impl SlidingSync { } /// Handle the HTTP response. - #[instrument(skip_all, fields(lists = self.inner.lists.read().unwrap().len()))] + #[instrument(skip_all)] async fn handle_response( &self, sliding_sync_response: v4::Response, @@ -303,7 +312,7 @@ impl SlidingSync { let update_summary = { // Update the rooms. let updated_rooms = { - let mut rooms_map = self.inner.rooms.write().unwrap(); + let mut rooms_map = self.inner.rooms.write().await; let mut updated_rooms = Vec::with_capacity(sliding_sync_response.rooms.len()); @@ -347,7 +356,7 @@ impl SlidingSync { // Update the lists. let updated_lists = { let mut updated_lists = Vec::with_capacity(sliding_sync_response.lists.len()); - let mut lists = self.inner.lists.write().unwrap(); + let mut lists = self.inner.lists.write().await; for (name, updates) in sliding_sync_response.lists { let Some(list) = lists.get_mut(&name) else { @@ -385,7 +394,7 @@ impl SlidingSync { let mut requests_lists = BTreeMap::new(); { - let mut lists = self.inner.lists.write().unwrap(); + let mut lists = self.inner.lists.write().await; if lists.is_empty() { return Ok(None); @@ -446,7 +455,7 @@ impl SlidingSync { // coming from the `OlmMachine::outgoing_requests()` method. #[cfg(feature = "e2e-encryption")] let response = { - debug!("Sliding Sync is sending the request along with outgoing E2EE requests"); + debug!("Sliding Sync is sending the request along with outgoing E2EE requests"); let (e2ee_uploads, response) = futures_util::future::join(self.inner.client.send_outgoing_requests(), request) @@ -872,7 +881,7 @@ mod tests { ) .await?; - let lists = sliding_sync.inner.lists.read().unwrap(); + let lists = sliding_sync.inner.lists.read().await; assert!(lists.contains_key("foo")); assert!(lists.contains_key("bar")); From d1708ececf8cad59a91d8b32e975dc5daaf1da15 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Fri, 26 May 2023 09:55:32 +0200 Subject: [PATCH 07/48] feat(ui): Update according to the last patch. --- crates/matrix-sdk-ui/src/room_list/mod.rs | 11 ++++++++++- .../tests/integration/room_list.rs | 17 ++++++++++++++--- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 056d3608e..9a6cd4a94 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -144,7 +144,16 @@ struct ChangeAllRoomsListToSelectiveSyncMode; impl Action for ChangeAllRoomsListToSelectiveSyncMode { async fn run(&self, sliding_sync: &SlidingSync) -> Result<(), Error> { sliding_sync - .on_list(ALL_ROOMS_LIST_NAME, |list| list.set_sync_mode(SlidingSyncMode::new_growing(20, None))) + .on_list(ALL_ROOMS_LIST_NAME, |list| { + let list = list.clone(); + + async move { + list.set_sync_mode( + SlidingSyncMode::new_growing(20, None) + ).await + } + }) + .await .unwrap() // transform into a custom `MyError::UnknownList` ?; diff --git a/crates/matrix-sdk-ui/tests/integration/room_list.rs b/crates/matrix-sdk-ui/tests/integration/room_list.rs index f2773df0a..3bf2eb63c 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list.rs @@ -1,3 +1,5 @@ +use std::future::ready; + use anyhow::{Context, Result}; use futures_util::{pin_mut, StreamExt}; use matrix_sdk_test::async_test; @@ -68,8 +70,14 @@ async fn test_one_list() -> Result<()> { let (_, room_list) = new_room_list().await?; let sliding_sync = room_list.sliding_sync(); - assert_eq!(sliding_sync.on_list(room_list::ALL_ROOMS_LIST_NAME, |_list| ()), Some(())); - assert_eq!(sliding_sync.on_list(room_list::VISIBLE_ROOMS_LIST_NAME, |_list| ()), None); + assert_eq!( + sliding_sync.on_list(room_list::ALL_ROOMS_LIST_NAME, |_list| ready(())).await, + Some(()) + ); + assert_eq!( + sliding_sync.on_list(room_list::VISIBLE_ROOMS_LIST_NAME, |_list| ready(())).await, + None + ); Ok(()) } @@ -111,7 +119,10 @@ async fn test_foo() -> Result<()> { }; assert_eq!( - room_list.sliding_sync().on_list(room_list::VISIBLE_ROOMS_LIST_NAME, |_list| ()), + room_list + .sliding_sync() + .on_list(room_list::VISIBLE_ROOMS_LIST_NAME, |_list| ready(())) + .await, Some(()) ); From 4e07ac1c766140c1a054d47dd88fcbac01731bba Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Fri, 26 May 2023 11:21:03 +0200 Subject: [PATCH 08/48] test(ui): Improve testability of `RoomList`. --- crates/matrix-sdk-ui/src/room_list/mod.rs | 2 +- .../tests/integration/room_list.rs | 67 ++++++++++++------- 2 files changed, 44 insertions(+), 25 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 9a6cd4a94..7da3f0f0b 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -73,7 +73,7 @@ impl RoomList { } } -#[derive(Debug, Copy, Clone)] +#[derive(Copy, Clone, Debug, PartialEq)] pub enum State { Init, LoadFirstRooms, diff --git a/crates/matrix-sdk-ui/tests/integration/room_list.rs b/crates/matrix-sdk-ui/tests/integration/room_list.rs index 3bf2eb63c..def9b4de0 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list.rs @@ -5,6 +5,7 @@ use futures_util::{pin_mut, StreamExt}; use matrix_sdk_test::async_test; use matrix_sdk_ui::{room_list, RoomList}; use serde_json::json; +use tokio::{spawn, sync::mpsc::unbounded_channel}; use wiremock::{http::Method, Match, Mock, MockServer, Request, ResponseTemplate}; use crate::logged_in_client; @@ -28,25 +29,36 @@ impl Match for SlidingSyncMatcher { macro_rules! sync_then_assert_request_and_fake_response { ( - sync with $server:ident and $room_list_sync_stream:ident, + [$server:ident, $room_list:ident, $room_list_sync_stream:ident] + sync once, + states = $state_0:ident $( -> $state_n:ident )+, assert request = { $( $request_json:tt )* }, respond with = { $( $response_json:tt )* } $(,)? ) => { { - let matcher = SlidingSyncMatcher; - - let _mock_guard = Mock::given(matcher) + let _mock_guard = Mock::given(SlidingSyncMatcher) .respond_with(ResponseTemplate::new(200).set_body_json( json!({ $( $response_json )* }) )) .mount_as_scoped(&$server) .await; + assert_eq!(room_list::State:: $state_0, $room_list.state(), "pre state"); + + let mut states = $room_list.state_stream(); + let (state_sender, mut state_receiver) = unbounded_channel(); + + let state_listener = spawn(async move { + while let Some(state) = states.next().await { + state_sender.send(state).expect("sending state failed"); + } + }); + let next = $room_list_sync_stream.next().await.context("`sync` trip")??; for request in $server.received_requests().await.expect("Request recording has been disabled").iter().rev() { - if matcher.matches(request) { + if SlidingSyncMatcher.matches(request) { let json_value = serde_json::from_slice::(&request.body).unwrap(); if let Err(error) = assert_json_diff::assert_json_matches_no_panic( @@ -57,31 +69,26 @@ macro_rules! sync_then_assert_request_and_fake_response { dbg!(json_value); panic!("{}", error); } + + break; } } + $( + assert_eq!( + room_list::State:: $state_n , + state_receiver.recv().await.expect("receiving state failed"), + "next state", + ); + )+ + + state_listener.abort(); + next } }; } -#[async_test] -async fn test_one_list() -> Result<()> { - let (_, room_list) = new_room_list().await?; - let sliding_sync = room_list.sliding_sync(); - - assert_eq!( - sliding_sync.on_list(room_list::ALL_ROOMS_LIST_NAME, |_list| ready(())).await, - Some(()) - ); - assert_eq!( - sliding_sync.on_list(room_list::VISIBLE_ROOMS_LIST_NAME, |_list| ready(())).await, - None - ); - - Ok(()) -} - #[async_test] async fn test_foo() -> Result<()> { let (server, room_list) = new_room_list().await?; @@ -90,11 +97,15 @@ async fn test_foo() -> Result<()> { pin_mut!(sync); sync_then_assert_request_and_fake_response! { - sync with server and sync, + [server, room_list, sync] + sync once, + states = Init -> LoadFirstRooms -> LoadAllRooms, assert request = { "lists": { "all_rooms": { - "ranges": [], + "ranges": [ + [0, 20] + ], "required_state": [ ["m.room.encryption", ""] ], @@ -126,5 +137,13 @@ async fn test_foo() -> Result<()> { Some(()) ); + assert_eq!( + room_list + .sliding_sync() + .on_list(room_list::VISIBLE_ROOMS_LIST_NAME, |_list| ready(())) + .await, + Some(()) + ); + Ok(()) } From 01366a08df7a9ff2d763a385b59e2554134f8bdd Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Fri, 26 May 2023 12:05:48 +0200 Subject: [PATCH 09/48] !foo --- crates/matrix-sdk-ui/src/room_list/mod.rs | 67 +++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 7da3f0f0b..5c753fe07 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -202,3 +202,70 @@ impl Actions { self.actions.as_slice() } } + +#[cfg(test)] +mod tests { + use matrix_sdk::{config::RequestConfig, Session}; + use matrix_sdk_test::async_test; + use ruma::{api::MatrixVersion, device_id, user_id}; + use wiremock::MockServer; + + use super::*; + + async fn new_client() -> (Client, MockServer) { + let session = Session { + access_token: "1234".to_owned(), + refresh_token: None, + user_id: user_id!("@example:localhost").to_owned(), + device_id: device_id!("DEVICEID").to_owned(), + }; + + let server = MockServer::start().await; + let client = Client::builder() + .homeserver_url(server.uri()) + .server_versions([MatrixVersion::V1_0]) + .request_config(RequestConfig::new().disable_retry()) + .build() + .await + .unwrap(); + client.restore_session(session).await.unwrap(); + + (client, server) + } + + async fn new_room_list() -> RoomList { + let (client, _) = new_client().await; + + RoomList::new(client).await.unwrap() + } + + #[async_test] + async fn test_foo() -> Result<()> { + let room_list = new_room_list().await; + + Ok(()) + } + + #[async_test] + async fn test_states() -> Result<()> { + let (client, _) = new_client().await; + let sliding_sync = + client.sliding_sync().storage_key(Some("foo".to_string())).build().await?; + + let state = State::Init; + + let state = state.next(&sliding_sync).await?; + assert_eq!(state, State::LoadFirstRooms); + + let state = state.next(&sliding_sync).await?; + assert_eq!(state, State::LoadAllRooms); + + let state = state.next(&sliding_sync).await?; + assert_eq!(state, State::Enjoy); + + let state = state.next(&sliding_sync).await?; + assert_eq!(state, State::Enjoy); + + Ok(()) + } +} From f89060bbe9056fa0276a3621679ba313f3514316 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Fri, 26 May 2023 13:59:04 +0200 Subject: [PATCH 10/48] chore(ui): Update after the rebase. --- crates/matrix-sdk-ui/src/room_list/mod.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 5c753fe07..b3623eabe 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -4,9 +4,7 @@ use async_stream::stream; use async_trait::async_trait; use eyeball::shared::Observable; use futures_util::{pin_mut, Stream, StreamExt}; -use matrix_sdk::{ - Client, Error, Result, SlidingSync, SlidingSyncList, SlidingSyncMode, UpdateSummary, -}; +use matrix_sdk::{Client, Error, Result, SlidingSync, SlidingSyncList, SlidingSyncMode}; use once_cell::sync::Lazy; pub const ALL_ROOMS_LIST_NAME: &str = "all_rooms"; @@ -113,8 +111,7 @@ impl Action for AddAllRoomsList { sliding_sync .add_cached_list( SlidingSyncList::builder(ALL_ROOMS_LIST_NAME) - .sync_mode(SlidingSyncMode::new_selective()) - .add_range(0..=20), + .sync_mode(SlidingSyncMode::new_selective().add_range(0..=20)), ) .await?; From bd175d9f6672b33c8f5083d6db69d0bb522a7607 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Fri, 26 May 2023 15:18:32 +0200 Subject: [PATCH 11/48] test(sdk): Store the sync-mode in `SlidingSyncListInner` only for tests. --- .../matrix-sdk/src/sliding_sync/list/builder.rs | 3 +++ crates/matrix-sdk/src/sliding_sync/list/mod.rs | 17 ++++++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/matrix-sdk/src/sliding_sync/list/builder.rs b/crates/matrix-sdk/src/sliding_sync/list/builder.rs index 3a627823f..5b8bfafef 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/builder.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/builder.rs @@ -173,6 +173,9 @@ impl SlidingSyncListBuilder { ) -> SlidingSyncList { let list = SlidingSyncList { inner: Arc::new(SlidingSyncListInner { + #[cfg(any(test, feature = "testing"))] + sync_mode: StdRwLock::new(self.sync_mode.clone()), + // From the builder sort: self.sort, required_state: self.required_state, diff --git a/crates/matrix-sdk/src/sliding_sync/list/mod.rs b/crates/matrix-sdk/src/sliding_sync/list/mod.rs index 602104f5f..485984b62 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/mod.rs @@ -202,14 +202,21 @@ impl SlidingSyncList { } } -#[cfg(test)] +#[cfg(any(test, feature = "testing"))] +#[allow(dead_code)] impl SlidingSyncList { + /// Set the maximum number of rooms. pub(super) fn set_maximum_number_of_rooms(&self, maximum_number_of_rooms: Option) { Observable::set( &mut self.inner.maximum_number_of_rooms.write().unwrap(), maximum_number_of_rooms, ); } + + /// Get the sync-mode. + pub fn sync_mode(&self) -> SlidingSyncMode { + self.inner.sync_mode.read().unwrap().clone() + } } #[derive(Debug)] @@ -254,6 +261,9 @@ pub(super) struct SlidingSyncListInner { /// The Sliding Sync internal channel sender. See /// [`SlidingSyncInner::internal_channel`] to learn more. sliding_sync_internal_channel_sender: Sender, + + #[cfg(any(test, feature = "testing"))] + sync_mode: StdRwLock, } impl SlidingSyncListInner { @@ -264,6 +274,11 @@ impl SlidingSyncListInner { /// The [`Self::state`] is immediately updated to reflect the new state. The /// [`Self::maximum_number_of_rooms`] won't change. pub fn set_sync_mode(&self, sync_mode: SlidingSyncMode) { + #[cfg(any(test, feature = "testing"))] + { + *self.sync_mode.write().unwrap() = sync_mode.clone(); + } + { let mut request_generator = self.request_generator.write().unwrap(); *request_generator = SlidingSyncListRequestGenerator::new(sync_mode); From 26a764d9ddfdf5061ca76ee177031245f901fcdd Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Fri, 26 May 2023 15:19:07 +0200 Subject: [PATCH 12/48] test(ui): Test all `RoomList` actions. --- crates/matrix-sdk-ui/Cargo.toml | 2 +- crates/matrix-sdk-ui/src/room_list/mod.rs | 100 ++++++++++++++++-- .../tests/integration/room_list.rs | 2 +- 3 files changed, 94 insertions(+), 10 deletions(-) diff --git a/crates/matrix-sdk-ui/Cargo.toml b/crates/matrix-sdk-ui/Cargo.toml index 37ac6068a..611c486b5 100644 --- a/crates/matrix-sdk-ui/Cargo.toml +++ b/crates/matrix-sdk-ui/Cargo.toml @@ -14,7 +14,7 @@ rustls-tls = ["matrix-sdk/rustls-tls"] experimental-room-list = ["experimental-sliding-sync", "dep:async-stream"] experimental-sliding-sync = ["matrix-sdk/experimental-sliding-sync"] -testing = [] +testing = ["matrix-sdk/testing"] [dependencies] async-stream = { workspace = true, optional = true } diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index b3623eabe..96fbdbf6e 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -65,7 +65,7 @@ impl RoomList { Observable::subscribe(&self.state) } - #[cfg(feature = "testing")] + #[cfg(any(test, feature = "testing"))] pub fn sliding_sync(&self) -> &SlidingSync { &self.sliding_sync } @@ -111,7 +111,7 @@ impl Action for AddAllRoomsList { sliding_sync .add_cached_list( SlidingSyncList::builder(ALL_ROOMS_LIST_NAME) - .sync_mode(SlidingSyncMode::new_selective().add_range(0..=20)), + .sync_mode(SlidingSyncMode::new_selective().add_range(0..=19)), ) .await?; @@ -135,10 +135,10 @@ impl Action for AddVisibleRoomsList { } } -struct ChangeAllRoomsListToSelectiveSyncMode; +struct ChangeAllRoomsListToGrowingSyncMode; #[async_trait] -impl Action for ChangeAllRoomsListToSelectiveSyncMode { +impl Action for ChangeAllRoomsListToGrowingSyncMode { async fn run(&self, sliding_sync: &SlidingSync) -> Result<(), Error> { sliding_sync .on_list(ALL_ROOMS_LIST_NAME, |list| { @@ -192,7 +192,7 @@ impl Actions { actions! { none => [], start => [AddAllRoomsList], - first_rooms_are_loaded => [ChangeAllRoomsListToSelectiveSyncMode, AddVisibleRoomsList], + first_rooms_are_loaded => [ChangeAllRoomsListToGrowingSyncMode, AddVisibleRoomsList], } fn iter(&self) -> &[OneAction] { @@ -202,6 +202,8 @@ impl Actions { #[cfg(test)] mod tests { + use std::future::ready; + use matrix_sdk::{config::RequestConfig, Session}; use matrix_sdk_test::async_test; use ruma::{api::MatrixVersion, device_id, user_id}; @@ -245,9 +247,8 @@ mod tests { #[async_test] async fn test_states() -> Result<()> { - let (client, _) = new_client().await; - let sliding_sync = - client.sliding_sync().storage_key(Some("foo".to_string())).build().await?; + let room_list = new_room_list().await; + let sliding_sync = room_list.sliding_sync(); let state = State::Init; @@ -265,4 +266,87 @@ mod tests { Ok(()) } + + #[async_test] + async fn test_action_add_all_rooms_list() { + let room_list = new_room_list().await; + let sliding_sync = room_list.sliding_sync(); + + // List is absent. + assert_eq!(sliding_sync.on_list(ALL_ROOMS_LIST_NAME, |_list| ready(())).await, None); + + // Run the action! + AddAllRoomsList.run(sliding_sync).await.unwrap(); + + // List is present! + assert_eq!( + sliding_sync + .on_list(ALL_ROOMS_LIST_NAME, |list| ready(matches!( + list.sync_mode(), + SlidingSyncMode::Selective { ranges } if ranges == vec![0..=19] + ))) + .await, + Some(true) + ); + } + + #[async_test] + async fn test_action_add_visible_rooms_list() { + let room_list = new_room_list().await; + let sliding_sync = room_list.sliding_sync(); + + // List is absent. + assert_eq!(sliding_sync.on_list(VISIBLE_ROOMS_LIST_NAME, |_list| ready(())).await, None); + + // Run the action! + AddVisibleRoomsList.run(sliding_sync).await.unwrap(); + + // List is present! + assert_eq!( + sliding_sync + .on_list(VISIBLE_ROOMS_LIST_NAME, |list| ready(matches!( + list.sync_mode(), + SlidingSyncMode::Selective { ranges } if ranges.is_empty() + ))) + .await, + Some(true) + ); + } + + #[async_test] + async fn test_action_change_all_rooms_list_to_growing_sync_mode() { + let room_list = new_room_list().await; + let sliding_sync = room_list.sliding_sync(); + + // List is absent. + assert_eq!(sliding_sync.on_list(VISIBLE_ROOMS_LIST_NAME, |_list| ready(())).await, None); + + // Just create the list. + AddAllRoomsList.run(sliding_sync).await.unwrap(); + + // List is present, in Selective mode. + assert_eq!( + sliding_sync + .on_list(ALL_ROOMS_LIST_NAME, |list| ready(matches!( + list.sync_mode(), + SlidingSyncMode::Selective { ranges } if ranges == vec![0..=19] + ))) + .await, + Some(true) + ); + + // Run the action! + ChangeAllRoomsListToGrowingSyncMode.run(sliding_sync).await.unwrap(); + + // List is still present, in Growing mode. + assert_eq!( + sliding_sync + .on_list(ALL_ROOMS_LIST_NAME, |list| ready(matches!( + list.sync_mode(), + SlidingSyncMode::Growing { batch_size: 20, .. } + ))) + .await, + Some(true) + ); + } } diff --git a/crates/matrix-sdk-ui/tests/integration/room_list.rs b/crates/matrix-sdk-ui/tests/integration/room_list.rs index def9b4de0..24c8b901c 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list.rs @@ -104,7 +104,7 @@ async fn test_foo() -> Result<()> { "lists": { "all_rooms": { "ranges": [ - [0, 20] + [0, 19] ], "required_state": [ ["m.room.encryption", ""] From 4d6a6ac17f7c0f20e6d98e4caeb5409bf707eb34 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 31 May 2023 10:47:01 +0200 Subject: [PATCH 13/48] feat(ui) Add `State::Terminated` in `RoomList`. --- crates/matrix-sdk-ui/src/room_list/mod.rs | 52 ++++++++++++++++------- 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 96fbdbf6e..176c3a1fd 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -41,19 +41,27 @@ impl RoomList { } while let Some(update_summary) = sync.next().await { - { - let next_state = self.state.read().next(&self.sliding_sync).await?; + match update_summary { + Ok(_update_summary) => { + { + let next_state = self.state.read().next(&self.sliding_sync).await?; - Observable::set(&self.state, next_state); + Observable::set(&self.state, next_state); + } + + yield Ok(()); + } + + Err(error) => { + yield Err(error); + } } - // if let State::Failure = state { - // yield Err(()); - // // transform into a custom `MyError::SyncFailure` - // } else { - yield Ok(()); - // } } + + let next_state = State::Terminated; + + Observable::set(&self.state, next_state); } } @@ -73,10 +81,23 @@ impl RoomList { #[derive(Copy, Clone, Debug, PartialEq)] pub enum State { + /// That's the first initial state. The next state is calculated before + /// doing the first sync. Init, + + /// At this state, the first rooms starts to be synced. LoadFirstRooms, + + /// At this state, all rooms starts to be synced. LoadAllRooms, + + /// This state is the cruising speed, i.e. the “normal” state, where nothing + /// fancy happens: all rooms are syncing, and life is great. Enjoy, + + /// At this state, the sync has been stopped (because it was requested, or + /// because it has errored too many times previously). + Terminated, } impl State { @@ -88,6 +109,7 @@ impl State { LoadFirstRooms => (LoadAllRooms, Actions::first_rooms_are_loaded()), LoadAllRooms => (Enjoy, Actions::none()), Enjoy => (Enjoy, Actions::none()), + Terminated => (Terminated, Actions::none()), }; for action in actions.iter() { @@ -238,13 +260,6 @@ mod tests { RoomList::new(client).await.unwrap() } - #[async_test] - async fn test_foo() -> Result<()> { - let room_list = new_room_list().await; - - Ok(()) - } - #[async_test] async fn test_states() -> Result<()> { let room_list = new_room_list().await; @@ -264,6 +279,11 @@ mod tests { let state = state.next(&sliding_sync).await?; assert_eq!(state, State::Enjoy); + let state = State::Terminated; + + let state = state.next(&sliding_sync).await?; + assert_eq!(state, State::Terminated); + Ok(()) } From 61f903a5a9bafc8ad37fb6c03fc59a83009fbefb Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 31 May 2023 10:47:29 +0200 Subject: [PATCH 14/48] test(ui): Clean up a test. --- .../tests/integration/room_list.rs | 20 +------------------ 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/crates/matrix-sdk-ui/tests/integration/room_list.rs b/crates/matrix-sdk-ui/tests/integration/room_list.rs index 24c8b901c..a5635fbbf 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list.rs @@ -1,5 +1,3 @@ -use std::future::ready; - use anyhow::{Context, Result}; use futures_util::{pin_mut, StreamExt}; use matrix_sdk_test::async_test; @@ -90,7 +88,7 @@ macro_rules! sync_then_assert_request_and_fake_response { } #[async_test] -async fn test_foo() -> Result<()> { +async fn test_init_to_load_first_rooms_to_load_all_rooms() -> Result<()> { let (server, room_list) = new_room_list().await?; let sync = room_list.sync(); @@ -129,21 +127,5 @@ async fn test_foo() -> Result<()> { }, }; - assert_eq!( - room_list - .sliding_sync() - .on_list(room_list::VISIBLE_ROOMS_LIST_NAME, |_list| ready(())) - .await, - Some(()) - ); - - assert_eq!( - room_list - .sliding_sync() - .on_list(room_list::VISIBLE_ROOMS_LIST_NAME, |_list| ready(())) - .await, - Some(()) - ); - Ok(()) } From 1173636b359c7029723e0a59816e72f7a6dc9f74 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 31 May 2023 11:12:12 +0200 Subject: [PATCH 15/48] feat(ui): Add `room_list::Error`. --- crates/matrix-sdk-ui/src/room_list/mod.rs | 71 ++++++++++++------- .../tests/integration/room_list.rs | 16 +++-- 2 files changed, 54 insertions(+), 33 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 176c3a1fd..ed7214287 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -4,8 +4,11 @@ use async_stream::stream; use async_trait::async_trait; use eyeball::shared::Observable; use futures_util::{pin_mut, Stream, StreamExt}; -use matrix_sdk::{Client, Error, Result, SlidingSync, SlidingSyncList, SlidingSyncMode}; +use matrix_sdk::{ + Client, Error as SlidingSyncError, SlidingSync, SlidingSyncList, SlidingSyncMode, +}; use once_cell::sync::Lazy; +use thiserror::Error; pub const ALL_ROOMS_LIST_NAME: &str = "all_rooms"; pub const VISIBLE_ROOMS_LIST_NAME: &str = "visible_rooms"; @@ -17,13 +20,14 @@ pub struct RoomList { } impl RoomList { - pub async fn new(client: Client) -> Result { + pub async fn new(client: Client) -> Result { Ok(Self { sliding_sync: client .sliding_sync() .storage_key(Some("matrix-sdk-ui-roomlist".to_string())) .build() - .await?, + .await + .map_err(Error::SlidingSync)?, state: Observable::new(State::Init), }) } @@ -53,7 +57,7 @@ impl RoomList { } Err(error) => { - yield Err(error); + yield Err(Error::SlidingSync(error)); } } @@ -79,6 +83,17 @@ impl RoomList { } } +#[derive(Debug, Error)] +pub enum Error { + /// Error from [`matrix_sdk::SlidingSync`]. + #[error("SlidingSync failed")] + SlidingSync(SlidingSyncError), + + /// An operation has been requested on an unknown list. + #[error("Unknown list `{0}`")] + UnknownList(String), +} + #[derive(Copy, Clone, Debug, PartialEq)] pub enum State { /// That's the first initial state. The next state is calculated before @@ -135,7 +150,8 @@ impl Action for AddAllRoomsList { SlidingSyncList::builder(ALL_ROOMS_LIST_NAME) .sync_mode(SlidingSyncMode::new_selective().add_range(0..=19)), ) - .await?; + .await + .map_err(Error::SlidingSync)?; Ok(()) } @@ -151,7 +167,8 @@ impl Action for AddVisibleRoomsList { SlidingSyncList::builder(VISIBLE_ROOMS_LIST_NAME) .sync_mode(SlidingSyncMode::new_selective()), ) - .await?; + .await + .map_err(Error::SlidingSync)?; Ok(()) } @@ -166,15 +183,11 @@ impl Action for ChangeAllRoomsListToGrowingSyncMode { .on_list(ALL_ROOMS_LIST_NAME, |list| { let list = list.clone(); - async move { - list.set_sync_mode( - SlidingSyncMode::new_growing(20, None) - ).await - } + async move { list.set_sync_mode(SlidingSyncMode::new_growing(20, None)).await } }) .await - .unwrap() // transform into a custom `MyError::UnknownList` - ?; + .ok_or_else(|| Error::UnknownList(ALL_ROOMS_LIST_NAME.to_string()))? + .map_err(Error::SlidingSync)?; Ok(()) } @@ -254,15 +267,15 @@ mod tests { (client, server) } - async fn new_room_list() -> RoomList { + async fn new_room_list() -> Result { let (client, _) = new_client().await; - RoomList::new(client).await.unwrap() + RoomList::new(client).await } #[async_test] - async fn test_states() -> Result<()> { - let room_list = new_room_list().await; + async fn test_states() -> Result<(), Error> { + let room_list = new_room_list().await?; let sliding_sync = room_list.sliding_sync(); let state = State::Init; @@ -288,15 +301,15 @@ mod tests { } #[async_test] - async fn test_action_add_all_rooms_list() { - let room_list = new_room_list().await; + async fn test_action_add_all_rooms_list() -> Result<(), Error> { + let room_list = new_room_list().await?; let sliding_sync = room_list.sliding_sync(); // List is absent. assert_eq!(sliding_sync.on_list(ALL_ROOMS_LIST_NAME, |_list| ready(())).await, None); // Run the action! - AddAllRoomsList.run(sliding_sync).await.unwrap(); + AddAllRoomsList.run(sliding_sync).await?; // List is present! assert_eq!( @@ -308,18 +321,20 @@ mod tests { .await, Some(true) ); + + Ok(()) } #[async_test] - async fn test_action_add_visible_rooms_list() { - let room_list = new_room_list().await; + async fn test_action_add_visible_rooms_list() -> Result<(), Error> { + let room_list = new_room_list().await?; let sliding_sync = room_list.sliding_sync(); // List is absent. assert_eq!(sliding_sync.on_list(VISIBLE_ROOMS_LIST_NAME, |_list| ready(())).await, None); // Run the action! - AddVisibleRoomsList.run(sliding_sync).await.unwrap(); + AddVisibleRoomsList.run(sliding_sync).await?; // List is present! assert_eq!( @@ -331,18 +346,20 @@ mod tests { .await, Some(true) ); + + Ok(()) } #[async_test] - async fn test_action_change_all_rooms_list_to_growing_sync_mode() { - let room_list = new_room_list().await; + async fn test_action_change_all_rooms_list_to_growing_sync_mode() -> Result<(), Error> { + let room_list = new_room_list().await?; let sliding_sync = room_list.sliding_sync(); // List is absent. assert_eq!(sliding_sync.on_list(VISIBLE_ROOMS_LIST_NAME, |_list| ready(())).await, None); // Just create the list. - AddAllRoomsList.run(sliding_sync).await.unwrap(); + AddAllRoomsList.run(sliding_sync).await?; // List is present, in Selective mode. assert_eq!( @@ -368,5 +385,7 @@ mod tests { .await, Some(true) ); + + Ok(()) } } diff --git a/crates/matrix-sdk-ui/tests/integration/room_list.rs b/crates/matrix-sdk-ui/tests/integration/room_list.rs index a5635fbbf..d66f8cba7 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list.rs @@ -1,14 +1,16 @@ -use anyhow::{Context, Result}; use futures_util::{pin_mut, StreamExt}; use matrix_sdk_test::async_test; -use matrix_sdk_ui::{room_list, RoomList}; +use matrix_sdk_ui::{ + room_list::{Error, State}, + RoomList, +}; use serde_json::json; use tokio::{spawn, sync::mpsc::unbounded_channel}; use wiremock::{http::Method, Match, Mock, MockServer, Request, ResponseTemplate}; use crate::logged_in_client; -async fn new_room_list() -> Result<(MockServer, RoomList)> { +async fn new_room_list() -> Result<(MockServer, RoomList), Error> { let (client, server) = logged_in_client().await; let room_list = RoomList::new(client).await?; @@ -42,7 +44,7 @@ macro_rules! sync_then_assert_request_and_fake_response { .mount_as_scoped(&$server) .await; - assert_eq!(room_list::State:: $state_0, $room_list.state(), "pre state"); + assert_eq!(State:: $state_0, $room_list.state(), "pre state"); let mut states = $room_list.state_stream(); let (state_sender, mut state_receiver) = unbounded_channel(); @@ -53,7 +55,7 @@ macro_rules! sync_then_assert_request_and_fake_response { } }); - let next = $room_list_sync_stream.next().await.context("`sync` trip")??; + let next = $room_list_sync_stream.next().await.unwrap()?; for request in $server.received_requests().await.expect("Request recording has been disabled").iter().rev() { if SlidingSyncMatcher.matches(request) { @@ -74,7 +76,7 @@ macro_rules! sync_then_assert_request_and_fake_response { $( assert_eq!( - room_list::State:: $state_n , + State:: $state_n , state_receiver.recv().await.expect("receiving state failed"), "next state", ); @@ -88,7 +90,7 @@ macro_rules! sync_then_assert_request_and_fake_response { } #[async_test] -async fn test_init_to_load_first_rooms_to_load_all_rooms() -> Result<()> { +async fn test_init_to_load_first_rooms_to_load_all_rooms() -> Result<(), Error> { let (server, room_list) = new_room_list().await?; let sync = room_list.sync(); From aa70c85e024a390347fde63fea6d284159c14f14 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 31 May 2023 11:36:40 +0200 Subject: [PATCH 16/48] test(ui): Test `RoomList` sync from `Init` to `Enjoy`. --- crates/matrix-sdk-ui/src/room_list/mod.rs | 4 +- .../tests/integration/room_list.rs | 111 +++++++++++++++++- 2 files changed, 107 insertions(+), 8 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index ed7214287..bece34c41 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -183,7 +183,7 @@ impl Action for ChangeAllRoomsListToGrowingSyncMode { .on_list(ALL_ROOMS_LIST_NAME, |list| { let list = list.clone(); - async move { list.set_sync_mode(SlidingSyncMode::new_growing(20, None)).await } + async move { list.set_sync_mode(SlidingSyncMode::new_growing(50, None)).await } }) .await .ok_or_else(|| Error::UnknownList(ALL_ROOMS_LIST_NAME.to_string()))? @@ -380,7 +380,7 @@ mod tests { sliding_sync .on_list(ALL_ROOMS_LIST_NAME, |list| ready(matches!( list.sync_mode(), - SlidingSyncMode::Growing { batch_size: 20, .. } + SlidingSyncMode::Growing { batch_size: 50, .. } ))) .await, Some(true) diff --git a/crates/matrix-sdk-ui/tests/integration/room_list.rs b/crates/matrix-sdk-ui/tests/integration/room_list.rs index d66f8cba7..d50f14d9b 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list.rs @@ -1,7 +1,9 @@ use futures_util::{pin_mut, StreamExt}; use matrix_sdk_test::async_test; use matrix_sdk_ui::{ - room_list::{Error, State}, + room_list::{ + Error, State, ALL_ROOMS_LIST_NAME as ALL_ROOMS, VISIBLE_ROOMS_LIST_NAME as VISIBLE_ROOMS, + }, RoomList, }; use serde_json::json; @@ -90,7 +92,7 @@ macro_rules! sync_then_assert_request_and_fake_response { } #[async_test] -async fn test_init_to_load_first_rooms_to_load_all_rooms() -> Result<(), Error> { +async fn test_init_to_enjoy() -> Result<(), Error> { let (server, room_list) = new_room_list().await?; let sync = room_list.sync(); @@ -102,7 +104,7 @@ async fn test_init_to_load_first_rooms_to_load_all_rooms() -> Result<(), Error> states = Init -> LoadFirstRooms -> LoadAllRooms, assert request = { "lists": { - "all_rooms": { + ALL_ROOMS: { "ranges": [ [0, 19] ], @@ -122,12 +124,109 @@ async fn test_init_to_load_first_rooms_to_load_all_rooms() -> Result<(), Error> } }, respond with = { - "pos": "foo", - "lists": {}, - "rooms": {}, + "pos": "0", + "lists": { + ALL_ROOMS: { + "count": 200, + "ops": [ + // let's ignore them for now + ] + } + }, + "rooms": { + // let's ignore them for now + }, "extensions": {}, }, }; + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + sync once, + states = LoadAllRooms -> Enjoy, + assert request = { + "lists": { + ALL_ROOMS: { + "ranges": [ + [0, 49] + ], + "required_state": [ + ["m.room.encryption", ""] + ], + "sort": ["by_recency", "by_name"], + }, + VISIBLE_ROOMS: { + "ranges": [], + "required_state": [ + ["m.room.encryption", ""] + ], + "sort": ["by_recency", "by_name"], + } + } + }, + respond with = { + "pos": "1", + "lists": { + ALL_ROOMS: { + "count": 200, + "ops": [ + // let's ignore them for now + ] + }, + VISIBLE_ROOMS: { + "count": 0, + "ops": [], + } + }, + "rooms": { + // let's ignore them for now + }, + }, + }; + + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + sync once, + states = Enjoy -> Enjoy, + assert request = { + "lists": { + ALL_ROOMS: { + "ranges": [ + [0, 99] + ], + "required_state": [ + ["m.room.encryption", ""] + ], + "sort": ["by_recency", "by_name"], + }, + VISIBLE_ROOMS: { + "ranges": [], + "required_state": [ + ["m.room.encryption", ""] + ], + "sort": ["by_recency", "by_name"], + } + } + }, + respond with = { + "pos": "2", + "lists": { + ALL_ROOMS: { + "count": 200, + "ops": [ + // let's ignore them for now + ] + }, + VISIBLE_ROOMS: { + "count": 0, + "ops": [], + } + }, + "rooms": { + // let's ignore them for now + }, + }, + }; + Ok(()) } From f058c595823e695f73fd0b8f6da28b4c0427104f Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 31 May 2023 13:54:39 +0200 Subject: [PATCH 17/48] feat(sdk): Add `SlidingSyncList::room_list_filtered_stream`. This patch adds a new method on `SlidingSyncList` named `room_list_filtered_stream`, which uses the new `eyeball-im-util` crate with its new `FilteredSubscriber` type. --- Cargo.lock | 13 +++++++++++++ Cargo.toml | 1 + crates/matrix-sdk/Cargo.toml | 3 ++- crates/matrix-sdk/src/sliding_sync/list/mod.rs | 18 ++++++++++++++++++ 4 files changed, 34 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index dffa536e6..a2ca53f31 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1530,6 +1530,18 @@ dependencies = [ "tokio-stream", ] +[[package]] +name = "eyeball-im-util" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "988b102aa389565187ccb138e51339c72258deb8af8e459180a582af40ca323b" +dependencies = [ + "eyeball-im", + "futures-core", + "imbl", + "pin-project-lite", +] + [[package]] name = "eyre" version = "0.6.8" @@ -2563,6 +2575,7 @@ dependencies = [ "event-listener", "eyeball", "eyeball-im", + "eyeball-im-util", "eyre", "futures-core", "futures-executor", diff --git a/Cargo.toml b/Cargo.toml index 13b163262..68365090a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,6 +30,7 @@ ctor = "0.2.0" dashmap = "5.2.0" eyeball = "0.7.0" eyeball-im = "0.2.0" +eyeball-im-util = "0.1.0" futures-core = "0.3.28" futures-executor = "0.3.21" futures-util = { version = "0.3.26", default-features = false, features = ["alloc"] } diff --git a/crates/matrix-sdk/Cargo.toml b/crates/matrix-sdk/Cargo.toml index 8d71e3e2a..56f52f286 100644 --- a/crates/matrix-sdk/Cargo.toml +++ b/crates/matrix-sdk/Cargo.toml @@ -47,7 +47,7 @@ appservice = ["ruma/appservice-api-s"] image-proc = ["dep:image"] image-rayon = ["image-proc", "image?/jpeg_rayon"] -experimental-sliding-sync = ["matrix-sdk-base/experimental-sliding-sync", "reqwest/gzip"] +experimental-sliding-sync = ["matrix-sdk-base/experimental-sliding-sync", "reqwest/gzip", "dep:eyeball-im-util"] docsrs = [ "e2e-encryption", @@ -68,6 +68,7 @@ dashmap = { workspace = true } event-listener = "2.5.2" eyeball = { workspace = true } eyeball-im = { workspace = true } +eyeball-im-util = { workspace = true, optional = true } eyre = { version = "0.6.8", optional = true } futures-core = { workspace = true } futures-util = { workspace = true } diff --git a/crates/matrix-sdk/src/sliding_sync/list/mod.rs b/crates/matrix-sdk/src/sliding_sync/list/mod.rs index 485984b62..f6d654fc2 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/mod.rs @@ -14,6 +14,7 @@ use std::{ pub use builder::*; use eyeball::unique::Observable; use eyeball_im::{ObservableVector, VectorDiff}; +use eyeball_im_util::VectorExt; pub(super) use frozen::FrozenSlidingSyncList; use futures_core::Stream; pub(super) use request_generator::*; @@ -128,6 +129,23 @@ impl SlidingSyncList { ObservableVector::subscribe(&self.inner.room_list.read().unwrap()) } + /// Get a stream of room list, but filtered. + /// + /// It's similar to [`Self::room_list_stream`] but the room list is filtered + /// by `filter`. + pub fn room_list_filtered_stream( + &self, + filter: F, + ) -> impl Stream> + where + F: Fn(&RoomListEntry) -> bool + Unpin, + { + let (_, stream) = + ObservableVector::subscribe_filtered(&self.inner.room_list.read().unwrap(), filter); + + stream + } + /// Get the maximum number of rooms. See [`Self::maximum_number_of_rooms`] /// to learn more. pub fn maximum_number_of_rooms(&self) -> Option { From 4557c2a7b20d86e0222a22f6ef5bda363438deef Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 31 May 2023 13:56:18 +0200 Subject: [PATCH 18/48] feat(ui): Add `RoomList::entries`. This patch implements the `RoomList::entries` method, which allows to get a stream of `VectorDiff` over the room list, and that can be filtered. --- Cargo.lock | 1 + crates/matrix-sdk-ui/Cargo.toml | 3 +- crates/matrix-sdk-ui/src/room_list/mod.rs | 132 ++++++++++-------- .../matrix-sdk/src/sliding_sync/list/mod.rs | 10 +- 4 files changed, 79 insertions(+), 67 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a2ca53f31..60541ac37 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3002,6 +3002,7 @@ dependencies = [ "ctor 0.2.0", "eyeball", "eyeball-im", + "eyeball-im-util", "futures-core", "futures-util", "imbl", diff --git a/crates/matrix-sdk-ui/Cargo.toml b/crates/matrix-sdk-ui/Cargo.toml index 611c486b5..54f7c58a5 100644 --- a/crates/matrix-sdk-ui/Cargo.toml +++ b/crates/matrix-sdk-ui/Cargo.toml @@ -11,7 +11,7 @@ e2e-encryption = ["matrix-sdk/e2e-encryption"] native-tls = ["matrix-sdk/native-tls"] rustls-tls = ["matrix-sdk/rustls-tls"] -experimental-room-list = ["experimental-sliding-sync", "dep:async-stream"] +experimental-room-list = ["experimental-sliding-sync", "dep:async-stream", "dep:eyeball-im-util"] experimental-sliding-sync = ["matrix-sdk/experimental-sliding-sync"] testing = ["matrix-sdk/testing"] @@ -22,6 +22,7 @@ async-trait = { workspace = true } chrono = "0.4.23" eyeball = { workspace = true } eyeball-im = { workspace = true } +eyeball-im-util = { workspace = true, optional = true } futures-core = { workspace = true } futures-util = { workspace = true } imbl = { version = "2.0.0", features = ["serde"] } diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index bece34c41..8e1a144de 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -1,11 +1,19 @@ //! `RoomList` API. +use std::{ + fmt, + future::ready, + sync::{Arc, Mutex}, +}; + use async_stream::stream; use async_trait::async_trait; use eyeball::shared::Observable; +use eyeball_im::VectorDiff; +use eyeball_im_util::FilteredVectorSubscriber; use futures_util::{pin_mut, Stream, StreamExt}; use matrix_sdk::{ - Client, Error as SlidingSyncError, SlidingSync, SlidingSyncList, SlidingSyncMode, + Client, Error as SlidingSyncError, RoomListEntry, SlidingSync, SlidingSyncList, SlidingSyncMode, }; use once_cell::sync::Lazy; use thiserror::Error; @@ -13,23 +21,53 @@ use thiserror::Error; pub const ALL_ROOMS_LIST_NAME: &str = "all_rooms"; pub const VISIBLE_ROOMS_LIST_NAME: &str = "visible_rooms"; -#[derive(Debug)] pub struct RoomList { sliding_sync: SlidingSync, + entries_stream: + FilteredVectorSubscriber bool + Sync + Send>>, state: Observable, } +impl fmt::Debug for RoomList { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .debug_struct("RoomList") + .field("sliding_sync", &self.sliding_sync) + .field("state", &self.state) + .finish_non_exhaustive() + } +} + impl RoomList { pub async fn new(client: Client) -> Result { - Ok(Self { - sliding_sync: client - .sliding_sync() - .storage_key(Some("matrix-sdk-ui-roomlist".to_string())) - .build() - .await - .map_err(Error::SlidingSync)?, - state: Observable::new(State::Init), - }) + let entries = Arc::new(Mutex::new(None)); + let sliding_sync = client + .sliding_sync() + .storage_key(Some("matrix-sdk-ui-roomlist".to_string())) + .add_cached_list( + SlidingSyncList::builder(ALL_ROOMS_LIST_NAME) + .sync_mode(SlidingSyncMode::new_selective().add_range(0..=19)) + .once_built({ + let entries = entries.clone(); + + move |list| { + *entries.lock().unwrap() = + Some(list.room_list_filtered_stream(Box::new(|_| true))); + + list + } + }), + ) + .await + .map_err(Error::SlidingSync)? + .build() + .await + .map_err(Error::SlidingSync)?; + + let entries = + Arc::try_unwrap(entries).map_err(|_| Error::Entries)?.into_inner().unwrap().unwrap(); + + Ok(Self { sliding_sync, entries_stream: entries, state: Observable::new(State::Init) }) } pub fn sync(&self) -> impl Stream> + '_ { @@ -92,6 +130,9 @@ pub enum Error { /// An operation has been requested on an unknown list. #[error("Unknown list `{0}`")] UnknownList(String), + + #[error("foo")] + Entries, } #[derive(Copy, Clone, Debug, PartialEq)] @@ -120,7 +161,7 @@ impl State { use State::*; let (next_state, actions) = match self { - Init => (LoadFirstRooms, Actions::start()), + Init => (LoadFirstRooms, Actions::none()), LoadFirstRooms => (LoadAllRooms, Actions::first_rooms_are_loaded()), LoadAllRooms => (Enjoy, Actions::none()), Enjoy => (Enjoy, Actions::none()), @@ -140,23 +181,6 @@ trait Action { async fn run(&self, sliding_sync: &SlidingSync) -> Result<(), Error>; } -struct AddAllRoomsList; - -#[async_trait] -impl Action for AddAllRoomsList { - async fn run(&self, sliding_sync: &SlidingSync) -> Result<(), Error> { - sliding_sync - .add_cached_list( - SlidingSyncList::builder(ALL_ROOMS_LIST_NAME) - .sync_mode(SlidingSyncMode::new_selective().add_range(0..=19)), - ) - .await - .map_err(Error::SlidingSync)?; - - Ok(()) - } -} - struct AddVisibleRoomsList; #[async_trait] @@ -226,7 +250,6 @@ macro_rules! actions { impl Actions { actions! { none => [], - start => [AddAllRoomsList], first_rooms_are_loaded => [ChangeAllRoomsListToGrowingSyncMode, AddVisibleRoomsList], } @@ -237,8 +260,6 @@ impl Actions { #[cfg(test)] mod tests { - use std::future::ready; - use matrix_sdk::{config::RequestConfig, Session}; use matrix_sdk_test::async_test; use ruma::{api::MatrixVersion, device_id, user_id}; @@ -273,6 +294,25 @@ mod tests { RoomList::new(client).await } + #[async_test] + async fn test_all_rooms_are_declared() -> Result<(), Error> { + let room_list = new_room_list().await?; + let sliding_sync = room_list.sliding_sync(); + + // List is present, in Selective mode. + assert_eq!( + sliding_sync + .on_list(ALL_ROOMS_LIST_NAME, |list| ready(matches!( + list.sync_mode(), + SlidingSyncMode::Selective { ranges } if ranges == vec![0..=19] + ))) + .await, + Some(true) + ); + + Ok(()) + } + #[async_test] async fn test_states() -> Result<(), Error> { let room_list = new_room_list().await?; @@ -300,31 +340,6 @@ mod tests { Ok(()) } - #[async_test] - async fn test_action_add_all_rooms_list() -> Result<(), Error> { - let room_list = new_room_list().await?; - let sliding_sync = room_list.sliding_sync(); - - // List is absent. - assert_eq!(sliding_sync.on_list(ALL_ROOMS_LIST_NAME, |_list| ready(())).await, None); - - // Run the action! - AddAllRoomsList.run(sliding_sync).await?; - - // List is present! - assert_eq!( - sliding_sync - .on_list(ALL_ROOMS_LIST_NAME, |list| ready(matches!( - list.sync_mode(), - SlidingSyncMode::Selective { ranges } if ranges == vec![0..=19] - ))) - .await, - Some(true) - ); - - Ok(()) - } - #[async_test] async fn test_action_add_visible_rooms_list() -> Result<(), Error> { let room_list = new_room_list().await?; @@ -358,9 +373,6 @@ mod tests { // List is absent. assert_eq!(sliding_sync.on_list(VISIBLE_ROOMS_LIST_NAME, |_list| ready(())).await, None); - // Just create the list. - AddAllRoomsList.run(sliding_sync).await?; - // List is present, in Selective mode. assert_eq!( sliding_sync diff --git a/crates/matrix-sdk/src/sliding_sync/list/mod.rs b/crates/matrix-sdk/src/sliding_sync/list/mod.rs index f6d654fc2..c37037037 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/mod.rs @@ -14,7 +14,7 @@ use std::{ pub use builder::*; use eyeball::unique::Observable; use eyeball_im::{ObservableVector, VectorDiff}; -use eyeball_im_util::VectorExt; +use eyeball_im_util::{FilteredVectorSubscriber, VectorExt}; pub(super) use frozen::FrozenSlidingSyncList; use futures_core::Stream; pub(super) use request_generator::*; @@ -133,12 +133,10 @@ impl SlidingSyncList { /// /// It's similar to [`Self::room_list_stream`] but the room list is filtered /// by `filter`. - pub fn room_list_filtered_stream( + pub fn room_list_filtered_stream( &self, - filter: F, - ) -> impl Stream> - where - F: Fn(&RoomListEntry) -> bool + Unpin, + filter: Box bool + Sync + Send>, + ) -> FilteredVectorSubscriber bool + Sync + Send>> { let (_, stream) = ObservableVector::subscribe_filtered(&self.inner.room_list.read().unwrap(), filter); From 1caa8b1aa66a96be406fd6e71529e9e58046c802 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 31 May 2023 17:18:22 +0200 Subject: [PATCH 19/48] chore(ui): Polish after the rebase. --- crates/matrix-sdk-ui/src/room_list/mod.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 8e1a144de..438de10aa 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -9,7 +9,6 @@ use std::{ use async_stream::stream; use async_trait::async_trait; use eyeball::shared::Observable; -use eyeball_im::VectorDiff; use eyeball_im_util::FilteredVectorSubscriber; use futures_util::{pin_mut, Stream, StreamExt}; use matrix_sdk::{ @@ -205,13 +204,10 @@ impl Action for ChangeAllRoomsListToGrowingSyncMode { async fn run(&self, sliding_sync: &SlidingSync) -> Result<(), Error> { sliding_sync .on_list(ALL_ROOMS_LIST_NAME, |list| { - let list = list.clone(); - - async move { list.set_sync_mode(SlidingSyncMode::new_growing(50, None)).await } + ready(list.set_sync_mode(SlidingSyncMode::new_growing(50))) }) .await - .ok_or_else(|| Error::UnknownList(ALL_ROOMS_LIST_NAME.to_string()))? - .map_err(Error::SlidingSync)?; + .ok_or_else(|| Error::UnknownList(ALL_ROOMS_LIST_NAME.to_string()))?; Ok(()) } From a3c96dcb62a8757a0c743a56639893e3fe651ab5 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 31 May 2023 18:45:25 +0200 Subject: [PATCH 20/48] feat(ui): Implement `RoomList::entries_stream`. --- Cargo.lock | 10 ++ Cargo.toml | 1 + crates/matrix-sdk-ui/Cargo.toml | 1 + crates/matrix-sdk-ui/src/room_list/mod.rs | 21 ++- .../tests/integration/room_list.rs | 138 ++++++++++++++---- .../src/sliding_sync/list/room_list_entry.rs | 2 +- 6 files changed, 137 insertions(+), 36 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 60541ac37..54a0aee16 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3016,6 +3016,7 @@ dependencies = [ "ruma", "serde", "serde_json", + "stream_assert", "thiserror", "tokio", "tracing", @@ -4889,6 +4890,15 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9091b6114800a5f2141aee1d1b9d6ca3592ac062dc5decb3764ec5895a47b4eb" +[[package]] +name = "stream_assert" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "58491272dc90918dba713fd9e3556a67a09e067621143f3616754788286489f1" +dependencies = [ + "futures-util", +] + [[package]] name = "string_cache" version = "0.8.7" diff --git a/Cargo.toml b/Cargo.toml index 68365090a..416ad2f67 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,6 +41,7 @@ once_cell = "1.16.0" serde = "1.0.151" serde_html_form = "0.2.0" serde_json = "1.0.91" +stream_assert = "0.1.0" thiserror = "1.0.38" tokio = { version = "1.24", default-features = false, features = ["sync"] } tracing = { version = "0.1.36", default-features = false, features = ["std"] } diff --git a/crates/matrix-sdk-ui/Cargo.toml b/crates/matrix-sdk-ui/Cargo.toml index 54f7c58a5..785df369f 100644 --- a/crates/matrix-sdk-ui/Cargo.toml +++ b/crates/matrix-sdk-ui/Cargo.toml @@ -45,6 +45,7 @@ assert-json-diff = "2.0" assert_matches = { workspace = true } ctor = { workspace = true } matrix-sdk-test = { version = "0.6.0", path = "../../testing/matrix-sdk-test" } +stream_assert = { workspace = true } tracing-subscriber = { version = "0.3.11", features = ["env-filter"] } wiremock = "0.5.13" diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 438de10aa..1ab60bbc1 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -3,16 +3,18 @@ use std::{ fmt, future::ready, - sync::{Arc, Mutex}, + sync::{Arc, Mutex, RwLock, RwLockWriteGuard}, }; use async_stream::stream; use async_trait::async_trait; use eyeball::shared::Observable; +use eyeball_im::VectorDiff; use eyeball_im_util::FilteredVectorSubscriber; use futures_util::{pin_mut, Stream, StreamExt}; +pub use matrix_sdk::RoomListEntry; use matrix_sdk::{ - Client, Error as SlidingSyncError, RoomListEntry, SlidingSync, SlidingSyncList, SlidingSyncMode, + Client, Error as SlidingSyncError, SlidingSync, SlidingSyncList, SlidingSyncMode, }; use once_cell::sync::Lazy; use thiserror::Error; @@ -22,8 +24,9 @@ pub const VISIBLE_ROOMS_LIST_NAME: &str = "visible_rooms"; pub struct RoomList { sliding_sync: SlidingSync, - entries_stream: + entries_stream: RwLock< FilteredVectorSubscriber bool + Sync + Send>>, + >, state: Observable, } @@ -66,7 +69,11 @@ impl RoomList { let entries = Arc::try_unwrap(entries).map_err(|_| Error::Entries)?.into_inner().unwrap().unwrap(); - Ok(Self { sliding_sync, entries_stream: entries, state: Observable::new(State::Init) }) + Ok(Self { + sliding_sync, + entries_stream: RwLock::new(entries), + state: Observable::new(State::Init), + }) } pub fn sync(&self) -> impl Stream> + '_ { @@ -114,6 +121,12 @@ impl RoomList { Observable::subscribe(&self.state) } + pub fn entries_stream( + &self, + ) -> RwLockWriteGuard>> { + self.entries_stream.write().unwrap() + } + #[cfg(any(test, feature = "testing"))] pub fn sliding_sync(&self) -> &SlidingSync { &self.sliding_sync diff --git a/crates/matrix-sdk-ui/tests/integration/room_list.rs b/crates/matrix-sdk-ui/tests/integration/room_list.rs index d50f14d9b..445e26e9f 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list.rs @@ -1,12 +1,18 @@ +use std::ops::DerefMut; + +use eyeball_im::VectorDiff; use futures_util::{pin_mut, StreamExt}; use matrix_sdk_test::async_test; use matrix_sdk_ui::{ room_list::{ - Error, State, ALL_ROOMS_LIST_NAME as ALL_ROOMS, VISIBLE_ROOMS_LIST_NAME as VISIBLE_ROOMS, + Error, RoomListEntry, State, ALL_ROOMS_LIST_NAME as ALL_ROOMS, + VISIBLE_ROOMS_LIST_NAME as VISIBLE_ROOMS, }, RoomList, }; +use ruma::room_id; use serde_json::json; +use stream_assert::*; use tokio::{spawn, sync::mpsc::unbounded_channel}; use wiremock::{http::Method, Match, Mock, MockServer, Request, ResponseTemplate}; @@ -106,13 +112,13 @@ async fn test_init_to_enjoy() -> Result<(), Error> { "lists": { ALL_ROOMS: { "ranges": [ - [0, 19] + [0, 19], ], "required_state": [ - ["m.room.encryption", ""] + ["m.room.encryption", ""], ], "sort": ["by_recency", "by_name"], - } + }, }, "extensions": { "e2ee": { @@ -120,8 +126,8 @@ async fn test_init_to_enjoy() -> Result<(), Error> { }, "to_device": { "enabled": true, - } - } + }, + }, }, respond with = { "pos": "0", @@ -130,8 +136,8 @@ async fn test_init_to_enjoy() -> Result<(), Error> { "count": 200, "ops": [ // let's ignore them for now - ] - } + ], + }, }, "rooms": { // let's ignore them for now @@ -148,17 +154,13 @@ async fn test_init_to_enjoy() -> Result<(), Error> { "lists": { ALL_ROOMS: { "ranges": [ - [0, 49] + [0, 49], ], - "required_state": [ - ["m.room.encryption", ""] - ], - "sort": ["by_recency", "by_name"], }, VISIBLE_ROOMS: { "ranges": [], "required_state": [ - ["m.room.encryption", ""] + ["m.room.encryption", ""], ], "sort": ["by_recency", "by_name"], } @@ -171,12 +173,12 @@ async fn test_init_to_enjoy() -> Result<(), Error> { "count": 200, "ops": [ // let's ignore them for now - ] + ], }, VISIBLE_ROOMS: { "count": 0, "ops": [], - } + }, }, "rooms": { // let's ignore them for now @@ -191,22 +193,12 @@ async fn test_init_to_enjoy() -> Result<(), Error> { assert request = { "lists": { ALL_ROOMS: { - "ranges": [ - [0, 99] - ], - "required_state": [ - ["m.room.encryption", ""] - ], - "sort": ["by_recency", "by_name"], + "ranges": [[0, 99]], }, VISIBLE_ROOMS: { "ranges": [], - "required_state": [ - ["m.room.encryption", ""] - ], - "sort": ["by_recency", "by_name"], - } - } + }, + }, }, respond with = { "pos": "2", @@ -215,12 +207,12 @@ async fn test_init_to_enjoy() -> Result<(), Error> { "count": 200, "ops": [ // let's ignore them for now - ] + ], }, VISIBLE_ROOMS: { "count": 0, "ops": [], - } + }, }, "rooms": { // let's ignore them for now @@ -230,3 +222,87 @@ async fn test_init_to_enjoy() -> Result<(), Error> { Ok(()) } + +#[async_test] +async fn test_entries_stream() -> Result<(), Error> { + let (server, room_list) = new_room_list().await?; + + let sync = room_list.sync(); + pin_mut!(sync); + + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + sync once, + states = Init -> LoadFirstRooms -> LoadAllRooms, + assert request = { + "lists": { + ALL_ROOMS: { + "ranges": [[0, 19]], + }, + }, + }, + respond with = { + "pos": "0", + "lists": { + ALL_ROOMS: { + "count": 10, + "ops": [ + { + "op": "SYNC", + "range": [0, 2], + "room_ids": [ + "!r0:bar.org", + "!r1:bar.org", + "!r2:bar.org", + ] + } + ] + } + }, + "rooms": { + "!r0:bar.org": { + "name": "Room #0", + "initial": true, + "timeline": [], + }, + "!r1:bar.org": { + "name": "Room #1", + "initial": true, + "timeline": [], + }, + "!r2:bar.org": { + "name": "Room #2", + "initial": true, + "timeline": [], + } + }, + }, + }; + + let mut entries = room_list.entries_stream(); + let entries = entries.deref_mut(); + pin_mut!(entries); + + assert_next_matches!(entries, VectorDiff::Append { values: _ }); + assert_next_matches!( + entries, + VectorDiff::Set { index: 0, value } => { + assert_eq!(value, RoomListEntry::Filled(room_id!("!r0:bar.org").to_owned())); + } + ); + assert_next_matches!( + entries, + VectorDiff::Set { index: 1, value } => { + assert_eq!(value, RoomListEntry::Filled(room_id!("!r1:bar.org").to_owned())); + } + ); + assert_next_matches!( + entries, + VectorDiff::Set { index: 2, value } => { + assert_eq!(value, RoomListEntry::Filled(room_id!("!r2:bar.org").to_owned())); + } + ); + assert_pending!(entries); + + Ok(()) +} diff --git a/crates/matrix-sdk/src/sliding_sync/list/room_list_entry.rs b/crates/matrix-sdk/src/sliding_sync/list/room_list_entry.rs index c84e5c579..1600eff41 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/room_list_entry.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/room_list_entry.rs @@ -3,7 +3,7 @@ use serde::{Deserialize, Serialize}; /// Represent a room entry in the [`SlidingSyncList`][super::SlidingSyncList]. #[derive(Clone, Debug, Default, Serialize, Deserialize)] -#[cfg_attr(test, derive(PartialEq))] +#[cfg_attr(any(test, feature = "testing"), derive(PartialEq))] pub enum RoomListEntry { /// This entry isn't known at this point and thus considered `Empty`. #[default] From 954d798ac3252bcefdf80c5f2098a02c8c5394a3 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 1 Jun 2023 08:33:11 +0200 Subject: [PATCH 21/48] test(ui): Write an `assert_entries_stream` macro. --- Cargo.lock | 10 -- Cargo.toml | 1 - crates/matrix-sdk-ui/Cargo.toml | 1 - .../tests/integration/room_list.rs | 136 ++++++++++++++---- .../matrix-sdk/src/sliding_sync/list/mod.rs | 1 - 5 files changed, 105 insertions(+), 44 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 54a0aee16..60541ac37 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3016,7 +3016,6 @@ dependencies = [ "ruma", "serde", "serde_json", - "stream_assert", "thiserror", "tokio", "tracing", @@ -4890,15 +4889,6 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9091b6114800a5f2141aee1d1b9d6ca3592ac062dc5decb3764ec5895a47b4eb" -[[package]] -name = "stream_assert" -version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "58491272dc90918dba713fd9e3556a67a09e067621143f3616754788286489f1" -dependencies = [ - "futures-util", -] - [[package]] name = "string_cache" version = "0.8.7" diff --git a/Cargo.toml b/Cargo.toml index 416ad2f67..68365090a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,7 +41,6 @@ once_cell = "1.16.0" serde = "1.0.151" serde_html_form = "0.2.0" serde_json = "1.0.91" -stream_assert = "0.1.0" thiserror = "1.0.38" tokio = { version = "1.24", default-features = false, features = ["sync"] } tracing = { version = "0.1.36", default-features = false, features = ["std"] } diff --git a/crates/matrix-sdk-ui/Cargo.toml b/crates/matrix-sdk-ui/Cargo.toml index 785df369f..54f7c58a5 100644 --- a/crates/matrix-sdk-ui/Cargo.toml +++ b/crates/matrix-sdk-ui/Cargo.toml @@ -45,7 +45,6 @@ assert-json-diff = "2.0" assert_matches = { workspace = true } ctor = { workspace = true } matrix-sdk-test = { version = "0.6.0", path = "../../testing/matrix-sdk-test" } -stream_assert = { workspace = true } tracing-subscriber = { version = "0.3.11", features = ["env-filter"] } wiremock = "0.5.13" diff --git a/crates/matrix-sdk-ui/tests/integration/room_list.rs b/crates/matrix-sdk-ui/tests/integration/room_list.rs index 445e26e9f..bbcc9a08b 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list.rs @@ -1,7 +1,7 @@ -use std::ops::DerefMut; - +use assert_matches::assert_matches; use eyeball_im::VectorDiff; -use futures_util::{pin_mut, StreamExt}; +use futures_util::{pin_mut, FutureExt, StreamExt}; +use imbl::vector; use matrix_sdk_test::async_test; use matrix_sdk_ui::{ room_list::{ @@ -12,7 +12,6 @@ use matrix_sdk_ui::{ }; use ruma::room_id; use serde_json::json; -use stream_assert::*; use tokio::{spawn, sync::mpsc::unbounded_channel}; use wiremock::{http::Method, Match, Mock, MockServer, Request, ResponseTemplate}; @@ -38,7 +37,6 @@ impl Match for SlidingSyncMatcher { macro_rules! sync_then_assert_request_and_fake_response { ( [$server:ident, $room_list:ident, $room_list_sync_stream:ident] - sync once, states = $state_0:ident $( -> $state_n:ident )+, assert request = { $( $request_json:tt )* }, respond with = { $( $response_json:tt )* } @@ -97,6 +95,99 @@ macro_rules! sync_then_assert_request_and_fake_response { }; } +macro_rules! entries { + ( @_ [ E $( , $( $rest:tt )* )? ] [ $( $accumulator:tt )* ] ) => { + entries!( @_ [ $( $( $rest )* )? ] [ $( $accumulator )* RoomListEntry::Empty, ] ) + }; + + ( @_ [ F( $room_id:literal ) $( , $( $rest:tt )* )? ] [ $( $accumulator:tt )* ] ) => { + entries!( @_ [ $( $( $rest )* )? ] [ $( $accumulator )* RoomListEntry::Filled(room_id!( $room_id ).to_owned()), ] ) + }; + + ( @_ [ I( $room_id:literal ) $( , $( $rest:tt )* )? ] [ $( $accumulator:tt )* ] ) => { + entries!( @_ [ $( $( $rest )* )? ] [ $( $accumulator )* RoomListEntry::Invalidated(room_id!( $room_id ).to_owned()), ] ) + }; + + ( @_ [] [ $( $accumulator:tt )+ ] ) => { + vector![ $( $accumulator )* ] + }; + + ( $( $all:tt )* ) => { + entries!( @_ [ $( $all )* ] [] ) + }; +} + +macro_rules! assert_entries_stream { + // `append [$entries]` + ( @_ [ $stream:ident ] [ append [ $( $entries:tt )+ ] ; $( $rest:tt )* ] [ $( $accumulator:tt )* ] ) => { + assert_entries_stream!( + @_ + [ $stream ] + [ $( $rest )* ] + [ + $( $accumulator )* + { + assert_matches!( + $stream.next().now_or_never(), + Some(Some(VectorDiff::Append { values })) => { + assert_eq!(values, entries!( $( $entries )+ )); + } + ); + } + ] + ) + }; + + // `set [$nth] [$entry]` + ( @_ [ $stream:ident ] [ set [ $index:literal ] [ $( $entry:tt )+ ] ; $( $rest:tt )* ] [ $( $accumulator:tt )* ] ) => { + assert_entries_stream!( + @_ + [ $stream ] + [ $( $rest )* ] + [ + $( $accumulator )* + { + assert_matches!( + $stream.next().now_or_never(), + Some(Some(VectorDiff::Set { index: $index, value })) => { + assert_eq!( + value, + entries!( $( $entry )+ )[0], + ); + } + ); + } + ] + ) + }; + + // `pending` + ( @_ [ $stream:ident ] [ pending ; $( $rest:tt )* ] [ $( $accumulator:tt )* ] ) => { + assert_entries_stream!( + @_ + [ $stream ] + [ $( $rest )* ] + [ + $( $accumulator )* + { + assert_eq!( + $stream.next().now_or_never(), + None, + ); + } + ] + ) + }; + + ( @_ [ $stream:ident ] [] [ $( $accumulator:tt )* ] ) => { + $( $accumulator )* + }; + + ( [ $stream:ident ] $( $all:tt )* ) => { + assert_entries_stream!( @_ [ $stream ] [ $( $all )* ] [] ) + }; +} + #[async_test] async fn test_init_to_enjoy() -> Result<(), Error> { let (server, room_list) = new_room_list().await?; @@ -106,7 +197,6 @@ async fn test_init_to_enjoy() -> Result<(), Error> { sync_then_assert_request_and_fake_response! { [server, room_list, sync] - sync once, states = Init -> LoadFirstRooms -> LoadAllRooms, assert request = { "lists": { @@ -148,7 +238,6 @@ async fn test_init_to_enjoy() -> Result<(), Error> { sync_then_assert_request_and_fake_response! { [server, room_list, sync] - sync once, states = LoadAllRooms -> Enjoy, assert request = { "lists": { @@ -188,7 +277,6 @@ async fn test_init_to_enjoy() -> Result<(), Error> { sync_then_assert_request_and_fake_response! { [server, room_list, sync] - sync once, states = Enjoy -> Enjoy, assert request = { "lists": { @@ -232,7 +320,6 @@ async fn test_entries_stream() -> Result<(), Error> { sync_then_assert_request_and_fake_response! { [server, room_list, sync] - sync once, states = Init -> LoadFirstRooms -> LoadAllRooms, assert request = { "lists": { @@ -279,30 +366,17 @@ async fn test_entries_stream() -> Result<(), Error> { }, }; - let mut entries = room_list.entries_stream(); - let entries = entries.deref_mut(); + let entries = room_list.entries_stream(); pin_mut!(entries); - assert_next_matches!(entries, VectorDiff::Append { values: _ }); - assert_next_matches!( - entries, - VectorDiff::Set { index: 0, value } => { - assert_eq!(value, RoomListEntry::Filled(room_id!("!r0:bar.org").to_owned())); - } - ); - assert_next_matches!( - entries, - VectorDiff::Set { index: 1, value } => { - assert_eq!(value, RoomListEntry::Filled(room_id!("!r1:bar.org").to_owned())); - } - ); - assert_next_matches!( - entries, - VectorDiff::Set { index: 2, value } => { - assert_eq!(value, RoomListEntry::Filled(room_id!("!r2:bar.org").to_owned())); - } - ); - assert_pending!(entries); + assert_entries_stream! { + [entries] + append [ E, E, E, E, E, E, E, E, E, E ]; + set[0] [ F("!r0:bar.org") ]; + set[1] [ F("!r1:bar.org") ]; + set[2] [ F("!r2:bar.org") ]; + pending; + } Ok(()) } diff --git a/crates/matrix-sdk/src/sliding_sync/list/mod.rs b/crates/matrix-sdk/src/sliding_sync/list/mod.rs index c37037037..48399429d 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/mod.rs @@ -1615,7 +1615,6 @@ mod tests { ( $( $all:tt )* ) => { entries!( @_ [ $( $all )* ] [] ) }; - } macro_rules! assert_sync_operations { From 697c92beb1a1174471a5f2c4af0efe9dbae66712 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 1 Jun 2023 09:18:03 +0200 Subject: [PATCH 22/48] test(ui): Test `RoomList::entries_stream` with more cases. --- .../tests/integration/room_list.rs | 147 ++++++++++++++---- 1 file changed, 115 insertions(+), 32 deletions(-) diff --git a/crates/matrix-sdk-ui/tests/integration/room_list.rs b/crates/matrix-sdk-ui/tests/integration/room_list.rs index bbcc9a08b..dbeb9a133 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list.rs @@ -126,14 +126,12 @@ macro_rules! assert_entries_stream { [ $( $rest )* ] [ $( $accumulator )* - { - assert_matches!( - $stream.next().now_or_never(), - Some(Some(VectorDiff::Append { values })) => { - assert_eq!(values, entries!( $( $entries )+ )); - } - ); - } + assert_matches!( + $stream.next().now_or_never(), + Some(Some(VectorDiff::Append { values })) => { + assert_eq!(values, entries!( $( $entries )+ )); + } + ); ] ) }; @@ -146,17 +144,46 @@ macro_rules! assert_entries_stream { [ $( $rest )* ] [ $( $accumulator )* - { - assert_matches!( - $stream.next().now_or_never(), - Some(Some(VectorDiff::Set { index: $index, value })) => { - assert_eq!( - value, - entries!( $( $entry )+ )[0], - ); - } - ); - } + assert_matches!( + $stream.next().now_or_never(), + Some(Some(VectorDiff::Set { index: $index, value })) => { + assert_eq!(value, entries!( $( $entry )+ )[0]); + } + ); + ] + ) + }; + + // `remove [$nth]` + ( @_ [ $stream:ident ] [ remove [ $index:literal ] ; $( $rest:tt )* ] [ $( $accumulator:tt )* ] ) => { + assert_entries_stream!( + @_ + [ $stream ] + [ $( $rest )* ] + [ + $( $accumulator )* + assert_eq!( + $stream.next().now_or_never(), + Some(Some(VectorDiff::Remove { index: $index })), + ); + ] + ) + }; + + // `insert [$nth] [ $entry ]` + ( @_ [ $stream:ident ] [ insert [ $index:literal ] [ $( $entry:tt )+ ] ; $( $rest:tt )* ] [ $( $accumulator:tt )* ] ) => { + assert_entries_stream!( + @_ + [ $stream ] + [ $( $rest )* ] + [ + $( $accumulator )* + assert_matches!( + $stream.next().now_or_never(), + Some(Some(VectorDiff::Insert { index: $index, value })) => { + assert_eq!(value, entries!( $( $entry )+ )[0]); + } + ); ] ) }; @@ -169,12 +196,7 @@ macro_rules! assert_entries_stream { [ $( $rest )* ] [ $( $accumulator )* - { - assert_eq!( - $stream.next().now_or_never(), - None, - ); - } + assert_eq!($stream.next().now_or_never(), None); ] ) }; @@ -318,6 +340,9 @@ async fn test_entries_stream() -> Result<(), Error> { let sync = room_list.sync(); pin_mut!(sync); + let entries = room_list.entries_stream(); + pin_mut!(entries); + sync_then_assert_request_and_fake_response! { [server, room_list, sync] states = Init -> LoadFirstRooms -> LoadAllRooms, @@ -341,10 +366,10 @@ async fn test_entries_stream() -> Result<(), Error> { "!r0:bar.org", "!r1:bar.org", "!r2:bar.org", - ] - } - ] - } + ], + }, + ], + }, }, "rooms": { "!r0:bar.org": { @@ -366,9 +391,6 @@ async fn test_entries_stream() -> Result<(), Error> { }, }; - let entries = room_list.entries_stream(); - pin_mut!(entries); - assert_entries_stream! { [entries] append [ E, E, E, E, E, E, E, E, E, E ]; @@ -378,5 +400,66 @@ async fn test_entries_stream() -> Result<(), Error> { pending; } + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + states = LoadAllRooms -> Enjoy, + assert request = { + "lists": { + ALL_ROOMS: { + "ranges": [ + [0, 9], + ], + }, + VISIBLE_ROOMS: { + "ranges": [], + } + } + }, + respond with = { + "pos": "1", + "lists": { + ALL_ROOMS: { + "count": 9, + "ops": [ + { + "op": "DELETE", + "index": 1, + }, + { + "op": "DELETE", + "index": 0, + }, + { + "op": "INSERT", + "index": 0, + "room_id": "!r3:bar.org" + }, + ], + }, + VISIBLE_ROOMS: { + "count": 0, + "ops": [ + // let's ignore them for now + ], + }, + }, + "rooms": { + "!r3:bar.org": { + "name": "Room #3", + "initial": true, + "timeline": [], + }, + }, + }, + }; + + assert_entries_stream! { + [entries] + remove[1]; + remove[0]; + insert[0] [ F("!r3:bar.org") ]; + pending; + } + Ok(()) } From a090a070eeac96fed493d29d3623cdaaaba2455c Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 1 Jun 2023 10:08:59 +0200 Subject: [PATCH 23/48] chore(cargo): Add missing EOF. --- crates/matrix-sdk-ui/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/matrix-sdk-ui/Cargo.toml b/crates/matrix-sdk-ui/Cargo.toml index 54f7c58a5..fedbb514b 100644 --- a/crates/matrix-sdk-ui/Cargo.toml +++ b/crates/matrix-sdk-ui/Cargo.toml @@ -50,4 +50,4 @@ wiremock = "0.5.13" [[test]] name = "integration" -required-features = ["testing"] \ No newline at end of file +required-features = ["testing"] From fbe1603190ace12549eb17bde2a4c78fcddf5483 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 1 Jun 2023 14:00:09 +0200 Subject: [PATCH 24/48] feat(ui): Rethink the state machine of `RoomList`. --- crates/matrix-sdk-ui/src/room_list/mod.rs | 68 +++++++++++-------- .../tests/integration/room_list.rs | 67 +++++++++++------- crates/matrix-sdk/src/sliding_sync/mod.rs | 2 + 3 files changed, 81 insertions(+), 56 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 1ab60bbc1..b8b9625c1 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -81,35 +81,41 @@ impl RoomList { let sync = self.sliding_sync.sync(); pin_mut!(sync); - // Before doing the first sync, let's transition to the next state. - { - let next_state = self.state.read().next(&self.sliding_sync).await?; + // This is a state machine implementation. + // Things happen in this order: + // + // 1. The next state is calculated, + // 2. The actions associated to the next state are run, + // 3. The next state is stored, + // 4. A sync is done. + // + // So the sync is done after the machine _has entered_ into a new state. + loop { + { + let next_state = self.state.read().next(&self.sliding_sync).await?; - Observable::set(&self.state, next_state); - } - - while let Some(update_summary) = sync.next().await { - match update_summary { - Ok(_update_summary) => { - { - let next_state = self.state.read().next(&self.sliding_sync).await?; - - Observable::set(&self.state, next_state); - } + Observable::set(&self.state, next_state); + } + match sync.next().await { + Some(Ok(_update_summary)) => { yield Ok(()); } - Err(error) => { + Some(Err(error)) => { + // TODO: what to do when an error is raised? yield Err(Error::SlidingSync(error)); } + + None => { + let next_state = State::Terminated; + + Observable::set(&self.state, next_state); + + break; + } } - } - - let next_state = State::Terminated; - - Observable::set(&self.state, next_state); } } @@ -143,21 +149,23 @@ pub enum Error { #[error("Unknown list `{0}`")] UnknownList(String), - #[error("foo")] + #[error("Failed to set up the entries correctly")] Entries, + + #[error("Failed to acquire a lock to update the entries filter")] + CannotUpdateEntriesFilter, } #[derive(Copy, Clone, Debug, PartialEq)] pub enum State { - /// That's the first initial state. The next state is calculated before - /// doing the first sync. + /// That's the first initial state. Init, /// At this state, the first rooms starts to be synced. - LoadFirstRooms, + FirstRooms, /// At this state, all rooms starts to be synced. - LoadAllRooms, + AllRooms, /// This state is the cruising speed, i.e. the “normal” state, where nothing /// fancy happens: all rooms are syncing, and life is great. @@ -173,9 +181,9 @@ impl State { use State::*; let (next_state, actions) = match self { - Init => (LoadFirstRooms, Actions::none()), - LoadFirstRooms => (LoadAllRooms, Actions::first_rooms_are_loaded()), - LoadAllRooms => (Enjoy, Actions::none()), + Init => (FirstRooms, Actions::none()), + FirstRooms => (AllRooms, Actions::first_rooms_are_loaded()), + AllRooms => (Enjoy, Actions::none()), Enjoy => (Enjoy, Actions::none()), Terminated => (Terminated, Actions::none()), }; @@ -330,10 +338,10 @@ mod tests { let state = State::Init; let state = state.next(&sliding_sync).await?; - assert_eq!(state, State::LoadFirstRooms); + assert_eq!(state, State::FirstRooms); let state = state.next(&sliding_sync).await?; - assert_eq!(state, State::LoadAllRooms); + assert_eq!(state, State::AllRooms); let state = state.next(&sliding_sync).await?; assert_eq!(state, State::Enjoy); diff --git a/crates/matrix-sdk-ui/tests/integration/room_list.rs b/crates/matrix-sdk-ui/tests/integration/room_list.rs index dbeb9a133..7de234254 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list.rs @@ -12,7 +12,6 @@ use matrix_sdk_ui::{ }; use ruma::room_id; use serde_json::json; -use tokio::{spawn, sync::mpsc::unbounded_channel}; use wiremock::{http::Method, Match, Mock, MockServer, Request, ResponseTemplate}; use crate::logged_in_client; @@ -37,7 +36,7 @@ impl Match for SlidingSyncMatcher { macro_rules! sync_then_assert_request_and_fake_response { ( [$server:ident, $room_list:ident, $room_list_sync_stream:ident] - states = $state_0:ident $( -> $state_n:ident )+, + states = $pre_state:ident -> $post_state:ident, assert request = { $( $request_json:tt )* }, respond with = { $( $response_json:tt )* } $(,)? @@ -50,16 +49,7 @@ macro_rules! sync_then_assert_request_and_fake_response { .mount_as_scoped(&$server) .await; - assert_eq!(State:: $state_0, $room_list.state(), "pre state"); - - let mut states = $room_list.state_stream(); - let (state_sender, mut state_receiver) = unbounded_channel(); - - let state_listener = spawn(async move { - while let Some(state) = states.next().await { - state_sender.send(state).expect("sending state failed"); - } - }); + assert_eq!(State:: $pre_state, $room_list.state(), "pre state"); let next = $room_list_sync_stream.next().await.unwrap()?; @@ -80,15 +70,7 @@ macro_rules! sync_then_assert_request_and_fake_response { } } - $( - assert_eq!( - State:: $state_n , - state_receiver.recv().await.expect("receiving state failed"), - "next state", - ); - )+ - - state_listener.abort(); + assert_eq!(State:: $post_state, $room_list.state(), "post state"); next } @@ -219,7 +201,7 @@ async fn test_init_to_enjoy() -> Result<(), Error> { sync_then_assert_request_and_fake_response! { [server, room_list, sync] - states = Init -> LoadFirstRooms -> LoadAllRooms, + states = Init -> FirstRooms, assert request = { "lists": { ALL_ROOMS: { @@ -260,7 +242,7 @@ async fn test_init_to_enjoy() -> Result<(), Error> { sync_then_assert_request_and_fake_response! { [server, room_list, sync] - states = LoadAllRooms -> Enjoy, + states = FirstRooms -> AllRooms, assert request = { "lists": { ALL_ROOMS: { @@ -299,7 +281,7 @@ async fn test_init_to_enjoy() -> Result<(), Error> { sync_then_assert_request_and_fake_response! { [server, room_list, sync] - states = Enjoy -> Enjoy, + states = AllRooms -> Enjoy, assert request = { "lists": { ALL_ROOMS: { @@ -330,6 +312,39 @@ async fn test_init_to_enjoy() -> Result<(), Error> { }, }; + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + states = Enjoy -> Enjoy, + assert request = { + "lists": { + ALL_ROOMS: { + "ranges": [[0, 149]], + }, + VISIBLE_ROOMS: { + "ranges": [], + }, + }, + }, + respond with = { + "pos": "2", + "lists": { + ALL_ROOMS: { + "count": 200, + "ops": [ + // let's ignore them for now + ], + }, + VISIBLE_ROOMS: { + "count": 0, + "ops": [], + }, + }, + "rooms": { + // let's ignore them for now + }, + }, + }; + Ok(()) } @@ -345,7 +360,7 @@ async fn test_entries_stream() -> Result<(), Error> { sync_then_assert_request_and_fake_response! { [server, room_list, sync] - states = Init -> LoadFirstRooms -> LoadAllRooms, + states = Init -> FirstRooms, assert request = { "lists": { ALL_ROOMS: { @@ -402,7 +417,7 @@ async fn test_entries_stream() -> Result<(), Error> { sync_then_assert_request_and_fake_response! { [server, room_list, sync] - states = LoadAllRooms -> Enjoy, + states = FirstRooms -> AllRooms, assert request = { "lists": { ALL_ROOMS: { diff --git a/crates/matrix-sdk/src/sliding_sync/mod.rs b/crates/matrix-sdk/src/sliding_sync/mod.rs index 7e7c55301..b2b66cdf1 100644 --- a/crates/matrix-sdk/src/sliding_sync/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/mod.rs @@ -574,6 +574,7 @@ impl SlidingSync { } Ok(None) => { + // Terminates the loop, and terminates the stream. break; } @@ -592,6 +593,7 @@ impl SlidingSync { // The session has expired too many times, let's raise an error! yield Err(error); + // Terminates the loop, and terminates the stream. break; } From 0a1c0547b4f8e5b963bf9c238d15d426ba4a113f Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 1 Jun 2023 14:18:47 +0200 Subject: [PATCH 25/48] test(ui): Add test to ensure `RoomList::sync` resumes from current state. --- .../tests/integration/room_list.rs | 107 +++++++++++++++++- 1 file changed, 106 insertions(+), 1 deletion(-) diff --git a/crates/matrix-sdk-ui/tests/integration/room_list.rs b/crates/matrix-sdk-ui/tests/integration/room_list.rs index 7de234254..c4f94fcb2 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list.rs @@ -193,7 +193,7 @@ macro_rules! assert_entries_stream { } #[async_test] -async fn test_init_to_enjoy() -> Result<(), Error> { +async fn test_sync_from_init_to_enjoy() -> Result<(), Error> { let (server, room_list) = new_room_list().await?; let sync = room_list.sync(); @@ -347,6 +347,111 @@ async fn test_init_to_enjoy() -> Result<(), Error> { Ok(()) } +#[async_test] + +async fn test_sync_resumes_from_previous_state() -> Result<(), Error> { + let (server, room_list) = new_room_list().await?; + + // Start a sync, and drop it at the end of the block. + { + let sync = room_list.sync(); + pin_mut!(sync); + + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + states = Init -> FirstRooms, + assert request = { + "lists": { + ALL_ROOMS: { + "ranges": [[0, 19]], + }, + }, + }, + respond with = { + "pos": "0", + "lists": { + ALL_ROOMS: { + "count": 10, + "ops": [] + }, + }, + "rooms": {}, + }, + }; + } + + // Start a sync, and drop it at the end of the block. + { + let sync = room_list.sync(); + pin_mut!(sync); + + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + states = FirstRooms -> AllRooms, + assert request = { + "lists": { + ALL_ROOMS: { + "ranges": [[0, 9]], + }, + VISIBLE_ROOMS: { + "ranges": [], + } + } + }, + respond with = { + "pos": "1", + "lists": { + ALL_ROOMS: { + "count": 10, + "ops": [], + }, + VISIBLE_ROOMS: { + "count": 0, + "ops": [], + }, + }, + "rooms": {}, + }, + }; + } + + // Start a sync, and drop it at the end of the block. + { + let sync = room_list.sync(); + pin_mut!(sync); + + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + states = AllRooms -> Enjoy, + assert request = { + "lists": { + ALL_ROOMS: { + "ranges": [[0, 9]], + }, + VISIBLE_ROOMS: { + "ranges": [], + }, + }, + }, + respond with = { + "pos": "2", + "lists": { + ALL_ROOMS: { + "count": 10, + "ops": [], + }, + VISIBLE_ROOMS: { + "count": 0, + "ops": [], + }, + }, + "rooms": {}, + }, + }; + } + + Ok(()) +} #[async_test] async fn test_entries_stream() -> Result<(), Error> { From 048b054a65b5027675ad421eacf6f658c96b2b88 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 1 Jun 2023 14:55:53 +0200 Subject: [PATCH 26/48] feat(ui): Implement `RoomList::update_entries_stream_filter`. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch implements `RoomList::update_entries_stream_filter` which allows to… change the… entries stream filter. This patch also provides a test for that. How nice it is. --- crates/matrix-sdk-ui/src/room_list/mod.rs | 16 ++ .../tests/integration/room_list.rs | 151 ++++++++++++++++++ 2 files changed, 167 insertions(+) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index b8b9625c1..2843b0b5a 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -133,6 +133,22 @@ impl RoomList { self.entries_stream.write().unwrap() } + pub async fn update_entries_stream_filter( + &self, + filter: Box bool + Sync + Send>, + ) -> Result<(), Error> { + let mut entries_stream = + self.entries_stream.try_write().map_err(|_| Error::CannotUpdateEntriesFilter)?; + + *entries_stream = self + .sliding_sync + .on_list(ALL_ROOMS_LIST_NAME, |list| ready(list.room_list_filtered_stream(filter))) + .await + .ok_or_else(|| Error::UnknownList(ALL_ROOMS_LIST_NAME.to_string()))?; + + Ok(()) + } + #[cfg(any(test, feature = "testing"))] pub fn sliding_sync(&self) -> &SlidingSync { &self.sliding_sync diff --git a/crates/matrix-sdk-ui/tests/integration/room_list.rs b/crates/matrix-sdk-ui/tests/integration/room_list.rs index c4f94fcb2..88b2dd294 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list.rs @@ -583,3 +583,154 @@ async fn test_entries_stream() -> Result<(), Error> { Ok(()) } + +#[async_test] +async fn test_entries_stream_with_updated_filter() -> Result<(), Error> { + let (server, room_list) = new_room_list().await?; + + let sync = room_list.sync(); + pin_mut!(sync); + + // Drop the entries stream at the end of the block. + { + let entries = room_list.entries_stream(); + pin_mut!(entries); + + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + states = Init -> FirstRooms, + assert request = { + "lists": { + ALL_ROOMS: { + "ranges": [[0, 19]], + }, + }, + }, + respond with = { + "pos": "0", + "lists": { + ALL_ROOMS: { + "count": 10, + "ops": [ + { + "op": "SYNC", + "range": [0, 0], + "room_ids": [ + "!r0:bar.org", + ], + }, + ], + }, + }, + "rooms": { + "!r0:bar.org": { + "name": "Room #0", + "initial": true, + "timeline": [], + }, + }, + }, + }; + + assert_entries_stream! { + [entries] + append [ E, E, E, E, E, E, E, E, E, E ]; + set[0] [ F("!r0:bar.org") ]; + pending; + }; + } + + // Update the filter. + // First, we need to drop the entries stream. + // (already done) + + // Second, update the filter. + room_list + .update_entries_stream_filter(Box::new(|room_list_entry| { + matches!( + room_list_entry.as_room_id(), + Some(room_id) if room_id.server_name() == "bar.org" + ) + })) + .await?; + + // Third, let's get a new entries stream. + { + let entries = room_list.entries_stream(); + pin_mut!(entries); + + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + states = FirstRooms -> AllRooms, + assert request = { + "lists": { + ALL_ROOMS: { + "ranges": [ + [0, 9], + ], + }, + VISIBLE_ROOMS: { + "ranges": [], + } + } + }, + respond with = { + "pos": "1", + "lists": { + ALL_ROOMS: { + "count": 10, + "ops": [ + { + "op": "SYNC", + "range": [1, 4], + "room_ids": [ + "!r1:bar.org", + "!r2:qux.org", + "!r3:qux.org", + "!r4:bar.org", + ], + }, + ], + }, + VISIBLE_ROOMS: { + "count": 0, + "ops": [ + // let's ignore them for now + ], + }, + }, + "rooms": { + "!r1:bar.org": { + "name": "Room #1", + "initial": true, + "timeline": [], + }, + "!r2:qux.org": { + "name": "Room #2", + "initial": true, + "timeline": [], + }, + "!r3:qux.org": { + "name": "Room #3", + "initial": true, + "timeline": [], + }, + "!r4:bar.org": { + "name": "Room #4", + "initial": true, + "timeline": [], + }, + }, + }, + }; + + assert_entries_stream! { + [entries] + insert[1] [ F("!r1:bar.org") ]; + insert[2] [ F("!r4:bar.org") ]; + pending; + }; + } + + Ok(()) +} From db0217d093ad4ce5afcfebb5c5589dd5fdd4b3b5 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Thu, 1 Jun 2023 15:33:58 +0200 Subject: [PATCH 27/48] Box internally --- crates/matrix-sdk-ui/src/room_list/mod.rs | 4 ++-- crates/matrix-sdk-ui/tests/integration/room_list.rs | 4 ++-- crates/matrix-sdk/src/sliding_sync/list/mod.rs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 2843b0b5a..8e1a5e6f9 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -54,7 +54,7 @@ impl RoomList { move |list| { *entries.lock().unwrap() = - Some(list.room_list_filtered_stream(Box::new(|_| true))); + Some(list.room_list_filtered_stream(|_| true)); list } @@ -135,7 +135,7 @@ impl RoomList { pub async fn update_entries_stream_filter( &self, - filter: Box bool + Sync + Send>, + filter: impl Fn(&RoomListEntry) -> bool + Sync + Send + 'static, ) -> Result<(), Error> { let mut entries_stream = self.entries_stream.try_write().map_err(|_| Error::CannotUpdateEntriesFilter)?; diff --git a/crates/matrix-sdk-ui/tests/integration/room_list.rs b/crates/matrix-sdk-ui/tests/integration/room_list.rs index 88b2dd294..5347f4392 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list.rs @@ -646,12 +646,12 @@ async fn test_entries_stream_with_updated_filter() -> Result<(), Error> { // Second, update the filter. room_list - .update_entries_stream_filter(Box::new(|room_list_entry| { + .update_entries_stream_filter(|room_list_entry| { matches!( room_list_entry.as_room_id(), Some(room_id) if room_id.server_name() == "bar.org" ) - })) + }) .await?; // Third, let's get a new entries stream. diff --git a/crates/matrix-sdk/src/sliding_sync/list/mod.rs b/crates/matrix-sdk/src/sliding_sync/list/mod.rs index 48399429d..9a8efa764 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/mod.rs @@ -135,7 +135,7 @@ impl SlidingSyncList { /// by `filter`. pub fn room_list_filtered_stream( &self, - filter: Box bool + Sync + Send>, + filter: impl Fn(&RoomListEntry) -> bool + Sync + Send + 'static, ) -> FilteredVectorSubscriber bool + Sync + Send>> { let (_, stream) = From 58e8c0a7ace017374acb78d0601b405daced3e70 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Thu, 1 Jun 2023 15:34:13 +0200 Subject: [PATCH 28/48] Fix warning --- crates/matrix-sdk-ui/src/room_list/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 8e1a5e6f9..70941a3df 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -129,7 +129,7 @@ impl RoomList { pub fn entries_stream( &self, - ) -> RwLockWriteGuard>> { + ) -> RwLockWriteGuard<'_, impl Stream>> { self.entries_stream.write().unwrap() } From 51062559113b1ffa56731be249cdd22771c38ede Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Thu, 1 Jun 2023 15:40:39 +0200 Subject: [PATCH 29/48] On-demand stream creation --- crates/matrix-sdk-ui/src/room_list/mod.rs | 69 +++++-------------- .../matrix-sdk/src/sliding_sync/list/mod.rs | 22 ++++-- 2 files changed, 31 insertions(+), 60 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 70941a3df..40a94e003 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -1,17 +1,13 @@ //! `RoomList` API. -use std::{ - fmt, - future::ready, - sync::{Arc, Mutex, RwLock, RwLockWriteGuard}, -}; +use std::future::ready; use async_stream::stream; use async_trait::async_trait; use eyeball::shared::Observable; use eyeball_im::VectorDiff; -use eyeball_im_util::FilteredVectorSubscriber; use futures_util::{pin_mut, Stream, StreamExt}; +use imbl::Vector; pub use matrix_sdk::RoomListEntry; use matrix_sdk::{ Client, Error as SlidingSyncError, SlidingSync, SlidingSyncList, SlidingSyncMode, @@ -22,43 +18,20 @@ use thiserror::Error; pub const ALL_ROOMS_LIST_NAME: &str = "all_rooms"; pub const VISIBLE_ROOMS_LIST_NAME: &str = "visible_rooms"; +#[derive(Debug)] pub struct RoomList { sliding_sync: SlidingSync, - entries_stream: RwLock< - FilteredVectorSubscriber bool + Sync + Send>>, - >, state: Observable, } -impl fmt::Debug for RoomList { - fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - formatter - .debug_struct("RoomList") - .field("sliding_sync", &self.sliding_sync) - .field("state", &self.state) - .finish_non_exhaustive() - } -} - impl RoomList { pub async fn new(client: Client) -> Result { - let entries = Arc::new(Mutex::new(None)); let sliding_sync = client .sliding_sync() .storage_key(Some("matrix-sdk-ui-roomlist".to_string())) .add_cached_list( SlidingSyncList::builder(ALL_ROOMS_LIST_NAME) - .sync_mode(SlidingSyncMode::new_selective().add_range(0..=19)) - .once_built({ - let entries = entries.clone(); - - move |list| { - *entries.lock().unwrap() = - Some(list.room_list_filtered_stream(|_| true)); - - list - } - }), + .sync_mode(SlidingSyncMode::new_selective().add_range(0..=19)), ) .await .map_err(Error::SlidingSync)? @@ -66,14 +39,7 @@ impl RoomList { .await .map_err(Error::SlidingSync)?; - let entries = - Arc::try_unwrap(entries).map_err(|_| Error::Entries)?.into_inner().unwrap().unwrap(); - - Ok(Self { - sliding_sync, - entries_stream: RwLock::new(entries), - state: Observable::new(State::Init), - }) + Ok(Self { sliding_sync, state: Observable::new(State::Init) }) } pub fn sync(&self) -> impl Stream> + '_ { @@ -127,26 +93,23 @@ impl RoomList { Observable::subscribe(&self.state) } - pub fn entries_stream( + pub async fn entries( &self, - ) -> RwLockWriteGuard<'_, impl Stream>> { - self.entries_stream.write().unwrap() + ) -> Result<(Vector, impl Stream>), Error> { + self.sliding_sync + .on_list(ALL_ROOMS_LIST_NAME, |list| ready(list.room_list_stream())) + .await + .ok_or_else(|| Error::UnknownList(ALL_ROOMS_LIST_NAME.to_string())) } - pub async fn update_entries_stream_filter( + pub async fn entries_filtered( &self, - filter: impl Fn(&RoomListEntry) -> bool + Sync + Send + 'static, - ) -> Result<(), Error> { - let mut entries_stream = - self.entries_stream.try_write().map_err(|_| Error::CannotUpdateEntriesFilter)?; - - *entries_stream = self - .sliding_sync + filter: impl Fn(&RoomListEntry) -> bool + Send + Sync + 'static, + ) -> Result<(Vector, impl Stream>), Error> { + self.sliding_sync .on_list(ALL_ROOMS_LIST_NAME, |list| ready(list.room_list_filtered_stream(filter))) .await - .ok_or_else(|| Error::UnknownList(ALL_ROOMS_LIST_NAME.to_string()))?; - - Ok(()) + .ok_or_else(|| Error::UnknownList(ALL_ROOMS_LIST_NAME.to_string())) } #[cfg(any(test, feature = "testing"))] diff --git a/crates/matrix-sdk/src/sliding_sync/list/mod.rs b/crates/matrix-sdk/src/sliding_sync/list/mod.rs index 9a8efa764..4d99277f7 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/mod.rs @@ -17,6 +17,7 @@ use eyeball_im::{ObservableVector, VectorDiff}; use eyeball_im_util::{FilteredVectorSubscriber, VectorExt}; pub(super) use frozen::FrozenSlidingSyncList; use futures_core::Stream; +use imbl::Vector; pub(super) use request_generator::*; pub use room_list_entry::RoomListEntry; use ruma::{api::client::sync::sync_events::v4, assign, events::StateEventType, OwnedRoomId}; @@ -49,6 +50,8 @@ pub struct SlidingSyncList { inner: Arc, } +type BoxedRoomListEntryFilter = Box bool + Sync + Send>; + impl SlidingSyncList { /// Create a new [`SlidingSyncListBuilder`] with the given name. pub fn builder(name: impl Into) -> SlidingSyncListBuilder { @@ -125,8 +128,13 @@ impl SlidingSyncList { /// /// There's no guarantee of ordering between items emitted by this stream /// and those emitted by other streams exposed on this structure. - pub fn room_list_stream(&self) -> impl Stream> { - ObservableVector::subscribe(&self.inner.room_list.read().unwrap()) + pub fn room_list_stream( + &self, + ) -> (Vector, impl Stream>) { + let read_lock = self.inner.room_list.read().unwrap(); + let values = (*read_lock).clone(); + let subscriber = ObservableVector::subscribe(&read_lock); + (values, subscriber) } /// Get a stream of room list, but filtered. @@ -136,12 +144,12 @@ impl SlidingSyncList { pub fn room_list_filtered_stream( &self, filter: impl Fn(&RoomListEntry) -> bool + Sync + Send + 'static, - ) -> FilteredVectorSubscriber bool + Sync + Send>> + ) -> (Vector, FilteredVectorSubscriber) { - let (_, stream) = - ObservableVector::subscribe_filtered(&self.inner.room_list.read().unwrap(), filter); - - stream + ObservableVector::subscribe_filtered( + &self.inner.room_list.read().unwrap(), + Box::new(filter), + ) } /// Get the maximum number of rooms. See [`Self::maximum_number_of_rooms`] From c2d082ca8d6c59897b3f7c330b1ee038ea3d1518 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 1 Jun 2023 15:58:12 +0200 Subject: [PATCH 30/48] test(ui): Update the tests according to previous commits. --- crates/matrix-sdk-ui/src/room_list/mod.rs | 9 +- .../tests/integration/room_list.rs | 242 +++++++++--------- .../matrix-sdk/src/sliding_sync/list/mod.rs | 13 +- 3 files changed, 130 insertions(+), 134 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 40a94e003..48fd38b6a 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -102,10 +102,13 @@ impl RoomList { .ok_or_else(|| Error::UnknownList(ALL_ROOMS_LIST_NAME.to_string())) } - pub async fn entries_filtered( + pub async fn entries_filtered( &self, - filter: impl Fn(&RoomListEntry) -> bool + Send + Sync + 'static, - ) -> Result<(Vector, impl Stream>), Error> { + filter: F, + ) -> Result<(Vector, impl Stream>), Error> + where + F: Fn(&RoomListEntry) -> bool + Send + Sync + 'static, + { self.sliding_sync .on_list(ALL_ROOMS_LIST_NAME, |list| ready(list.room_list_filtered_stream(filter))) .await diff --git a/crates/matrix-sdk-ui/tests/integration/room_list.rs b/crates/matrix-sdk-ui/tests/integration/room_list.rs index 5347f4392..4e0e6753a 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list.rs @@ -90,7 +90,7 @@ macro_rules! entries { entries!( @_ [ $( $( $rest )* )? ] [ $( $accumulator )* RoomListEntry::Invalidated(room_id!( $room_id ).to_owned()), ] ) }; - ( @_ [] [ $( $accumulator:tt )+ ] ) => { + ( @_ [] [ $( $accumulator:tt )* ] ) => { vector![ $( $accumulator )* ] }; @@ -460,8 +460,8 @@ async fn test_entries_stream() -> Result<(), Error> { let sync = room_list.sync(); pin_mut!(sync); - let entries = room_list.entries_stream(); - pin_mut!(entries); + let (previous_entries, entries_stream) = room_list.entries().await?; + pin_mut!(entries_stream); sync_then_assert_request_and_fake_response! { [server, room_list, sync] @@ -511,8 +511,9 @@ async fn test_entries_stream() -> Result<(), Error> { }, }; + assert!(previous_entries.is_empty()); assert_entries_stream! { - [entries] + [entries_stream] append [ E, E, E, E, E, E, E, E, E, E ]; set[0] [ F("!r0:bar.org") ]; set[1] [ F("!r1:bar.org") ]; @@ -574,7 +575,7 @@ async fn test_entries_stream() -> Result<(), Error> { }; assert_entries_stream! { - [entries] + [entries_stream] remove[1]; remove[0]; insert[0] [ F("!r3:bar.org") ]; @@ -591,146 +592,135 @@ async fn test_entries_stream_with_updated_filter() -> Result<(), Error> { let sync = room_list.sync(); pin_mut!(sync); - // Drop the entries stream at the end of the block. - { - let entries = room_list.entries_stream(); - pin_mut!(entries); + let (previous_entries, entries_stream) = room_list.entries().await?; + pin_mut!(entries_stream); - sync_then_assert_request_and_fake_response! { - [server, room_list, sync] - states = Init -> FirstRooms, - assert request = { - "lists": { - ALL_ROOMS: { - "ranges": [[0, 19]], - }, + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + states = Init -> FirstRooms, + assert request = { + "lists": { + ALL_ROOMS: { + "ranges": [[0, 19]], }, }, - respond with = { - "pos": "0", - "lists": { - ALL_ROOMS: { - "count": 10, - "ops": [ - { - "op": "SYNC", - "range": [0, 0], - "room_ids": [ - "!r0:bar.org", - ], - }, - ], - }, - }, - "rooms": { - "!r0:bar.org": { - "name": "Room #0", - "initial": true, - "timeline": [], - }, + }, + respond with = { + "pos": "0", + "lists": { + ALL_ROOMS: { + "count": 10, + "ops": [ + { + "op": "SYNC", + "range": [0, 0], + "room_ids": [ + "!r0:bar.org", + ], + }, + ], }, }, - }; + "rooms": { + "!r0:bar.org": { + "name": "Room #0", + "initial": true, + "timeline": [], + }, + }, + }, + }; - assert_entries_stream! { - [entries] - append [ E, E, E, E, E, E, E, E, E, E ]; - set[0] [ F("!r0:bar.org") ]; - pending; - }; - } + assert!(previous_entries.is_empty()); + assert_entries_stream! { + [entries_stream] + append [ E, E, E, E, E, E, E, E, E, E ]; + set[0] [ F("!r0:bar.org") ]; + pending; + }; - // Update the filter. - // First, we need to drop the entries stream. - // (already done) - - // Second, update the filter. - room_list - .update_entries_stream_filter(|room_list_entry| { + let (previous_entries, entries_stream) = room_list + .entries_filtered(|room_list_entry| { matches!( room_list_entry.as_room_id(), Some(room_id) if room_id.server_name() == "bar.org" ) }) .await?; + pin_mut!(entries_stream); - // Third, let's get a new entries stream. - { - let entries = room_list.entries_stream(); - pin_mut!(entries); - - sync_then_assert_request_and_fake_response! { - [server, room_list, sync] - states = FirstRooms -> AllRooms, - assert request = { - "lists": { - ALL_ROOMS: { - "ranges": [ - [0, 9], - ], - }, - VISIBLE_ROOMS: { - "ranges": [], - } + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + states = FirstRooms -> AllRooms, + assert request = { + "lists": { + ALL_ROOMS: { + "ranges": [ + [0, 9], + ], + }, + VISIBLE_ROOMS: { + "ranges": [], } - }, - respond with = { - "pos": "1", - "lists": { - ALL_ROOMS: { - "count": 10, - "ops": [ - { - "op": "SYNC", - "range": [1, 4], - "room_ids": [ - "!r1:bar.org", - "!r2:qux.org", - "!r3:qux.org", - "!r4:bar.org", - ], - }, - ], - }, - VISIBLE_ROOMS: { - "count": 0, - "ops": [ - // let's ignore them for now - ], - }, + } + }, + respond with = { + "pos": "1", + "lists": { + ALL_ROOMS: { + "count": 10, + "ops": [ + { + "op": "SYNC", + "range": [1, 4], + "room_ids": [ + "!r1:bar.org", + "!r2:qux.org", + "!r3:qux.org", + "!r4:bar.org", + ], + }, + ], }, - "rooms": { - "!r1:bar.org": { - "name": "Room #1", - "initial": true, - "timeline": [], - }, - "!r2:qux.org": { - "name": "Room #2", - "initial": true, - "timeline": [], - }, - "!r3:qux.org": { - "name": "Room #3", - "initial": true, - "timeline": [], - }, - "!r4:bar.org": { - "name": "Room #4", - "initial": true, - "timeline": [], - }, + VISIBLE_ROOMS: { + "count": 0, + "ops": [ + // let's ignore them for now + ], }, }, - }; + "rooms": { + "!r1:bar.org": { + "name": "Room #1", + "initial": true, + "timeline": [], + }, + "!r2:qux.org": { + "name": "Room #2", + "initial": true, + "timeline": [], + }, + "!r3:qux.org": { + "name": "Room #3", + "initial": true, + "timeline": [], + }, + "!r4:bar.org": { + "name": "Room #4", + "initial": true, + "timeline": [], + }, + }, + }, + }; - assert_entries_stream! { - [entries] - insert[1] [ F("!r1:bar.org") ]; - insert[2] [ F("!r4:bar.org") ]; - pending; - }; - } + assert_eq!(previous_entries, entries![F("!r0:bar.org")]); + assert_entries_stream! { + [entries_stream] + insert[1] [ F("!r1:bar.org") ]; + insert[2] [ F("!r4:bar.org") ]; + pending; + }; Ok(()) } diff --git a/crates/matrix-sdk/src/sliding_sync/list/mod.rs b/crates/matrix-sdk/src/sliding_sync/list/mod.rs index 4d99277f7..1f043cd40 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/mod.rs @@ -132,19 +132,22 @@ impl SlidingSyncList { &self, ) -> (Vector, impl Stream>) { let read_lock = self.inner.room_list.read().unwrap(); - let values = (*read_lock).clone(); + let previous_values = (*read_lock).clone(); let subscriber = ObservableVector::subscribe(&read_lock); - (values, subscriber) + + (previous_values, subscriber) } /// Get a stream of room list, but filtered. /// /// It's similar to [`Self::room_list_stream`] but the room list is filtered /// by `filter`. - pub fn room_list_filtered_stream( + pub fn room_list_filtered_stream( &self, - filter: impl Fn(&RoomListEntry) -> bool + Sync + Send + 'static, + filter: F, ) -> (Vector, FilteredVectorSubscriber) + where + F: Fn(&RoomListEntry) -> bool + Sync + Send + 'static, { ObservableVector::subscribe_filtered( &self.inner.room_list.read().unwrap(), @@ -1616,7 +1619,7 @@ mod tests { entries!( @_ [ $( $( $rest )* )? ] [ $( $accumulator )* RoomListEntry::Invalidated(room_id!( $room_id ).to_owned()), ] ) }; - ( @_ [] [ $( $accumulator:tt )+ ] ) => { + ( @_ [] [ $( $accumulator:tt )* ] ) => { vector![ $( $accumulator )* ] }; From f5c950c54aa74ad25cf992af5941c538162f9752 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 1 Jun 2023 15:58:58 +0200 Subject: [PATCH 31/48] fix(ffi): Update according to last commits. --- bindings/matrix-sdk-ffi/src/sliding_sync.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bindings/matrix-sdk-ffi/src/sliding_sync.rs b/bindings/matrix-sdk-ffi/src/sliding_sync.rs index ebefb2f85..a8967b8f3 100644 --- a/bindings/matrix-sdk-ffi/src/sliding_sync.rs +++ b/bindings/matrix-sdk-ffi/src/sliding_sync.rs @@ -598,7 +598,7 @@ impl SlidingSyncList { &self, observer: Box, ) -> Arc { - let mut room_list_stream = self.inner.room_list_stream(); + let (_, mut room_list_stream) = self.inner.room_list_stream(); Arc::new(TaskHandle::new(RUNTIME.spawn(async move { loop { From e2d2cf787dac531ab97953d8aa05c37783d79b10 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 1 Jun 2023 16:52:09 +0200 Subject: [PATCH 32/48] test(sdk): Fix according to previous commits. --- crates/matrix-sdk/src/sliding_sync/README.md | 2 +- crates/matrix-sdk/src/sliding_sync/list/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/matrix-sdk/src/sliding_sync/README.md b/crates/matrix-sdk/src/sliding_sync/README.md index 89a0678a0..ec450ff03 100644 --- a/crates/matrix-sdk/src/sliding_sync/README.md +++ b/crates/matrix-sdk/src/sliding_sync/README.md @@ -441,7 +441,7 @@ let sliding_sync = sliding_sync_builder // subscribe to the list APIs for updates -let (list_state_stream, list_count_stream, list_stream) = sliding_sync.on_list(&active_list_name, |list| { +let (list_state_stream, list_count_stream, (_, list_stream)) = sliding_sync.on_list(&active_list_name, |list| { ready((list.state_stream(), list.maximum_number_of_rooms_stream(), list.room_list_stream())) }).await.unwrap(); diff --git a/crates/matrix-sdk/src/sliding_sync/list/mod.rs b/crates/matrix-sdk/src/sliding_sync/list/mod.rs index 1f043cd40..cfc172cc9 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/mod.rs @@ -1499,7 +1499,7 @@ mod tests { ); } - let mut room_list_stream = list.room_list_stream(); + let (_, mut room_list_stream) = list.room_list_stream(); let (room_list_stream_sender, mut room_list_stream_receiver) = unbounded_channel(); spawn(async move { From 13d38993f85be6cd39ce6a7f3849a59eb6334905 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 1 Jun 2023 16:52:16 +0200 Subject: [PATCH 33/48] feat(sdk): Expose the `Range` and `Ranges` type in `matrix_sdk::sliding_sync`. --- crates/matrix-sdk/src/sliding_sync/list/mod.rs | 16 ++++++++++++++-- .../src/sliding_sync/list/request_generator.rs | 15 ++++++--------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/crates/matrix-sdk/src/sliding_sync/list/mod.rs b/crates/matrix-sdk/src/sliding_sync/list/mod.rs index cfc172cc9..d5e89048c 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/mod.rs @@ -39,9 +39,15 @@ pub(crate) enum SlidingSyncListCachePolicy { } /// The type used to express natural bounds (including but not limited to: -/// ranges, timeline limit) in the sliding sync SDK. +/// ranges, timeline limit) in the Sliding Sync. pub type Bound = u32; +/// One range of rooms in a response from Sliding Sync. +pub type Range = RangeInclusive; + +/// Many ranges of rooms. +pub type Ranges = Vec; + /// Holding a specific filtered list within the concept of sliding sync. /// /// It is OK to clone this type as much as you need: cloning it is cheap. @@ -764,10 +770,16 @@ impl SlidingSyncSelectiveModeBuilder { } /// Select a range to fetch. - pub fn add_range(mut self, range: RangeInclusive) -> Self { + pub fn add_range(mut self, range: Range) -> Self { self.ranges.push(range); self } + + /// Select many ranges to fetch. + pub fn add_ranges(mut self, ranges: Ranges) -> Self { + self.ranges.extend(ranges); + self + } } impl From for SlidingSyncMode { diff --git a/crates/matrix-sdk/src/sliding_sync/list/request_generator.rs b/crates/matrix-sdk/src/sliding_sync/list/request_generator.rs index 38e4ae89d..234a499a8 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/request_generator.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/request_generator.rs @@ -29,14 +29,11 @@ //! user-specified limit representing the maximum number of rooms the user //! actually wants to load. -use std::{cmp::min, ops::RangeInclusive}; +use std::cmp::min; -use super::{Bound, SlidingSyncMode}; +use super::{Range, Ranges, SlidingSyncMode}; use crate::{sliding_sync::Error, SlidingSyncState}; -/// Range of rooms in a response from sliding sync. -pub type Ranges = Vec>; - /// The kind of request generator. #[derive(Debug, PartialEq)] pub(super) enum SlidingSyncListRequestGeneratorKind { @@ -80,7 +77,7 @@ pub(in super::super) struct SlidingSyncListRequestGenerator { /// The current ranges used by this request generator. /// /// Note there's only one range in the `Growing` and `Paging` mode. - ranges: Vec>, + ranges: Ranges, /// The kind of request generator. pub(super) kind: SlidingSyncListRequestGeneratorKind, } @@ -122,7 +119,7 @@ impl SlidingSyncListRequestGenerator { /// For generators in the selective mode, this is the initial set of ranges. /// For growing and paginated generators, this is the range committed in the /// latest response received from the server. - pub(super) fn requested_ranges(&self) -> &[RangeInclusive] { + pub(super) fn requested_ranges(&self) -> &[Range] { &self.ranges } @@ -291,7 +288,7 @@ fn create_range( desired_size: u32, maximum_number_of_rooms_to_fetch: Option, maximum_number_of_rooms: Option, -) -> Result, Error> { +) -> Result { // Calculate the range. // The `start` bound is given. Let's calculate the `end` bound. @@ -322,7 +319,7 @@ fn create_range( return Err(Error::InvalidRange { start, end }); } - Ok(RangeInclusive::new(start, end)) + Ok(Range::new(start, end)) } #[cfg(test)] From 2f68c86796b9211ad9d950f09a448a8ef6b726ec Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 1 Jun 2023 16:52:43 +0200 Subject: [PATCH 34/48] feat(ui): Add the `RoomList` `Input` API. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch lands the first design of the `Input` API. An `Input` is something external that can be understood by the `RoomList` state machine. The first inpuput is `Viewport` to change the “viewport” of the `RoomList`, which translates to the change of the ranges of one specific Sliding Sync list. --- crates/matrix-sdk-ui/src/room_list/mod.rs | 30 ++++++- .../tests/integration/room_list.rs | 78 ++++++++++++++++++- 2 files changed, 106 insertions(+), 2 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 48fd38b6a..1d03f4285 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -10,7 +10,8 @@ use futures_util::{pin_mut, Stream, StreamExt}; use imbl::Vector; pub use matrix_sdk::RoomListEntry; use matrix_sdk::{ - Client, Error as SlidingSyncError, SlidingSync, SlidingSyncList, SlidingSyncMode, + sliding_sync::Ranges, Client, Error as SlidingSyncError, SlidingSync, SlidingSyncList, + SlidingSyncMode, }; use once_cell::sync::Lazy; use thiserror::Error; @@ -115,6 +116,25 @@ impl RoomList { .ok_or_else(|| Error::UnknownList(ALL_ROOMS_LIST_NAME.to_string())) } + pub async fn apply_input(&self, input: Input) -> Result<(), Error> { + use Input::*; + + match input { + Viewport(ranges) => { + self.sliding_sync + .on_list(VISIBLE_ROOMS_LIST_NAME, |list| { + ready(list.set_sync_mode( + SlidingSyncMode::new_selective().add_ranges(ranges.clone()), + )) + }) + .await + .ok_or_else(|| Error::InputHasNotBeenApplied(Viewport(ranges)))?; + } + } + + Ok(()) + } + #[cfg(any(test, feature = "testing"))] pub fn sliding_sync(&self) -> &SlidingSync { &self.sliding_sync @@ -136,6 +156,9 @@ pub enum Error { #[error("Failed to acquire a lock to update the entries filter")] CannotUpdateEntriesFilter, + + #[error("The input has been not applied")] + InputHasNotBeenApplied(Input), } #[derive(Copy, Clone, Debug, PartialEq)] @@ -257,6 +280,11 @@ impl Actions { } } +#[derive(Debug)] +pub enum Input { + Viewport(Ranges), +} + #[cfg(test)] mod tests { use matrix_sdk::{config::RequestConfig, Session}; diff --git a/crates/matrix-sdk-ui/tests/integration/room_list.rs b/crates/matrix-sdk-ui/tests/integration/room_list.rs index 4e0e6753a..6a4ef442d 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list.rs @@ -5,7 +5,7 @@ use imbl::vector; use matrix_sdk_test::async_test; use matrix_sdk_ui::{ room_list::{ - Error, RoomListEntry, State, ALL_ROOMS_LIST_NAME as ALL_ROOMS, + Error, Input, RoomListEntry, State, ALL_ROOMS_LIST_NAME as ALL_ROOMS, VISIBLE_ROOMS_LIST_NAME as VISIBLE_ROOMS, }, RoomList, @@ -724,3 +724,79 @@ async fn test_entries_stream_with_updated_filter() -> Result<(), Error> { Ok(()) } + +#[async_test] +async fn test_input_viewport() -> Result<(), Error> { + let (server, room_list) = new_room_list().await?; + + let sync = room_list.sync(); + pin_mut!(sync); + + // The input cannot be applied because the `VISIBLE_ROOMS_LIST_NAME` list isn't + // present. + assert_matches!( + room_list.apply_input(Input::Viewport(vec![10..=15])).await, + Err(Error::InputHasNotBeenApplied(_)) + ); + + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + states = Init -> FirstRooms, + assert request = { + "lists": { + ALL_ROOMS: { + "ranges": [[0, 19]], + }, + }, + }, + respond with = { + "pos": "0", + "lists": {}, + "rooms": {}, + }, + }; + + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + states = FirstRooms -> AllRooms, + assert request = { + "lists": { + ALL_ROOMS: { + "ranges": [[0, 49]], + }, + VISIBLE_ROOMS: { + "ranges": [], + } + } + }, + respond with = { + "pos": "1", + "lists": {}, + "rooms": {}, + }, + }; + + assert!(room_list.apply_input(Input::Viewport(vec![10..=15, 20..=25])).await.is_ok()); + + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + states = AllRooms -> Enjoy, + assert request = { + "lists": { + ALL_ROOMS: { + "ranges": [[0, 49]], + }, + VISIBLE_ROOMS: { + "ranges": [[10, 15], [20, 25]], + } + } + }, + respond with = { + "pos": "1", + "lists": {}, + "rooms": {}, + }, + }; + + Ok(()) +} From 8ee84be839de101916f02dc0fbd079d05899195d Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Fri, 2 Jun 2023 20:06:31 +0200 Subject: [PATCH 35/48] doc(ui): Write documentation for `RoomList`. --- crates/matrix-sdk-ui/src/room_list/mod.rs | 112 ++++++++++++++++++++-- 1 file changed, 106 insertions(+), 6 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 1d03f4285..3c01ccf19 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -1,4 +1,64 @@ +// Copyright 2023 The Matrix.org Foundation C.I.C. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for that specific language governing permissions and +// limitations under the License. + //! `RoomList` API. +//! +//! The `RoomList` is a UI API dedicated to present a list of Matrix rooms to +//! the user. The syncing is handled by +//! [`SlidingSync`][matrix_sdk::SlidingSync]. The idea is to expose a simple API +//! to handle most of the client app use cases, like: Showing and updating a +//! list of rooms, filtering a list of rooms, handling particular updates of a +//! range of rooms (the ones the client app is showing to the view, i.e. the +//! rooms present in the viewport) etc. +//! +//! As such, the `RoomList` works as an opinionated state machine. The states +//! are defined by [`State`]. Actions are attached to the each state transition. +//! Apart from that, one can apply [`Input`]s on the state machine, like +//! notifying that the client app viewport of the room list has changed (if the +//! user of the client app has scrolled in the room list for example) etc. +//! +//! The API is purposely small. Sliding Sync is versatile. `RoomList` is _one_ +//! specific usage of Sliding Sync. +//! +//! # Basic principle +//! +//! `RoomList` works with 2 Sliding Sync List: +//! +//! * `all_rooms` (refered by the constant [`ALL_ROOMS_LIST_NAME`]) is the main +//! list. It's goal is to load all the user' rooms. It starts with a +//! [`SlidingSyncMode::Selective`] sync-mode with a small range (i.e. a small +//! set of rooms) to load the first rooms quickly, and then updates to a +//! [`SlidingSyncMode::Growing`] sync-mode to load the remaining rooms “in the +//! background”: it will sync the existing rooms and will fetch new rooms, by +//! a certain batch size. +//! * `visible_rooms` (refered by the constant [`VISIBLE_ROOMS_LIST_NAME`]) is +//! the “reactive” list. It's goal is to react to the client app user actions. +//! If the user scrolls in the room list, the `visible_rooms` will be +//! configured to sync for the particular range of rooms the user is actually +//! seeing (the rooms in the current viewport). `visible_rooms` has a +//! different configuration than `all_rooms` as it loads more timeline events: +//! it means that the room will already have a “history”, a timeline, ready to +//! be presented when the user enters the room. +//! +//! This behavior has proven to be empirically satisfying to provide a fast and +//! fluid user experience for a Matrix client. +//! +//! [`RoomList::entries`] provides a way to get a stream of room list entry. +//! This stream can be filtered, and the filter can be changed over time. +//! +//! [`RoomList::state_stream`] provides a way to get a stream of the state +//! machine's state, which can be pretty helpful for the client app. use std::future::ready; @@ -19,6 +79,7 @@ use thiserror::Error; pub const ALL_ROOMS_LIST_NAME: &str = "all_rooms"; pub const VISIBLE_ROOMS_LIST_NAME: &str = "visible_rooms"; +/// The [`RoomList`] type. See the module's documentation to learn more. #[derive(Debug)] pub struct RoomList { sliding_sync: SlidingSync, @@ -26,6 +87,10 @@ pub struct RoomList { } impl RoomList { + /// Create a new `RoomList`. + /// + /// A [`matrix_sdk::SlidingSync`] client will be created, with a cached list + /// already pre-configured. pub async fn new(client: Client) -> Result { let sliding_sync = client .sliding_sync() @@ -43,6 +108,19 @@ impl RoomList { Ok(Self { sliding_sync, state: Observable::new(State::Init) }) } + /// Start to sync the room list. + /// + /// It's the main method of this entire API. Calling `sync` allows to + /// receive updates on the room list: new rooms, rooms updates etc. Those + /// updates can be read with [`Self::entries`]. This method returns a + /// [`Stream`] where produced items only hold an empty value in case of a + /// sync success, otherwise an error. + /// + /// The `RoomList`' state machine is run by this method. + /// + /// Stopping the [`Stream`] (i.e. stop polling it) and calling + /// [`Self::sync`] again will resume from the previous state of the state + /// machine. pub fn sync(&self) -> impl Stream> + '_ { stream! { let sync = self.sliding_sync.sync(); @@ -86,14 +164,18 @@ impl RoomList { } } + /// Get the actual state of the state machine. pub fn state(&self) -> State { self.state.get() } + /// Get a [`Stream`] of [`State`]s. pub fn state_stream(&self) -> impl Stream { Observable::subscribe(&self.state) } + /// Get all previous room list entries, in addition to a [`Stream`] to room + /// list entry's updates. pub async fn entries( &self, ) -> Result<(Vector, impl Stream>), Error> { @@ -103,6 +185,8 @@ impl RoomList { .ok_or_else(|| Error::UnknownList(ALL_ROOMS_LIST_NAME.to_string())) } + /// Similar to [`Self::entries`] except that it's possible to provide a + /// filter that will filter out room list entries. pub async fn entries_filtered( &self, filter: F, @@ -116,6 +200,7 @@ impl RoomList { .ok_or_else(|| Error::UnknownList(ALL_ROOMS_LIST_NAME.to_string())) } + /// Pass an [`Input`] onto the state machine. pub async fn apply_input(&self, input: Input) -> Result<(), Error> { use Input::*; @@ -141,6 +226,7 @@ impl RoomList { } } +/// [`RoomList`]'s errors. #[derive(Debug, Error)] pub enum Error { /// Error from [`matrix_sdk::SlidingSync`]. @@ -151,16 +237,12 @@ pub enum Error { #[error("Unknown list `{0}`")] UnknownList(String), - #[error("Failed to set up the entries correctly")] - Entries, - - #[error("Failed to acquire a lock to update the entries filter")] - CannotUpdateEntriesFilter, - + /// An input was asked to be applied but it wasn't possible to apply it. #[error("The input has been not applied")] InputHasNotBeenApplied(Input), } +/// The state of the [`RoomList`]' state machine. #[derive(Copy, Clone, Debug, PartialEq)] pub enum State { /// That's the first initial state. @@ -182,6 +264,8 @@ pub enum State { } impl State { + /// Transition to the next state, and execute the associated transition's + /// [`Actions`]. async fn next(&self, sliding_sync: &SlidingSync) -> Result { use State::*; @@ -201,6 +285,7 @@ impl State { } } +/// A trait to define what an `Action` is. #[async_trait] trait Action { async fn run(&self, sliding_sync: &SlidingSync) -> Result<(), Error>; @@ -239,9 +324,15 @@ impl Action for ChangeAllRoomsListToGrowingSyncMode { } } +/// Type alias to represent one action. type OneAction = Box; + +/// Type alias to represent many actions. type ManyActions = Vec; +/// A type to represent multiple actions. +/// +/// It contains helper methods to create pre-configured set of actions. struct Actions { actions: &'static Lazy, } @@ -280,8 +371,17 @@ impl Actions { } } +/// An input for the [`RoomList`]' state machine. +/// +/// An input is something that has happened or is happening or is requested by +/// the client app using this [`RoomList`]. #[derive(Debug)] pub enum Input { + /// The client app's viewport of the room list has changed. + /// + /// Use this input when the user of the client app is scrolling inside the + /// room list, and the viewport has changed. The viewport is defined as the + /// range of visible rooms in the room list. Viewport(Ranges), } From e18728aa5ff65dcceac95c3c238468ca6672c5c8 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Fri, 2 Jun 2023 20:06:50 +0200 Subject: [PATCH 36/48] feat(ui): Set `timeline_limit` for `all_rooms` and `visible_rooms`. --- crates/matrix-sdk-ui/src/room_list/mod.rs | 6 ++++-- crates/matrix-sdk-ui/tests/integration/room_list.rs | 3 +++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 3c01ccf19..1b2c1fcbc 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -97,7 +97,8 @@ impl RoomList { .storage_key(Some("matrix-sdk-ui-roomlist".to_string())) .add_cached_list( SlidingSyncList::builder(ALL_ROOMS_LIST_NAME) - .sync_mode(SlidingSyncMode::new_selective().add_range(0..=19)), + .sync_mode(SlidingSyncMode::new_selective().add_range(0..=19)) + .timeline_limit(1), ) .await .map_err(Error::SlidingSync)? @@ -299,7 +300,8 @@ impl Action for AddVisibleRoomsList { sliding_sync .add_list( SlidingSyncList::builder(VISIBLE_ROOMS_LIST_NAME) - .sync_mode(SlidingSyncMode::new_selective()), + .sync_mode(SlidingSyncMode::new_selective()) + .timeline_limit(20), ) .await .map_err(Error::SlidingSync)?; diff --git a/crates/matrix-sdk-ui/tests/integration/room_list.rs b/crates/matrix-sdk-ui/tests/integration/room_list.rs index 6a4ef442d..8759deb12 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list.rs @@ -212,6 +212,7 @@ async fn test_sync_from_init_to_enjoy() -> Result<(), Error> { ["m.room.encryption", ""], ], "sort": ["by_recency", "by_name"], + "timeline_limit": 1, }, }, "extensions": { @@ -249,6 +250,7 @@ async fn test_sync_from_init_to_enjoy() -> Result<(), Error> { "ranges": [ [0, 49], ], + "timeline_limit": 1, }, VISIBLE_ROOMS: { "ranges": [], @@ -256,6 +258,7 @@ async fn test_sync_from_init_to_enjoy() -> Result<(), Error> { ["m.room.encryption", ""], ], "sort": ["by_recency", "by_name"], + "timeline_limit": 20, } } }, From dcad8970846dbf62c02d7e987f23b4288cec1f68 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Fri, 2 Jun 2023 20:49:33 +0200 Subject: [PATCH 37/48] test(ui): Ensure the `timeline_limit` is not reset. --- crates/matrix-sdk-ui/tests/integration/room_list.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/matrix-sdk-ui/tests/integration/room_list.rs b/crates/matrix-sdk-ui/tests/integration/room_list.rs index 8759deb12..a2dee96a2 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list.rs @@ -769,6 +769,7 @@ async fn test_input_viewport() -> Result<(), Error> { }, VISIBLE_ROOMS: { "ranges": [], + "timeline_limit": 20, } } }, @@ -791,6 +792,7 @@ async fn test_input_viewport() -> Result<(), Error> { }, VISIBLE_ROOMS: { "ranges": [[10, 15], [20, 25]], + "timeline_limit": 20, } } }, From 537f95b6835637d91fc988fad28c4fa9a86ff041 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Fri, 2 Jun 2023 20:57:35 +0200 Subject: [PATCH 38/48] feat(sdk): `SlidingSync::get_rooms?` are now async. --- bindings/matrix-sdk-ffi/src/sliding_sync.rs | 45 +++++++++++-------- crates/matrix-sdk-ui/src/room_list/mod.rs | 28 +++++++++++- .../integration/timeline/sliding_sync.rs | 3 +- crates/matrix-sdk/src/sliding_sync/mod.rs | 12 ++--- 4 files changed, 60 insertions(+), 28 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/sliding_sync.rs b/bindings/matrix-sdk-ffi/src/sliding_sync.rs index a8967b8f3..00c3a2a6a 100644 --- a/bindings/matrix-sdk-ffi/src/sliding_sync.rs +++ b/bindings/matrix-sdk-ffi/src/sliding_sync.rs @@ -701,12 +701,16 @@ impl SlidingSync { } pub fn get_room(&self, room_id: String) -> Result>, ClientError> { - Ok(self.inner.get_room(<&RoomId>::try_from(room_id.as_str())?).map(|inner| { - Arc::new(SlidingSyncRoom { - inner, - sliding_sync: self.inner.clone(), - client: self.client.clone(), - timeline: Default::default(), + let room_id = <&RoomId>::try_from(room_id.as_str())?; + + Ok(RUNTIME.block_on(async move { + self.inner.get_room(room_id).await.map(|inner| { + Arc::new(SlidingSyncRoom { + inner, + sliding_sync: self.inner.clone(), + client: self.client.clone(), + timeline: Default::default(), + }) }) })) } @@ -719,21 +723,24 @@ impl SlidingSync { .into_iter() .map(OwnedRoomId::try_from) .collect::, IdParseError>>()?; - Ok(self - .inner - .get_rooms(actual_ids.into_iter()) - .into_iter() - .map(|o| { - o.map(|inner| { - Arc::new(SlidingSyncRoom { - inner, - sliding_sync: self.inner.clone(), - client: self.client.clone(), - timeline: Default::default(), + + Ok(RUNTIME.block_on(async move { + self.inner + .get_rooms(actual_ids.into_iter()) + .await + .into_iter() + .map(|o| { + o.map(|inner| { + Arc::new(SlidingSyncRoom { + inner, + sliding_sync: self.inner.clone(), + client: self.client.clone(), + timeline: Default::default(), + }) }) }) - }) - .collect()) + .collect() + })) } pub fn add_list(&self, list_builder: Arc) -> Arc { diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 1b2c1fcbc..8af73b48d 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -60,7 +60,7 @@ //! [`RoomList::state_stream`] provides a way to get a stream of the state //! machine's state, which can be pretty helpful for the client app. -use std::future::ready; +use std::{future::ready, sync::Arc}; use async_stream::stream; use async_trait::async_trait; @@ -71,9 +71,10 @@ use imbl::Vector; pub use matrix_sdk::RoomListEntry; use matrix_sdk::{ sliding_sync::Ranges, Client, Error as SlidingSyncError, SlidingSync, SlidingSyncList, - SlidingSyncMode, + SlidingSyncMode, SlidingSyncRoom, }; use once_cell::sync::Lazy; +use ruma::RoomId; use thiserror::Error; pub const ALL_ROOMS_LIST_NAME: &str = "all_rooms"; @@ -221,12 +222,35 @@ impl RoomList { Ok(()) } + pub fn get_room(&self, room_id: &RoomId) -> Option { + self.sliding_sync.get_room(room_id).map(Room::new) + } + #[cfg(any(test, feature = "testing"))] pub fn sliding_sync(&self) -> &SlidingSync { &self.sliding_sync } } +#[derive(Clone, Debug)] +pub struct Room { + inner: Arc, +} + +#[derive(Debug)] +struct RoomInner { + sliding_sync_room: SlidingSyncRoom, + room: Option, +} + +impl Room { + fn new(sliding_sync_room: SlidingSyncRoom) -> Self { + let room = sliding_sync_room.client().get_room(sliding_sync_room.room_id()); + + Self { inner: Arc::new(RoomInner { sliding_sync_room, room }) } + } +} + /// [`RoomList`]'s errors. #[derive(Debug, Error)] pub enum Error { diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/sliding_sync.rs b/crates/matrix-sdk-ui/tests/integration/timeline/sliding_sync.rs index b9f650c83..d8a006949 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/sliding_sync.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/sliding_sync.rs @@ -187,7 +187,7 @@ async fn create_one_room( assert!(update.rooms.contains(&room_id.to_owned())); - let room = sliding_sync.get_room(room_id).context("`get_room`")?; + let room = sliding_sync.get_room(room_id).await.context("`get_room`")?; assert_eq!(room.name(), Some(room_name.clone())); Ok(()) @@ -199,6 +199,7 @@ async fn timeline( ) -> Result<(Vector>, impl Stream>>)> { Ok(sliding_sync .get_room(room_id) + .await .unwrap() .timeline() .await diff --git a/crates/matrix-sdk/src/sliding_sync/mod.rs b/crates/matrix-sdk/src/sliding_sync/mod.rs index b2b66cdf1..50536737a 100644 --- a/crates/matrix-sdk/src/sliding_sync/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/mod.rs @@ -164,8 +164,8 @@ impl SlidingSync { } /// Lookup a specific room - pub fn get_room(&self, room_id: &RoomId) -> Option { - self.inner.rooms.blocking_read().get(room_id).cloned() + pub async fn get_room(&self, room_id: &RoomId) -> Option { + self.inner.rooms.read().await.get(room_id).cloned() } /// Check the number of rooms. @@ -247,18 +247,18 @@ impl SlidingSync { } /// Lookup a set of rooms - pub fn get_rooms>( + pub async fn get_rooms>( &self, room_ids: I, ) -> Vec> { - let rooms = self.inner.rooms.blocking_read(); + let rooms = self.inner.rooms.read().await; room_ids.map(|room_id| rooms.get(&room_id).cloned()).collect() } /// Get all rooms. - pub fn get_all_rooms(&self) -> Vec { - self.inner.rooms.blocking_read().values().cloned().collect() + pub async fn get_all_rooms(&self) -> Vec { + self.inner.rooms.read().await.values().cloned().collect() } fn prepare_extension_config(&self, pos: Option<&str>) -> ExtensionsConfig { From 67ef42f4c52afa5bf875448249e44c81f6344991 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Fri, 2 Jun 2023 21:28:45 +0200 Subject: [PATCH 39/48] feat(ui): Implement `RoomList::room`. This patch implements `RoomList::room`, which returns a `Result`: the room ay or may not exist. A new `Room` type is created. It wraps an `Arc` type, so that it's cheap to clone it. The `RoomInner` type owns a `SlidingSyncRoom` and `matrix_sdk::room::Room` type. If some API or data are missing on the former, the latter acts as a backup. This is the case for the `Room::name` method which makes it best to return a name to the caller. --- crates/matrix-sdk-ui/src/room_list/mod.rs | 29 ++++- .../tests/integration/room_list.rs | 105 +++++++++++++++++- 2 files changed, 128 insertions(+), 6 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 8af73b48d..387650077 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -74,7 +74,7 @@ use matrix_sdk::{ SlidingSyncMode, SlidingSyncRoom, }; use once_cell::sync::Lazy; -use ruma::RoomId; +use ruma::{OwnedRoomId, RoomId}; use thiserror::Error; pub const ALL_ROOMS_LIST_NAME: &str = "all_rooms"; @@ -222,8 +222,12 @@ impl RoomList { Ok(()) } - pub fn get_room(&self, room_id: &RoomId) -> Option { - self.sliding_sync.get_room(room_id).map(Room::new) + pub async fn room(&self, room_id: &RoomId) -> Result { + self.sliding_sync + .get_room(room_id) + .await + .map(Room::new) + .ok_or_else(|| Error::RoomNotFound(room_id.to_owned())) } #[cfg(any(test, feature = "testing"))] @@ -232,6 +236,9 @@ impl RoomList { } } +/// A room in the room list. +/// +/// It's cheap to clone this type. #[derive(Clone, Debug)] pub struct Room { inner: Arc, @@ -244,11 +251,23 @@ struct RoomInner { } impl Room { + /// Create a new `Room`. fn new(sliding_sync_room: SlidingSyncRoom) -> Self { let room = sliding_sync_room.client().get_room(sliding_sync_room.room_id()); Self { inner: Arc::new(RoomInner { sliding_sync_room, room }) } } + + /// Get the best possible name for the room. + /// + /// If the sliding sync room has received a name from the server, then use + /// it, otherwise, let's calculate a name. + pub async fn name(&self) -> Option { + Some(match self.inner.sliding_sync_room.name() { + Some(name) => name, + None => self.inner.room.as_ref()?.display_name().await.ok()?.to_string(), + }) + } } /// [`RoomList`]'s errors. @@ -265,6 +284,10 @@ pub enum Error { /// An input was asked to be applied but it wasn't possible to apply it. #[error("The input has been not applied")] InputHasNotBeenApplied(Input), + + /// The requested room doesn't exist. + #[error("Room `{0}` not found")] + RoomNotFound(OwnedRoomId), } /// The state of the [`RoomList`]' state machine. diff --git a/crates/matrix-sdk-ui/tests/integration/room_list.rs b/crates/matrix-sdk-ui/tests/integration/room_list.rs index a2dee96a2..c8f7a570c 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list.rs @@ -36,7 +36,7 @@ impl Match for SlidingSyncMatcher { macro_rules! sync_then_assert_request_and_fake_response { ( [$server:ident, $room_list:ident, $room_list_sync_stream:ident] - states = $pre_state:ident -> $post_state:ident, + $( states = $pre_state:ident -> $post_state:ident, )? assert request = { $( $request_json:tt )* }, respond with = { $( $response_json:tt )* } $(,)? @@ -49,7 +49,7 @@ macro_rules! sync_then_assert_request_and_fake_response { .mount_as_scoped(&$server) .await; - assert_eq!(State:: $pre_state, $room_list.state(), "pre state"); + $( assert_eq!(State:: $pre_state, $room_list.state(), "pre state"); )? let next = $room_list_sync_stream.next().await.unwrap()?; @@ -70,7 +70,7 @@ macro_rules! sync_then_assert_request_and_fake_response { } } - assert_eq!(State:: $post_state, $room_list.state(), "post state"); + $( assert_eq!(State:: $post_state, $room_list.state(), "post state"); )? next } @@ -728,6 +728,105 @@ async fn test_entries_stream_with_updated_filter() -> Result<(), Error> { Ok(()) } +#[async_test] +async fn test_room() -> Result<(), Error> { + let (server, room_list) = new_room_list().await?; + + let sync = room_list.sync(); + pin_mut!(sync); + + let room_id_0 = room_id!("!r0:bar.org"); + let room_id_1 = room_id!("!r1:bar.org"); + + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + assert request = {}, + respond with = { + "pos": "0", + "lists": { + ALL_ROOMS: { + "count": 2, + "ops": [ + { + "op": "SYNC", + "range": [0, 1], + "room_ids": [ + room_id_0, + room_id_1, + ], + }, + ], + }, + }, + "rooms": { + room_id_0: { + "name": "Room #0", + "initial": true, + }, + room_id_1: { + "initial": true, + }, + }, + }, + }; + + // Room has received a name from sliding sync. + let room0 = room_list.room(room_id_0).await?; + assert_eq!(room0.name().await, Some("Room #0".to_string())); + + // Room has not received a name from sliding sync, then it's calculated. + let room1 = room_list.room(room_id_1).await?; + assert_eq!(room1.name().await, Some("Empty Room".to_string())); + + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + assert request = {}, + respond with = { + "pos": "1", + "lists": { + ALL_ROOMS: { + "count": 2, + "ops": [ + { + "op": "SYNC", + "range": [1, 1], + "room_ids": [ + room_id_1, + ], + }, + ], + }, + }, + "rooms": { + room_id_1: { + "name": "Room #1", + }, + }, + }, + }; + + // Room has _now_ received a name from sliding sync! + assert_eq!(room1.name().await, Some("Room #1".to_string())); + + Ok(()) +} + +#[async_test] +async fn test_room_not_found() -> Result<(), Error> { + let (_server, room_list) = new_room_list().await?; + + let room_id = room_id!("!foo:bar.org"); + + assert_matches!( + room_list.room(room_id).await, + Err(Error::RoomNotFound(error_room_id)) => { + assert_eq!(error_room_id, room_id.to_owned()); + } + ); + + Ok(()) +} + #[async_test] async fn test_input_viewport() -> Result<(), Error> { let (server, room_list) = new_room_list().await?; From c1a24cf0337b8459dd3e3d4e8b35f6777bc42208 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Fri, 2 Jun 2023 22:32:57 +0200 Subject: [PATCH 40/48] feat(ui): Implement `Room::timeline` and `::latest_event`. This patch changes `RoomInner` to have an `room: Option` field to `room: matrix_sdk::room::Room`. `room` is not longer an `Option`. Then, it's easier to have a new `timeline: Timeline` field, along with a `sneaky_timeline: Timeline` field. Both are used by the new `Room::timeline()` and `Room::latest_event()` method. --- crates/matrix-sdk-ui/src/room_list/mod.rs | 61 +++++- .../tests/integration/room_list.rs | 183 +++++++++++++++++- .../tests/integration/timeline/mod.rs | 2 +- .../integration/timeline/sliding_sync.rs | 11 +- 4 files changed, 243 insertions(+), 14 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 387650077..e1305dbee 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -77,6 +77,8 @@ use once_cell::sync::Lazy; use ruma::{OwnedRoomId, RoomId}; use thiserror::Error; +use crate::{timeline::EventTimelineItem, Timeline}; + pub const ALL_ROOMS_LIST_NAME: &str = "all_rooms"; pub const VISIBLE_ROOMS_LIST_NAME: &str = "visible_rooms"; @@ -222,12 +224,12 @@ impl RoomList { Ok(()) } + /// Get a [`Room`] if it exists. pub async fn room(&self, room_id: &RoomId) -> Result { - self.sliding_sync - .get_room(room_id) - .await - .map(Room::new) - .ok_or_else(|| Error::RoomNotFound(room_id.to_owned())) + match self.sliding_sync.get_room(room_id).await { + Some(room) => Room::new(room).await, + None => Err(Error::RoomNotFound(room_id.to_owned())), + } } #[cfg(any(test, feature = "testing"))] @@ -246,16 +248,42 @@ pub struct Room { #[derive(Debug)] struct RoomInner { + /// The Sliding Sync room. sliding_sync_room: SlidingSyncRoom, - room: Option, + + /// The underlying client room. + room: matrix_sdk::room::Room, + + /// The timeline of the room. + timeline: Timeline, + + /// The “sneaky” timeline of the room, i.e. this timeline doesn't track the + /// read marker nor the receipts. + sneaky_timeline: Timeline, } impl Room { /// Create a new `Room`. - fn new(sliding_sync_room: SlidingSyncRoom) -> Self { - let room = sliding_sync_room.client().get_room(sliding_sync_room.room_id()); + async fn new(sliding_sync_room: SlidingSyncRoom) -> Result { + let room = sliding_sync_room + .client() + .get_room(sliding_sync_room.room_id()) + .ok_or_else(|| Error::RoomNotFound(sliding_sync_room.room_id().to_owned()))?; - Self { inner: Arc::new(RoomInner { sliding_sync_room, room }) } + let timeline = Timeline::builder(&room) + .events(sliding_sync_room.prev_batch(), sliding_sync_room.timeline_queue()) + .track_read_marker_and_receipts() + .build() + .await; + + let sneaky_timeline = Timeline::builder(&room) + .events(sliding_sync_room.prev_batch(), sliding_sync_room.timeline_queue()) + .build() + .await; + + Ok(Self { + inner: Arc::new(RoomInner { sliding_sync_room, room, timeline, sneaky_timeline }), + }) } /// Get the best possible name for the room. @@ -265,9 +293,22 @@ impl Room { pub async fn name(&self) -> Option { Some(match self.inner.sliding_sync_room.name() { Some(name) => name, - None => self.inner.room.as_ref()?.display_name().await.ok()?.to_string(), + None => self.inner.room.display_name().await.ok()?.to_string(), }) } + + /// Get the timeline of the room. + pub fn timeline(&self) -> &Timeline { + &self.inner.timeline + } + + /// Get the latest event of the timeline. + /// + /// It's different from `Self::timeline().latest_event()` as it won't track + /// the read marker and receipts. + pub async fn latest_event(&self) -> Option { + self.inner.sneaky_timeline.latest_event().await + } } /// [`RoomList`]'s errors. diff --git a/crates/matrix-sdk-ui/tests/integration/room_list.rs b/crates/matrix-sdk-ui/tests/integration/room_list.rs index c8f7a570c..8e94d1c02 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list.rs @@ -8,13 +8,17 @@ use matrix_sdk_ui::{ Error, Input, RoomListEntry, State, ALL_ROOMS_LIST_NAME as ALL_ROOMS, VISIBLE_ROOMS_LIST_NAME as VISIBLE_ROOMS, }, + timeline::{TimelineItem, VirtualTimelineItem}, RoomList, }; -use ruma::room_id; +use ruma::{event_id, room_id}; use serde_json::json; use wiremock::{http::Method, Match, Mock, MockServer, Request, ResponseTemplate}; -use crate::logged_in_client; +use crate::{ + logged_in_client, + timeline::sliding_sync::{assert_timeline_stream, timeline_event}, +}; async fn new_room_list() -> Result<(MockServer, RoomList), Error> { let (client, server) = logged_in_client().await; @@ -827,6 +831,181 @@ async fn test_room_not_found() -> Result<(), Error> { Ok(()) } +#[async_test] +async fn test_room_timeline() -> Result<(), Error> { + let (server, room_list) = new_room_list().await?; + + let sync = room_list.sync(); + pin_mut!(sync); + + let room_id = room_id!("!r0:bar.org"); + + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + assert request = {}, + respond with = { + "pos": "0", + "lists": { + ALL_ROOMS: { + "count": 2, + "ops": [ + { + "op": "SYNC", + "range": [0, 0], + "room_ids": [room_id], + }, + ], + }, + }, + "rooms": { + room_id: { + "name": "Room #0", + "initial": true, + "timeline": [ + timeline_event!("$x0:bar.org" at 0 sec), + ], + }, + }, + }, + }; + + let room = room_list.room(room_id).await?; + let timeline = room.timeline(); + + let (previous_timeline_items, mut timeline_items_stream) = timeline.subscribe().await; + + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + assert request = {}, + respond with = { + "pos": "0", + "lists": {}, + "rooms": { + room_id: { + "timeline": [ + timeline_event!("$x1:bar.org" at 1 sec), + timeline_event!("$x2:bar.org" at 2 sec), + ], + }, + }, + }, + }; + + // Previous timeline items. + assert_matches!( + previous_timeline_items[0].as_ref(), + TimelineItem::Virtual(VirtualTimelineItem::DayDivider(_)) + ); + assert_matches!( + previous_timeline_items[1].as_ref(), + TimelineItem::Event(item) => { + assert_eq!(item.event_id().unwrap().as_str(), "$x0:bar.org"); + } + ); + + // Timeline items stream. + assert_timeline_stream! { + [timeline_items_stream] + update[1] "$x0:bar.org"; + append "$x1:bar.org"; + update[2] "$x1:bar.org"; + append "$x2:bar.org"; + }; + + Ok(()) +} + +#[async_test] +async fn test_room_latest_event() -> Result<(), Error> { + let (server, room_list) = new_room_list().await?; + + let sync = room_list.sync(); + pin_mut!(sync); + + let room_id = room_id!("!r0:bar.org"); + + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + assert request = {}, + respond with = { + "pos": "0", + "lists": { + ALL_ROOMS: { + "count": 2, + "ops": [ + { + "op": "SYNC", + "range": [0, 0], + "room_ids": [room_id], + }, + ], + }, + }, + "rooms": { + room_id: { + "name": "Room #0", + "initial": true, + }, + }, + }, + }; + + let room = room_list.room(room_id).await?; + + // The latest event does not exist. + assert!(room.latest_event().await.is_none()); + + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + assert request = {}, + respond with = { + "pos": "0", + "lists": {}, + "rooms": { + room_id: { + "timeline": [ + timeline_event!("$x0:bar.org" at 0 sec), + ], + }, + }, + }, + }; + + // The latest event exists. + assert_matches!( + room.latest_event().await, + Some(event) => { + assert_eq!(event.event_id(), Some(event_id!("$x0:bar.org"))); + } + ); + + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + assert request = {}, + respond with = { + "pos": "0", + "lists": {}, + "rooms": { + room_id: { + "timeline": [ + timeline_event!("$x1:bar.org" at 1 sec), + ], + }, + }, + }, + }; + + // The latest event has been updated. + assert_matches!( + room.latest_event().await, + Some(event) => { + assert_eq!(event.event_id(), Some(event_id!("$x1:bar.org"))); + } + ); + + Ok(()) +} + #[async_test] async fn test_input_viewport() -> Result<(), Error> { let (server, room_list) = new_room_list().await?; diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/mod.rs b/crates/matrix-sdk-ui/tests/integration/timeline/mod.rs index a5b1588ed..a4870f542 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/mod.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/mod.rs @@ -28,7 +28,7 @@ use wiremock::{ mod read_receipts; #[cfg(feature = "experimental-sliding-sync")] -mod sliding_sync; +pub(crate) mod sliding_sync; use crate::{logged_in_client, mock_encryption_state, mock_sync}; diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/sliding_sync.rs b/crates/matrix-sdk-ui/tests/integration/timeline/sliding_sync.rs index d8a006949..a6b7cd3dc 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/sliding_sync.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/sliding_sync.rs @@ -50,6 +50,8 @@ macro_rules! timeline_event { } } +pub(crate) use timeline_event; + macro_rules! assert_timeline_stream { // `--- day divider ---` ( @_ [ $stream:ident ] [ --- day divider --- ; $( $rest:tt )* ] [ $( $accumulator:tt )* ] ) => { @@ -63,7 +65,12 @@ macro_rules! assert_timeline_stream { assert_matches!( $stream.next().now_or_never(), Some(Some(VectorDiff::PushBack { value })) => { - assert_matches!(value.as_ref(), TimelineItem::Virtual(VirtualTimelineItem::DayDivider(_))); + assert_matches!( + value.as_ref(), + TimelineItem::Virtual( + VirtualTimelineItem::DayDivider(_) + ) + ); } ); } @@ -148,6 +155,8 @@ macro_rules! assert_timeline_stream { }; } +pub(crate) use assert_timeline_stream; + async fn new_sliding_sync(lists: Vec) -> Result<(MockServer, SlidingSync)> { let (client, server) = logged_in_client().await; From 4bfc48b4ae992b4282026efad0127265d7a87676 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 5 Jun 2023 15:05:42 +0200 Subject: [PATCH 41/48] feat(sdk): Simplify returned values of `SlidingSync::sync`. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First off, `SlidingSync::sync_once` no longer returns `Option` but `UpdateSummary`. It simplifies the rest of the patch. Next, `SlidingSync::sync` returns either `Ok(…)` and continues to sync, or it returns `Err(…)` and it stops the sync stream immediately. No more error could be returned with the stream to continue syncing. Finally, the `UnknownPos` error no longer emits an err, but silently skip over the sync-loop until the threshold is reached; which in this case will generate a proper error. --- crates/matrix-sdk/src/sliding_sync/mod.rs | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/crates/matrix-sdk/src/sliding_sync/mod.rs b/crates/matrix-sdk/src/sliding_sync/mod.rs index 50536737a..ee63719c3 100644 --- a/crates/matrix-sdk/src/sliding_sync/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/mod.rs @@ -388,7 +388,7 @@ impl SlidingSync { } #[instrument(skip_all, fields(pos))] - async fn sync_once(&self) -> Result> { + async fn sync_once(&self) -> Result { let (request, request_config, requested_room_unsubscriptions) = { // Collect requests for lists. let mut requests_lists = BTreeMap::new(); @@ -396,10 +396,6 @@ impl SlidingSync { { let mut lists = self.inner.lists.write().await; - if lists.is_empty() { - return Ok(None); - } - for (name, list) in lists.iter_mut() { requests_lists.insert(name.clone(), list.next_request()?); } @@ -520,7 +516,7 @@ impl SlidingSync { debug!("Sliding Sync response has been fully handled"); - Ok(Some(updates)) + Ok(updates) }; spawn(future.instrument(Span::current())).await.unwrap() @@ -530,6 +526,10 @@ impl SlidingSync { /// /// This method returns a `Stream`, which will send requests and will handle /// responses automatically. Lists and rooms are updated automatically. + /// + /// This function returns `Ok(…)` if everything went well, otherwise it will + /// return `Err(…)`. An `Err` will _always_ lead to the `Stream` + /// termination. #[allow(unknown_lints, clippy::let_with_type_underscore)] // triggered by instrument macro #[instrument(name = "sync_stream", skip_all)] pub fn sync(&self) -> impl Stream> + '_ { @@ -567,17 +567,12 @@ impl SlidingSync { update_summary = self.sync_once().instrument(sync_span.clone()) => { match update_summary { - Ok(Some(updates)) => { + Ok(updates) => { self.inner.reset_counter.store(0, Ordering::SeqCst); yield Ok(updates); } - Ok(None) => { - // Terminates the loop, and terminates the stream. - break; - } - Err(error) => { if error.client_api_error_kind() == Some(&ErrorKind::UnknownPos) { // The session has expired. @@ -609,11 +604,13 @@ impl SlidingSync { debug!(?self.inner.extensions, ?self.inner.position, "Sliding Sync has been reset"); }); + + continue; } yield Err(error); - continue; + break; } } } From ad8d7caecbb0cd75f5d7370b9f5c4645e87d621d Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 5 Jun 2023 15:09:10 +0200 Subject: [PATCH 42/48] feat(ui): Handle `RoomList`'s `State::Terminated` properly. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch handles errors from the Sliding Sync loop properly. When an error is received, the `RoomList` state machine enters the `Terminated` state. Immediately after that, the sync stream is stopped. When the sync stream is restarted, the next state will be calculated. When the current state is `Terminated`, the next state is the state that led to the `Terminated` state. To avoid having a “first” huge sync (imagine a room list of 1000 rooms, you don't want to “resume” from an error with a first sync for 1000 rooms), lists are partially “reset” depending of the state where the machine is in. An important test has been added to test all possibles cases with errors and list' states. --- crates/matrix-sdk-ui/src/room_list/mod.rs | 111 ++++-- .../tests/integration/room_list.rs | 337 ++++++++++++++++-- 2 files changed, 404 insertions(+), 44 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index e1305dbee..06c9a3f9c 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -152,12 +152,17 @@ impl RoomList { } Some(Err(error)) => { - // TODO: what to do when an error is raised? + let next_state = State::Terminated { from: Box::new(self.state.get()) }; + + Observable::set(&self.state, next_state); + yield Err(Error::SlidingSync(error)); + + break; } None => { - let next_state = State::Terminated; + let next_state = State::Terminated { from: Box::new(self.state.get()) }; Observable::set(&self.state, next_state); @@ -210,20 +215,26 @@ impl RoomList { match input { Viewport(ranges) => { - self.sliding_sync - .on_list(VISIBLE_ROOMS_LIST_NAME, |list| { - ready(list.set_sync_mode( - SlidingSyncMode::new_selective().add_ranges(ranges.clone()), - )) - }) - .await - .ok_or_else(|| Error::InputHasNotBeenApplied(Viewport(ranges)))?; + self.update_viewport(ranges).await?; } } Ok(()) } + async fn update_viewport(&self, ranges: Ranges) -> Result<(), Error> { + self.sliding_sync + .on_list(VISIBLE_ROOMS_LIST_NAME, |list| { + ready( + list.set_sync_mode(SlidingSyncMode::new_selective().add_ranges(ranges.clone())), + ) + }) + .await + .ok_or_else(|| Error::InputHasNotBeenApplied(Input::Viewport(ranges)))?; + + Ok(()) + } + /// Get a [`Room`] if it exists. pub async fn room(&self, room_id: &RoomId) -> Result { match self.sliding_sync.get_room(room_id).await { @@ -332,7 +343,7 @@ pub enum Error { } /// The state of the [`RoomList`]' state machine. -#[derive(Copy, Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq)] pub enum State { /// That's the first initial state. Init, @@ -349,7 +360,7 @@ pub enum State { /// At this state, the sync has been stopped (because it was requested, or /// because it has errored too many times previously). - Terminated, + Terminated { from: Box }, } impl State { @@ -363,7 +374,27 @@ impl State { FirstRooms => (AllRooms, Actions::first_rooms_are_loaded()), AllRooms => (Enjoy, Actions::none()), Enjoy => (Enjoy, Actions::none()), - Terminated => (Terminated, Actions::none()), + // If the state was `Terminated` but the next state is calculated again, it means the + // sync has been restarted. In this case, let's jump back on the previous state that led + // to the termination. No action is required in this scenario. + Terminated { from: previous_state } => { + match previous_state.as_ref() { + state @ Init | state @ FirstRooms => { + // Do nothing. + (state.to_owned(), Actions::none()) + } + + state @ AllRooms | state @ Enjoy => { + // Refresh the lists. + (state.to_owned(), Actions::refresh_lists()) + } + + Terminated { .. } => { + // Having `Terminated { from: Terminated { … } }` is not allowed. + unreachable!("It's impossible to reach `Terminated` from `Terminated`"); + } + } + } }; for action in actions.iter() { @@ -398,10 +429,10 @@ impl Action for AddVisibleRoomsList { } } -struct ChangeAllRoomsListToGrowingSyncMode; +struct SetAllRoomsListToGrowingSyncMode; #[async_trait] -impl Action for ChangeAllRoomsListToGrowingSyncMode { +impl Action for SetAllRoomsListToGrowingSyncMode { async fn run(&self, sliding_sync: &SlidingSync) -> Result<(), Error> { sliding_sync .on_list(ALL_ROOMS_LIST_NAME, |list| { @@ -453,7 +484,8 @@ macro_rules! actions { impl Actions { actions! { none => [], - first_rooms_are_loaded => [ChangeAllRoomsListToGrowingSyncMode, AddVisibleRoomsList], + first_rooms_are_loaded => [SetAllRoomsListToGrowingSyncMode, AddVisibleRoomsList], + refresh_lists => [SetAllRoomsListToGrowingSyncMode], } fn iter(&self) -> &[OneAction] { @@ -535,24 +567,59 @@ mod tests { let room_list = new_room_list().await?; let sliding_sync = room_list.sliding_sync(); + // First state. let state = State::Init; + // Hypothetical termination. + { + let state = + State::Terminated { from: Box::new(state.clone()) }.next(&sliding_sync).await?; + assert_eq!(state, State::Init); + } + + // Next state. let state = state.next(&sliding_sync).await?; assert_eq!(state, State::FirstRooms); + // Hypothetical termination. + { + let state = + State::Terminated { from: Box::new(state.clone()) }.next(&sliding_sync).await?; + assert_eq!(state, State::FirstRooms); + } + + // Next state. let state = state.next(&sliding_sync).await?; assert_eq!(state, State::AllRooms); + // Hypothetical termination. + { + let state = + State::Terminated { from: Box::new(state.clone()) }.next(&sliding_sync).await?; + assert_eq!(state, State::AllRooms); + } + + // Next state. let state = state.next(&sliding_sync).await?; assert_eq!(state, State::Enjoy); + // Hypothetical termination. + { + let state = + State::Terminated { from: Box::new(state.clone()) }.next(&sliding_sync).await?; + assert_eq!(state, State::Enjoy); + } + + // Next state. let state = state.next(&sliding_sync).await?; assert_eq!(state, State::Enjoy); - let state = State::Terminated; - - let state = state.next(&sliding_sync).await?; - assert_eq!(state, State::Terminated); + // Hypothetical termination. + { + let state = + State::Terminated { from: Box::new(state.clone()) }.next(&sliding_sync).await?; + assert_eq!(state, State::Enjoy); + } Ok(()) } @@ -583,7 +650,7 @@ mod tests { } #[async_test] - async fn test_action_change_all_rooms_list_to_growing_sync_mode() -> Result<(), Error> { + async fn test_action_set_all_rooms_list_to_growing_sync_mode() -> Result<(), Error> { let room_list = new_room_list().await?; let sliding_sync = room_list.sliding_sync(); @@ -602,7 +669,7 @@ mod tests { ); // Run the action! - ChangeAllRoomsListToGrowingSyncMode.run(sliding_sync).await.unwrap(); + SetAllRoomsListToGrowingSyncMode.run(sliding_sync).await.unwrap(); // List is still present, in Growing mode. assert_eq!( diff --git a/crates/matrix-sdk-ui/tests/integration/room_list.rs b/crates/matrix-sdk-ui/tests/integration/room_list.rs index 8e94d1c02..1da379f18 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list.rs @@ -40,22 +40,48 @@ impl Match for SlidingSyncMatcher { macro_rules! sync_then_assert_request_and_fake_response { ( [$server:ident, $room_list:ident, $room_list_sync_stream:ident] - $( states = $pre_state:ident -> $post_state:ident, )? + $( states = $pre_state:pat => $post_state:pat, )? assert request = { $( $request_json:tt )* }, - respond with = { $( $response_json:tt )* } + respond with = $( ( code $code:expr ) )? { $( $response_json:tt )* } + $(,)? + ) => { + sync_then_assert_request_and_fake_response! { + [$server, $room_list, $room_list_sync_stream] + sync matches Some(Ok(_)), + $( states = $pre_state => $post_state, )? + assert request = { $( $request_json )* }, + respond with = $( ( code $code ) )? { $( $response_json )* }, + } + }; + + ( + [$server:ident, $room_list:ident, $room_list_sync_stream:ident] + sync matches $sync_result:pat, + $( states = $pre_state:pat => $post_state:pat, )? + assert request = { $( $request_json:tt )* }, + respond with = $( ( code $code:expr ) )? { $( $response_json:tt )* } $(,)? ) => { { + let _code = 200; + $( let _code = $code; )? + let _mock_guard = Mock::given(SlidingSyncMatcher) - .respond_with(ResponseTemplate::new(200).set_body_json( + .respond_with(ResponseTemplate::new(_code).set_body_json( json!({ $( $response_json )* }) )) .mount_as_scoped(&$server) .await; - $( assert_eq!(State:: $pre_state, $room_list.state(), "pre state"); )? + $( + use State::*; - let next = $room_list_sync_stream.next().await.unwrap()?; + assert_matches!($room_list.state(), $pre_state, "pre state"); + )? + + let next = $room_list_sync_stream.next().await; + + assert_matches!(next, $sync_result, "sync's result"); for request in $server.received_requests().await.expect("Request recording has been disabled").iter().rev() { if SlidingSyncMatcher.matches(request) { @@ -74,7 +100,7 @@ macro_rules! sync_then_assert_request_and_fake_response { } } - $( assert_eq!(State:: $post_state, $room_list.state(), "post state"); )? + $( assert_matches!($room_list.state(), $post_state, "post state"); )? next } @@ -205,7 +231,7 @@ async fn test_sync_from_init_to_enjoy() -> Result<(), Error> { sync_then_assert_request_and_fake_response! { [server, room_list, sync] - states = Init -> FirstRooms, + states = Init => FirstRooms, assert request = { "lists": { ALL_ROOMS: { @@ -247,7 +273,7 @@ async fn test_sync_from_init_to_enjoy() -> Result<(), Error> { sync_then_assert_request_and_fake_response! { [server, room_list, sync] - states = FirstRooms -> AllRooms, + states = FirstRooms => AllRooms, assert request = { "lists": { ALL_ROOMS: { @@ -288,7 +314,7 @@ async fn test_sync_from_init_to_enjoy() -> Result<(), Error> { sync_then_assert_request_and_fake_response! { [server, room_list, sync] - states = AllRooms -> Enjoy, + states = AllRooms => Enjoy, assert request = { "lists": { ALL_ROOMS: { @@ -321,7 +347,7 @@ async fn test_sync_from_init_to_enjoy() -> Result<(), Error> { sync_then_assert_request_and_fake_response! { [server, room_list, sync] - states = Enjoy -> Enjoy, + states = Enjoy => Enjoy, assert request = { "lists": { ALL_ROOMS: { @@ -366,7 +392,7 @@ async fn test_sync_resumes_from_previous_state() -> Result<(), Error> { sync_then_assert_request_and_fake_response! { [server, room_list, sync] - states = Init -> FirstRooms, + states = Init => FirstRooms, assert request = { "lists": { ALL_ROOMS: { @@ -394,7 +420,7 @@ async fn test_sync_resumes_from_previous_state() -> Result<(), Error> { sync_then_assert_request_and_fake_response! { [server, room_list, sync] - states = FirstRooms -> AllRooms, + states = FirstRooms => AllRooms, assert request = { "lists": { ALL_ROOMS: { @@ -402,8 +428,8 @@ async fn test_sync_resumes_from_previous_state() -> Result<(), Error> { }, VISIBLE_ROOMS: { "ranges": [], - } - } + }, + }, }, respond with = { "pos": "1", @@ -429,7 +455,7 @@ async fn test_sync_resumes_from_previous_state() -> Result<(), Error> { sync_then_assert_request_and_fake_response! { [server, room_list, sync] - states = AllRooms -> Enjoy, + states = AllRooms => Enjoy, assert request = { "lists": { ALL_ROOMS: { @@ -460,6 +486,273 @@ async fn test_sync_resumes_from_previous_state() -> Result<(), Error> { Ok(()) } +#[async_test] +async fn test_sync_resumes_from_terminated() -> Result<(), Error> { + let (server, room_list) = new_room_list().await?; + + let sync = room_list.sync(); + pin_mut!(sync); + + // Simulate an error from the `Init` state. + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + sync matches Some(Err(_)), + states = Init => Terminated { .. }, + assert request = { + "lists": { + ALL_ROOMS: { + // The default range, in selective sync-mode. + "ranges": [[0, 19]], + }, + }, + }, + respond with = (code 400) { + "error": "foo", + "errcode": "M_UNKNOWN", + }, + }; + + // Ensure sync is terminated. + assert!(sync.next().await.is_none()); + + // Start a new sync. + let sync = room_list.sync(); + pin_mut!(sync); + + // Do a regular sync from the `Terminated` state. + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + states = Terminated { .. } => FirstRooms, + assert request = { + "lists": { + ALL_ROOMS: { + // Still the default range, in selective sync-mode. + "ranges": [[0, 19]], + }, + }, + }, + respond with = { + "pos": "1", + "lists": { + ALL_ROOMS: { + "count": 110, + }, + }, + "rooms": {}, + }, + }; + + // Simulate an error from the `FirstRooms` state. + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + sync matches Some(Err(_)), + states = FirstRooms => Terminated { .. }, + assert request = { + "lists": { + ALL_ROOMS: { + // In `FirstRooms`, the sync-mode has changed to growing, with + // its initial range. + "ranges": [[0, 49]], + }, + VISIBLE_ROOMS: { + // Hello new list. + "ranges": [], + }, + }, + }, + respond with = (code 400) { + "error": "foo", + "errcode": "M_UNKNOWN", + }, + }; + + // Ensure sync is terminated. + assert!(sync.next().await.is_none()); + + // Start a new sync. + let sync = room_list.sync(); + pin_mut!(sync); + + // Update the viewport, just to be sure it's not reset later. + room_list.apply_input(Input::Viewport(vec![5..=10])).await?; + + // Do a regular sync from the `Terminated` state. + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + states = Terminated { .. } => AllRooms, + assert request = { + "lists": { + ALL_ROOMS: { + // In `AllRooms`, the sync-mode is still growing, but the range + // hasn't been modified due to previous error. + "ranges": [[0, 49]], + }, + VISIBLE_ROOMS: { + // We have set a viewport, which reflects here. + "ranges": [[5, 10]], + }, + }, + }, + respond with = { + "pos": "2", + "lists": { + ALL_ROOMS: { + "count": 110, + }, + }, + "rooms": {}, + }, + }; + + // Simulate an error from the `AllRooms` state. + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + sync matches Some(Err(_)), + states = AllRooms => Terminated { .. }, + assert request = { + "lists": { + ALL_ROOMS: { + // In `AllRooms`, the sync-mode is still growing, and the range + // has made progress. + "ranges": [[0, 99]], + }, + VISIBLE_ROOMS: { + // Despites the error, the range is kept. + "ranges": [[5, 10]], + }, + }, + }, + respond with = (code 400) { + "error": "foo", + "errcode": "M_UNKNOWN", + }, + }; + + // Ensure sync is terminated. + assert!(sync.next().await.is_none()); + + // Start a new sync. + let sync = room_list.sync(); + pin_mut!(sync); + + // Do a regular sync from the `Terminated` state. + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + states = Terminated { .. } => Enjoy, + assert request = { + "lists": { + ALL_ROOMS: { + // Due to the error, the range is reset to its initial value. + "ranges": [[0, 49]], + }, + VISIBLE_ROOMS: { + // Despites the error, the range is kept. + "ranges": [[5, 10]], + }, + }, + }, + respond with = { + "pos": "3", + "lists": { + ALL_ROOMS: { + "count": 110, + }, + }, + "rooms": {}, + }, + }; + + // Do a regular sync from the `Enjoy` state to update the `ALL_ROOMS` list + // again. + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + states = Enjoy => Enjoy, + assert request = { + "lists": { + ALL_ROOMS: { + // No error. The range is making progress. + "ranges": [[0, 99]], + }, + VISIBLE_ROOMS: { + // No error. The range is still here. + "ranges": [[5, 10]], + }, + }, + }, + respond with = { + "pos": "4", + "lists": { + ALL_ROOMS: { + "count": 110, + }, + }, + "rooms": {}, + }, + }; + + // Simulate an error from the `Enjoy` state. + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + sync matches Some(Err(_)), + states = Enjoy => Terminated { .. }, + assert request = { + "lists": { + ALL_ROOMS: { + // Range is making progress and is even reaching the maximum + // number of rooms. + "ranges": [[0, 109]], + }, + VISIBLE_ROOMS: { + // The range is still here. + "ranges": [[5, 10]], + }, + }, + }, + respond with = (code 400) { + "error": "foo", + "errcode": "M_UNKNOWN", + }, + }; + + // Ensure sync is terminated. + assert!(sync.next().await.is_none()); + + // Start a new sync. + let sync = room_list.sync(); + pin_mut!(sync); + + // Do a regular sync from the `Terminated` state. + sync_then_assert_request_and_fake_response! { + [server, room_list, sync] + states = Terminated { .. } => Enjoy, + assert request = { + "lists": { + ALL_ROOMS: { + // An error was received at the previous sync iteration. + // The list is still in growing sync-mode, but its range has + // been reset. + "ranges": [[0, 49]], + }, + VISIBLE_ROOMS: { + // The range is still here. + "ranges": [[5, 10]], + }, + }, + }, + respond with = { + "pos": "5", + "lists": { + ALL_ROOMS: { + "count": 110, + }, + }, + "rooms": {}, + }, + }; + + Ok(()) +} + #[async_test] async fn test_entries_stream() -> Result<(), Error> { let (server, room_list) = new_room_list().await?; @@ -472,7 +765,7 @@ async fn test_entries_stream() -> Result<(), Error> { sync_then_assert_request_and_fake_response! { [server, room_list, sync] - states = Init -> FirstRooms, + states = Init => FirstRooms, assert request = { "lists": { ALL_ROOMS: { @@ -530,7 +823,7 @@ async fn test_entries_stream() -> Result<(), Error> { sync_then_assert_request_and_fake_response! { [server, room_list, sync] - states = FirstRooms -> AllRooms, + states = FirstRooms => AllRooms, assert request = { "lists": { ALL_ROOMS: { @@ -604,7 +897,7 @@ async fn test_entries_stream_with_updated_filter() -> Result<(), Error> { sync_then_assert_request_and_fake_response! { [server, room_list, sync] - states = Init -> FirstRooms, + states = Init => FirstRooms, assert request = { "lists": { ALL_ROOMS: { @@ -658,7 +951,7 @@ async fn test_entries_stream_with_updated_filter() -> Result<(), Error> { sync_then_assert_request_and_fake_response! { [server, room_list, sync] - states = FirstRooms -> AllRooms, + states = FirstRooms => AllRooms, assert request = { "lists": { ALL_ROOMS: { @@ -1022,7 +1315,7 @@ async fn test_input_viewport() -> Result<(), Error> { sync_then_assert_request_and_fake_response! { [server, room_list, sync] - states = Init -> FirstRooms, + states = Init => FirstRooms, assert request = { "lists": { ALL_ROOMS: { @@ -1039,7 +1332,7 @@ async fn test_input_viewport() -> Result<(), Error> { sync_then_assert_request_and_fake_response! { [server, room_list, sync] - states = FirstRooms -> AllRooms, + states = FirstRooms => AllRooms, assert request = { "lists": { ALL_ROOMS: { @@ -1062,7 +1355,7 @@ async fn test_input_viewport() -> Result<(), Error> { sync_then_assert_request_and_fake_response! { [server, room_list, sync] - states = AllRooms -> Enjoy, + states = AllRooms => Enjoy, assert request = { "lists": { ALL_ROOMS: { From 4a62949b6c1a57cfffd4db493b613f3b706fead5 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 5 Jun 2023 15:19:18 +0200 Subject: [PATCH 43/48] test(sdk): Simplify a test. --- crates/matrix-sdk/src/sliding_sync/mod.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/crates/matrix-sdk/src/sliding_sync/mod.rs b/crates/matrix-sdk/src/sliding_sync/mod.rs index ee63719c3..d6c04c3b3 100644 --- a/crates/matrix-sdk/src/sliding_sync/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/mod.rs @@ -901,26 +901,20 @@ mod tests { pin_mut!(stream); // The sync-loop is actually running. - for _ in 0..3 { - assert!(stream.next().await.is_some()); - } + assert!(stream.next().await.is_some()); // Stop the sync-loop. sliding_sync.stop_sync()?; // The sync-loop is actually stopped. - for _ in 0..3 { - assert!(stream.next().await.is_none()); - } + assert!(stream.next().await.is_none()); // Start a new sync-loop. let stream = sliding_sync.sync(); pin_mut!(stream); // The sync-loop is actually running. - for _ in 0..3 { - assert!(stream.next().await.is_some()); - } + assert!(stream.next().await.is_some()); Ok(()) } From ac7a576035e7b742638b2dd730ddbc77bb8568ba Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 5 Jun 2023 19:13:48 +0200 Subject: [PATCH 44/48] chore(ui): Address feedbacks. --- crates/matrix-sdk-ui/src/room_list/mod.rs | 11 ++++------- crates/matrix-sdk/src/sliding_sync/list/mod.rs | 4 ++++ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 06c9a3f9c..ceeb05d12 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -36,7 +36,7 @@ //! `RoomList` works with 2 Sliding Sync List: //! //! * `all_rooms` (refered by the constant [`ALL_ROOMS_LIST_NAME`]) is the main -//! list. It's goal is to load all the user' rooms. It starts with a +//! list. Its goal is to load all the user' rooms. It starts with a //! [`SlidingSyncMode::Selective`] sync-mode with a small range (i.e. a small //! set of rooms) to load the first rooms quickly, and then updates to a //! [`SlidingSyncMode::Growing`] sync-mode to load the remaining rooms “in the @@ -173,7 +173,7 @@ impl RoomList { } } - /// Get the actual state of the state machine. + /// Get the current state of the state machine. pub fn state(&self) -> State { self.state.get() } @@ -348,10 +348,10 @@ pub enum State { /// That's the first initial state. Init, - /// At this state, the first rooms starts to be synced. + /// At this state, the first rooms start to be synced. FirstRooms, - /// At this state, all rooms starts to be synced. + /// At this state, all rooms start to be synced. AllRooms, /// This state is the cruising speed, i.e. the “normal” state, where nothing @@ -654,9 +654,6 @@ mod tests { let room_list = new_room_list().await?; let sliding_sync = room_list.sliding_sync(); - // List is absent. - assert_eq!(sliding_sync.on_list(VISIBLE_ROOMS_LIST_NAME, |_list| ready(())).await, None); - // List is present, in Selective mode. assert_eq!( sliding_sync diff --git a/crates/matrix-sdk/src/sliding_sync/list/mod.rs b/crates/matrix-sdk/src/sliding_sync/list/mod.rs index d5e89048c..9f9ab1a7b 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/mod.rs @@ -134,6 +134,10 @@ impl SlidingSyncList { /// /// There's no guarantee of ordering between items emitted by this stream /// and those emitted by other streams exposed on this structure. + /// + /// The first part of the returned tuple is the actual room list entries, + /// and the second part is the `Stream` to receive updates on the room list + /// entries. pub fn room_list_stream( &self, ) -> (Vector, impl Stream>) { From d1bccacef92357c31cd852093359f6adb6a4cbf3 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 5 Jun 2023 19:40:51 +0200 Subject: [PATCH 45/48] chore: Make CI happy. --- .github/workflows/ci.yml | 2 +- crates/matrix-sdk-ui/src/room_list/mod.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8346f1d74..ed0e45565 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -185,7 +185,7 @@ jobs: - name: Test run: | cargo nextest run --workspace \ - --exclude matrix-sdk-integration-testing --exclude sliding-sync-integration-test + --exclude matrix-sdk-integration-testing --exclude sliding-sync-integration-test --features testing - name: Test documentation run: | diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 4352d80f1..2b6682cd5 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -35,14 +35,14 @@ //! //! `RoomList` works with 2 Sliding Sync List: //! -//! * `all_rooms` (refered by the constant [`ALL_ROOMS_LIST_NAME`]) is the main +//! * `all_rooms` (referred by the constant [`ALL_ROOMS_LIST_NAME`]) is the main //! list. Its goal is to load all the user' rooms. It starts with a //! [`SlidingSyncMode::Selective`] sync-mode with a small range (i.e. a small //! set of rooms) to load the first rooms quickly, and then updates to a //! [`SlidingSyncMode::Growing`] sync-mode to load the remaining rooms “in the //! background”: it will sync the existing rooms and will fetch new rooms, by //! a certain batch size. -//! * `visible_rooms` (refered by the constant [`VISIBLE_ROOMS_LIST_NAME`]) is +//! * `visible_rooms` (referred by the constant [`VISIBLE_ROOMS_LIST_NAME`]) is //! the “reactive” list. It's goal is to react to the client app user actions. //! If the user scrolls in the room list, the `visible_rooms` will be //! configured to sync for the particular range of rooms the user is actually From 42df0d0e21fa1902f4d1e817a2fd95e4ba3bc7f7 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 5 Jun 2023 19:43:32 +0200 Subject: [PATCH 46/48] fix: Fix merge commit. --- crates/matrix-sdk/src/sliding_sync/cache.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/matrix-sdk/src/sliding_sync/cache.rs b/crates/matrix-sdk/src/sliding_sync/cache.rs index 6c9ec9482..59564bb4c 100644 --- a/crates/matrix-sdk/src/sliding_sync/cache.rs +++ b/crates/matrix-sdk/src/sliding_sync/cache.rs @@ -272,7 +272,7 @@ mod tests { // Modify both lists, so we can check expected caching behavior later. { - let lists = sliding_sync.inner.lists.write().unwrap(); + let lists = sliding_sync.inner.lists.write().await; let list_foo = lists.get("list_foo").unwrap(); list_foo.set_maximum_number_of_rooms(Some(42)); @@ -330,7 +330,7 @@ mod tests { // Check the list' state. { - let lists = sliding_sync.inner.lists.write().unwrap(); + let lists = sliding_sync.inner.lists.write().await; // This one was cached. let list_foo = lists.get("list_foo").unwrap(); @@ -352,7 +352,8 @@ mod tests { } // Clean the cache. - clean_storage(&client, &storage_key, &sliding_sync.inner.lists.read().unwrap()).await; + let lists = sliding_sync.inner.lists.read().await; + clean_storage(&client, &storage_key, &lists).await; storage_key }; From e6fdcfdf524619f23e1e23637e61d8a32d9e08a5 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 5 Jun 2023 20:05:46 +0200 Subject: [PATCH 47/48] chore: Make Clippy happy. --- crates/matrix-sdk-ui/src/room_list/mod.rs | 28 ++++++++++++----------- xtask/src/ci.rs | 5 ++-- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 2b6682cd5..fadf3b986 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -227,9 +227,9 @@ impl RoomList { async fn update_viewport(&self, ranges: Ranges) -> Result<(), Error> { self.sliding_sync .on_list(VISIBLE_ROOMS_LIST_NAME, |list| { - ready( - list.set_sync_mode(SlidingSyncMode::new_selective().add_ranges(ranges.clone())), - ) + list.set_sync_mode(SlidingSyncMode::new_selective().add_ranges(ranges.clone())); + + ready(()) }) .await .ok_or_else(|| Error::InputHasNotBeenApplied(Input::Viewport(ranges)))?; @@ -438,7 +438,9 @@ impl Action for SetAllRoomsListToGrowingSyncMode { async fn run(&self, sliding_sync: &SlidingSync) -> Result<(), Error> { sliding_sync .on_list(ALL_ROOMS_LIST_NAME, |list| { - ready(list.set_sync_mode(SlidingSyncMode::new_growing(50))) + list.set_sync_mode(SlidingSyncMode::new_growing(50)); + + ready(()) }) .await .ok_or_else(|| Error::UnknownList(ALL_ROOMS_LIST_NAME.to_string()))?; @@ -575,51 +577,51 @@ mod tests { // Hypothetical termination. { let state = - State::Terminated { from: Box::new(state.clone()) }.next(&sliding_sync).await?; + State::Terminated { from: Box::new(state.clone()) }.next(sliding_sync).await?; assert_eq!(state, State::Init); } // Next state. - let state = state.next(&sliding_sync).await?; + let state = state.next(sliding_sync).await?; assert_eq!(state, State::FirstRooms); // Hypothetical termination. { let state = - State::Terminated { from: Box::new(state.clone()) }.next(&sliding_sync).await?; + State::Terminated { from: Box::new(state.clone()) }.next(sliding_sync).await?; assert_eq!(state, State::FirstRooms); } // Next state. - let state = state.next(&sliding_sync).await?; + let state = state.next(sliding_sync).await?; assert_eq!(state, State::AllRooms); // Hypothetical termination. { let state = - State::Terminated { from: Box::new(state.clone()) }.next(&sliding_sync).await?; + State::Terminated { from: Box::new(state.clone()) }.next(sliding_sync).await?; assert_eq!(state, State::AllRooms); } // Next state. - let state = state.next(&sliding_sync).await?; + let state = state.next(sliding_sync).await?; assert_eq!(state, State::Enjoy); // Hypothetical termination. { let state = - State::Terminated { from: Box::new(state.clone()) }.next(&sliding_sync).await?; + State::Terminated { from: Box::new(state.clone()) }.next(sliding_sync).await?; assert_eq!(state, State::Enjoy); } // Next state. - let state = state.next(&sliding_sync).await?; + let state = state.next(sliding_sync).await?; assert_eq!(state, State::Enjoy); // Hypothetical termination. { let state = - State::Terminated { from: Box::new(state.clone()) }.next(&sliding_sync).await?; + State::Terminated { from: Box::new(state.clone()) }.next(sliding_sync).await?; assert_eq!(state, State::Enjoy); } diff --git a/xtask/src/ci.rs b/xtask/src/ci.rs index 2551cfba2..15ba3b4b4 100644 --- a/xtask/src/ci.rs +++ b/xtask/src/ci.rs @@ -169,12 +169,13 @@ fn check_typos() -> Result<()> { } fn check_clippy() -> Result<()> { - cmd!("rustup run {NIGHTLY} cargo clippy --all-targets -- -D warnings").run()?; + cmd!("rustup run {NIGHTLY} cargo clippy --all-targets --features testing -- -D warnings") + .run()?; cmd!( "rustup run {NIGHTLY} cargo clippy --workspace --all-targets --exclude matrix-sdk-crypto --exclude xtask --no-default-features - --features native-tls,experimental-sliding-sync,sso-login + --features native-tls,experimental-sliding-sync,sso-login,testing -- -D warnings" ) .run()?; From 16932abeaa43c1c5160c06aa08d8e509f6368a54 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 5 Jun 2023 20:17:19 +0200 Subject: [PATCH 48/48] chore: Make Tarpaulin happy. --- .github/workflows/coverage.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 33abd9d9e..427fc4ad7 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -52,7 +52,7 @@ jobs: - name: Run tarpaulin run: | - cargo tarpaulin --out Xml -e sliding-sync-integration-test + cargo tarpaulin --out Xml -e sliding-sync-integration-test --features testing - name: Upload to codecov.io uses: codecov/codecov-action@v3