diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 8fe36d50a..4977d1e88 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -87,15 +87,27 @@ use ruma::{ }; pub use state::*; use thiserror::Error; -use tokio::sync::RwLock; +use tokio::sync::{Mutex, RwLock}; /// The [`RoomListService`] type. See the module's documentation to learn more. #[derive(Debug)] pub struct RoomListService { + /// The Sliding Sync instance. sliding_sync: Arc, + + /// The current state of the `RoomListService`. + /// + /// `RoomListService` is a simple state-machine. + state: Observable, + /// Room cache, to avoid recreating `Room`s everytime users fetch them. rooms: Arc>>, - state: Observable, + + /// The current viewport ranges. + /// + /// This is useful to avoid resetting the ranges to the same value, + /// which would cancel the current in-flight sync request. + viewport_ranges: Mutex, } impl RoomListService { @@ -158,7 +170,12 @@ impl RoomListService { .map(Arc::new) .map_err(Error::SlidingSync)?; - Ok(Self { sliding_sync, state: Observable::new(State::Init), rooms: Default::default() }) + Ok(Self { + sliding_sync, + state: Observable::new(State::Init), + rooms: Default::default(), + viewport_ranges: Mutex::new(vec![state::VISIBLE_ROOMS_DEFAULT_RANGE]), + }) } /// Start to sync the room list. @@ -258,19 +275,23 @@ impl RoomListService { } /// Pass an [`Input`] onto the state machine. - pub async fn apply_input(&self, input: Input) -> Result<(), Error> { + pub async fn apply_input(&self, input: Input) -> Result { use Input::*; match input { - Viewport(ranges) => { - self.update_viewport(ranges).await?; - } + Viewport(ranges) => self.update_viewport(ranges).await, } - - Ok(()) } - async fn update_viewport(&self, ranges: Ranges) -> Result<(), Error> { + async fn update_viewport(&self, ranges: Ranges) -> Result { + let mut viewport_ranges = self.viewport_ranges.lock().await; + + // Is it worth updating the viewport? + // The viewport has the same ranges. Don't update it. + if *viewport_ranges == ranges { + return Ok(InputResult::Ignored); + } + self.sliding_sync .on_list(VISIBLE_ROOMS_LIST_NAME, |list| { list.set_sync_mode(SlidingSyncMode::new_selective().add_ranges(ranges.clone())); @@ -278,9 +299,11 @@ impl RoomListService { ready(()) }) .await - .ok_or_else(|| Error::InputHasNotBeenApplied(Input::Viewport(ranges)))?; + .ok_or_else(|| Error::InputCannotBeApplied(Input::Viewport(ranges.clone())))?; - Ok(()) + *viewport_ranges = ranges; + + Ok(InputResult::Applied) } /// Get a [`Room`] if it exists. @@ -320,8 +343,8 @@ pub enum Error { UnknownList(String), /// An input was asked to be applied but it wasn't possible to apply it. - #[error("The input has been not applied")] - InputHasNotBeenApplied(Input), + #[error("The input cannot be applied")] + InputCannotBeApplied(Input), /// The requested room doesn't exist. #[error("Room `{0}` not found")] @@ -342,6 +365,18 @@ pub enum Input { Viewport(Ranges), } +/// An [`Input`] Ok result: whether it's been applied, or ignored. +#[derive(Debug, Eq, PartialEq)] +pub enum InputResult { + /// The input has been applied. + Applied, + + /// The input has been ignored. + /// + /// Note that this is not an error. The input was valid, but simply ignored. + Ignored, +} + #[cfg(test)] mod tests { use matrix_sdk::{ diff --git a/crates/matrix-sdk-ui/tests/integration/room_list.rs b/crates/matrix-sdk-ui/tests/integration/room_list.rs index 4da17629a..17bf3e76f 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list.rs @@ -7,8 +7,9 @@ use imbl::vector; use matrix_sdk_test::async_test; use matrix_sdk_ui::{ room_list::{ - Error, Input, RoomListEntry, RoomListLoadingState, State, ALL_ROOMS_LIST_NAME as ALL_ROOMS, - INVITES_LIST_NAME as INVITES, VISIBLE_ROOMS_LIST_NAME as VISIBLE_ROOMS, + Error, Input, InputResult, RoomListEntry, RoomListLoadingState, State, + ALL_ROOMS_LIST_NAME as ALL_ROOMS, INVITES_LIST_NAME as INVITES, + VISIBLE_ROOMS_LIST_NAME as VISIBLE_ROOMS, }, timeline::{TimelineItem, VirtualTimelineItem}, RoomListService, @@ -658,7 +659,7 @@ async fn test_sync_resumes_from_error() -> Result<(), Error> { 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?; + assert_eq!(room_list.apply_input(Input::Viewport(vec![5..=10])).await?, InputResult::Applied); // Do a regular sync from the `Error` state. sync_then_assert_request_and_fake_response! { @@ -950,7 +951,7 @@ async fn test_sync_resumes_from_terminated() -> Result<(), Error> { 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?; + assert_eq!(room_list.apply_input(Input::Viewport(vec![5..=10])).await?, InputResult::Applied); // Do a regular sync from the `Terminated` state. sync_then_assert_request_and_fake_response! { @@ -2141,7 +2142,7 @@ async fn test_input_viewport() -> Result<(), Error> { // present. assert_matches!( room_list.apply_input(Input::Viewport(vec![10..=15])).await, - Err(Error::InputHasNotBeenApplied(_)) + Err(Error::InputCannotBeApplied(_)) ); sync_then_assert_request_and_fake_response! { @@ -2185,7 +2186,17 @@ async fn test_input_viewport() -> Result<(), Error> { }, }; - assert!(room_list.apply_input(Input::Viewport(vec![10..=15, 20..=25])).await.is_ok()); + // Now we can change the viewport.. + assert_eq!( + room_list.apply_input(Input::Viewport(vec![10..=15, 20..=25])).await?, + InputResult::Applied + ); + + // Re-changing the viewport has no effect. + assert_eq!( + room_list.apply_input(Input::Viewport(vec![10..=15, 20..=25])).await?, + InputResult::Ignored + ); // The `timeline_limit` is not repeated because it's sticky. sync_then_assert_request_and_fake_response! {