feat(ui): Implement RoomListService::sync_indicator

feat(ui): Implement `RoomListService::sync_indicator`
This commit is contained in:
Ivan Enderlin
2023-09-06 17:56:11 +02:00
committed by GitHub
4 changed files with 339 additions and 15 deletions

View File

@@ -115,6 +115,21 @@ impl RoomListService {
async fn apply_input(&self, input: RoomListInput) -> Result<(), RoomListError> {
self.inner.apply_input(input.into()).await.map(|_| ()).map_err(Into::into)
}
fn sync_indicator(
&self,
listener: Box<dyn RoomListServiceSyncIndicatorListener>,
) -> Arc<TaskHandle> {
let sync_indicator_stream = self.inner.sync_indicator();
Arc::new(TaskHandle::new(RUNTIME.spawn(async move {
pin_mut!(sync_indicator_stream);
while let Some(sync_indicator) = sync_indicator_stream.next().await {
listener.on_update(sync_indicator.into());
}
})))
}
}
#[derive(uniffi::Object)]
@@ -216,15 +231,32 @@ pub enum RoomListServiceState {
impl From<matrix_sdk_ui::room_list_service::State> for RoomListServiceState {
fn from(value: matrix_sdk_ui::room_list_service::State) -> Self {
use matrix_sdk_ui::room_list_service::State::*;
use matrix_sdk_ui::room_list_service::State as S;
match value {
Init => Self::Initial,
SettingUp => Self::SettingUp,
Recovering => Self::Recovering,
Running => Self::Running,
Error { .. } => Self::Error,
Terminated { .. } => Self::Terminated,
S::Init => Self::Initial,
S::SettingUp => Self::SettingUp,
S::Recovering => Self::Recovering,
S::Running => Self::Running,
S::Error { .. } => Self::Error,
S::Terminated { .. } => Self::Terminated,
}
}
}
#[derive(uniffi::Enum)]
pub enum RoomListServiceSyncIndicator {
Show,
Hide,
}
impl From<matrix_sdk_ui::room_list_service::SyncIndicator> for RoomListServiceSyncIndicator {
fn from(value: matrix_sdk_ui::room_list_service::SyncIndicator) -> Self {
use matrix_sdk_ui::room_list_service::SyncIndicator as SI;
match value {
SI::Show => Self::Show,
SI::Hide => Self::Hide,
}
}
}
@@ -237,11 +269,11 @@ pub enum RoomListLoadingState {
impl From<matrix_sdk_ui::room_list_service::RoomListLoadingState> for RoomListLoadingState {
fn from(value: matrix_sdk_ui::room_list_service::RoomListLoadingState) -> Self {
use matrix_sdk_ui::room_list_service::RoomListLoadingState::*;
use matrix_sdk_ui::room_list_service::RoomListLoadingState as LS;
match value {
NotLoaded => Self::NotLoaded,
Loaded { maximum_number_of_rooms } => Self::Loaded { maximum_number_of_rooms },
LS::NotLoaded => Self::NotLoaded,
LS::Loaded { maximum_number_of_rooms } => Self::Loaded { maximum_number_of_rooms },
}
}
}
@@ -256,6 +288,11 @@ pub trait RoomListLoadingStateListener: Send + Sync + Debug {
fn on_update(&self, state: RoomListLoadingState);
}
#[uniffi::export(callback_interface)]
pub trait RoomListServiceSyncIndicatorListener: Send + Sync + Debug {
fn on_update(&self, sync_indicator: RoomListServiceSyncIndicator);
}
#[derive(uniffi::Enum)]
pub enum RoomListEntriesUpdate {
Append { values: Vec<RoomListEntry> },

View File

@@ -67,7 +67,7 @@ mod room;
mod room_list;
mod state;
use std::{future::ready, sync::Arc};
use std::{future::ready, sync::Arc, time::Duration};
use async_stream::stream;
use eyeball::{SharedObservable, Subscriber};
@@ -91,7 +91,22 @@ use ruma::{
};
pub use state::*;
use thiserror::Error;
use tokio::sync::{Mutex, RwLock};
use tokio::{
sync::{Mutex, RwLock},
time::timeout,
};
/// Delay time before actually sending a [`SyncIndicator::Show`].
///
/// It's not because a `SyncIndicator` should be shown that it must be done
/// immediately. In case of a normal network conditions, without any delay, it
/// can lead to a “blinking” visual effect. This constant configures how long it
/// takes to consider that a request is “slow”, and that the `SyncIndicator` is
/// necessary to be shown.
pub const SYNC_INDICATOR_DELAY_BEFORE_SHOWING: Duration = Duration::from_millis(200);
/// Delay time before actually sending a [`SyncIndicator::Hide`].
pub const SYNC_INDICATOR_DELAY_BEFORE_HIDING: Duration = Duration::from_millis(0);
/// The [`RoomListService`] type. See the module's documentation to learn more.
#[derive(Debug)]
@@ -323,6 +338,59 @@ impl RoomListService {
}
}
/// Get a [`Stream`] of [`SyncIndicator`].
///
/// Read the documentation of [`SyncIndicator`] to learn more about it.
pub fn sync_indicator(&self) -> impl Stream<Item = SyncIndicator> {
let mut state = self.state();
stream! {
// Ensure the `SyncIndicator` is always hidden to start with.
yield SyncIndicator::Hide;
// Let's not wait for an update to happen. The `SyncIndicator` must be
// computed as fast as possible.
let mut current_state = state.next_now();
loop {
let (sync_indicator, yield_delay) = match current_state {
State::Init | State::Recovering | State::Error { .. } => {
(SyncIndicator::Show, SYNC_INDICATOR_DELAY_BEFORE_SHOWING)
}
State::SettingUp | State::Running | State::Terminated { .. } => {
(SyncIndicator::Hide, SYNC_INDICATOR_DELAY_BEFORE_HIDING)
}
};
// `state.next().await` has a maximum of `yield_delay` time to execute…
let next_state = match timeout(yield_delay, state.next()).await {
// A new state has been received before `yield_delay` time. The new
// `sync_indicator` value won't be yielded.
Ok(next_state) => next_state,
// No new state has been received before `yield_delay` time. The
// `sync_indicator` value can be yielded.
Err(_) => {
yield sync_indicator;
// Now that `sync_indicator` has been yielded, let's wait on
// the next state again.
state.next().await
}
};
if let Some(next_state) = next_state {
// Update the `current_state`.
current_state = next_state;
} else {
// Something is broken with `self.state`. Let's stop this stream too.
break;
}
}
}
}
/// Get the [`Client`] that has been used to create [`Self`].
pub fn client(&self) -> &Client {
&self.client
@@ -474,6 +542,28 @@ pub enum InputResult {
Ignored,
}
/// An hint whether a _sync spinner/loader/toaster_ should be prompted to the
/// user, indicating that the [`RoomListService`] is syncing.
///
/// This is entirely arbitrary and optinionated. Of course, once
/// [`RoomListService::sync`] has been called, it's going to be constantly
/// syncing, until [`RoomListService::stop_sync`] is called, or until an error
/// happened. But in some cases, it's better for the user experience to prompt
/// to the user that a sync is happening. It's usually the first sync, or the
/// recovering sync. However, the sync indicator must be prompted if the
/// aforementioned sync is “slow”, otherwise the indicator is likely to “blink”
/// pretty fast, which can be very confusing. It's also common to indicate to
/// the user that a syncing is happening in case of a network error, that
/// something is catching up etc.
#[derive(Debug, Eq, PartialEq)]
pub enum SyncIndicator {
/// Show the sync indicator.
Show,
/// Hide the sync indicator.
Hide,
}
#[cfg(test)]
mod tests {
use std::future::ready;

View File

@@ -1,4 +1,7 @@
use std::ops::Not;
use std::{
ops::Not,
time::{Duration, Instant},
};
use assert_matches::assert_matches;
use eyeball_im::VectorDiff;
@@ -9,8 +12,9 @@ use matrix_sdk_test::async_test;
use matrix_sdk_ui::{
room_list_service::{
filters::{new_filter_all, new_filter_fuzzy_match_room_name},
Error, Input, InputResult, RoomListEntry, RoomListLoadingState, State,
Error, Input, InputResult, RoomListEntry, RoomListLoadingState, State, SyncIndicator,
ALL_ROOMS_LIST_NAME as ALL_ROOMS, INVITES_LIST_NAME as INVITES,
SYNC_INDICATOR_DELAY_BEFORE_HIDING, SYNC_INDICATOR_DELAY_BEFORE_SHOWING,
VISIBLE_ROOMS_LIST_NAME as VISIBLE_ROOMS,
},
timeline::{TimelineItemKind, VirtualTimelineItem},
@@ -24,7 +28,7 @@ use ruma::{
};
use serde_json::json;
use stream_assert::{assert_next_matches, assert_pending};
use tokio::task::yield_now;
use tokio::{spawn, sync::mpsc::channel, task::yield_now};
use wiremock::MockServer;
use crate::{
@@ -47,6 +51,7 @@ macro_rules! sync_then_assert_request_and_fake_response {
$( states = $pre_state:pat => $post_state:pat, )?
assert request $assert_request:tt { $( $request_json:tt )* },
respond with = $( ( code $code:expr ) )? { $( $response_json:tt )* }
$( , after delay = $response_delay:expr )?
$(,)?
) => {
sync_then_assert_request_and_fake_response! {
@@ -55,6 +60,7 @@ macro_rules! sync_then_assert_request_and_fake_response {
$( states = $pre_state => $post_state, )?
assert request $assert_request { $( $request_json )* },
respond with = $( ( code $code ) )? { $( $response_json )* },
$( after delay = $response_delay, )?
}
};
@@ -64,6 +70,7 @@ macro_rules! sync_then_assert_request_and_fake_response {
$( states = $pre_state:pat => $post_state:pat, )?
assert request $assert_request:tt { $( $request_json:tt )* },
respond with = $( ( code $code:expr ) )? { $( $response_json:tt )* }
$( , after delay = $response_delay:expr )?
$(,)?
) => {
{
@@ -80,6 +87,7 @@ macro_rules! sync_then_assert_request_and_fake_response {
sync matches $sync_result,
assert request $assert_request { $( $request_json )* },
respond with = $( ( code $code ) )? { $( $response_json )* },
$( after delay = $response_delay, )?
};
$( assert_matches!(state.next().now_or_never(), Some(Some($post_state)), "post state"); )?
@@ -2539,3 +2547,188 @@ async fn test_input_viewport() -> Result<(), Error> {
Ok(())
}
#[async_test]
async fn test_sync_indicator() -> Result<(), Error> {
let (_, server, room_list) = new_room_list_service().await?;
let sync = room_list.sync();
pin_mut!(sync);
let sync_indicator = room_list.sync_indicator();
let request_margin = Duration::from_millis(20);
let request_1_delay = SYNC_INDICATOR_DELAY_BEFORE_SHOWING * 2;
let request_2_delay = SYNC_INDICATOR_DELAY_BEFORE_SHOWING * 3;
let request_4_delay = SYNC_INDICATOR_DELAY_BEFORE_SHOWING * 2;
let request_5_delay = SYNC_INDICATOR_DELAY_BEFORE_SHOWING * 2;
let (in_between_requests_synchronizer_sender, mut in_between_requests_synchronizer) =
channel(1);
macro_rules! assert_next_sync_indicator {
($sync_indicator:ident, $pattern:pat, now $(,)?) => {
assert_matches!($sync_indicator.next().now_or_never(), Some(Some($pattern)));
};
($sync_indicator:ident, $pattern:pat, under $time:expr $(,)?) => {
let now = Instant::now();
assert_matches!($sync_indicator.next().await, Some($pattern));
assert!(now.elapsed() < $time);
};
}
let sync_indicator_task = spawn(async move {
pin_mut!(sync_indicator);
// `SyncIndicator` is forced to be hidden to begin with.
assert_next_sync_indicator!(sync_indicator, SyncIndicator::Hide, now);
// Request 1.
{
// Sync has started, the `SyncIndicator` must be shown… but not immediately!
assert_next_sync_indicator!(
sync_indicator,
SyncIndicator::Show,
under SYNC_INDICATOR_DELAY_BEFORE_SHOWING + request_margin,
);
// Then, once the sync is done, the `SyncIndicator` must be hidden.
assert_next_sync_indicator!(
sync_indicator,
SyncIndicator::Hide,
under request_1_delay - SYNC_INDICATOR_DELAY_BEFORE_SHOWING
+ SYNC_INDICATOR_DELAY_BEFORE_HIDING
+ request_margin,
);
}
in_between_requests_synchronizer.recv().await.unwrap();
assert_pending!(sync_indicator);
// Request 2.
{
// Nothing happens, as the state transitions from `SettingUp` to
// `Running`, no `SyncIndicator` must be shown.
}
in_between_requests_synchronizer.recv().await.unwrap();
assert_pending!(sync_indicator);
// Request 3.
{
// Sync has errored, the `SyncIndicator` should be show. Fortunately
// for us (fictional situation), the sync is restarted
// immediately, and `SyncIndicator` doesn't have time to
// be shown for this particular state update.
}
in_between_requests_synchronizer.recv().await.unwrap();
assert_pending!(sync_indicator);
// Request 4.
{
// The system is recovering, It takes times (fictional situation)!
// `SyncIndicator` has time to show (off).
assert_next_sync_indicator!(
sync_indicator,
SyncIndicator::Show,
under SYNC_INDICATOR_DELAY_BEFORE_SHOWING + request_margin,
);
}
in_between_requests_synchronizer.recv().await.unwrap();
assert_pending!(sync_indicator);
// Request 5.
{
// It takes time for the system to recover…
assert_next_sync_indicator!(
sync_indicator,
SyncIndicator::Show,
under SYNC_INDICATOR_DELAY_BEFORE_SHOWING + request_margin,
);
// But finally, the system has recovered and is running. Time to hide the
// `SyncIndicator`.
assert_next_sync_indicator!(
sync_indicator,
SyncIndicator::Hide,
under request_5_delay - SYNC_INDICATOR_DELAY_BEFORE_SHOWING
+ SYNC_INDICATOR_DELAY_BEFORE_HIDING
+ request_margin,
);
}
});
// Request 1.
sync_then_assert_request_and_fake_response! {
[server, room_list, sync]
states = Init => SettingUp,
assert request >= {},
respond with = {
"pos": "0",
},
after delay = request_1_delay, // Slow request!
};
in_between_requests_synchronizer_sender.send(()).await.unwrap();
// Request 2.
sync_then_assert_request_and_fake_response! {
[server, room_list, sync]
states = SettingUp => Running,
assert request >= {},
respond with = {
"pos": "1",
},
after delay = request_2_delay, // Slow request!
};
in_between_requests_synchronizer_sender.send(()).await.unwrap();
// Request 3.
sync_then_assert_request_and_fake_response! {
[server, room_list, sync]
sync matches Some(Err(_)),
states = Running => Error { .. },
assert request >= {},
respond with = (code 400) {
"error": "foo",
"errcode": "M_UNKNOWN",
},
};
let sync = room_list.sync();
pin_mut!(sync);
in_between_requests_synchronizer_sender.send(()).await.unwrap();
// Request 4.
sync_then_assert_request_and_fake_response! {
[server, room_list, sync]
states = Error { .. } => Recovering,
assert request >= {},
respond with = {
"pos": "2",
},
after delay = request_4_delay, // Slow request!
};
in_between_requests_synchronizer_sender.send(()).await.unwrap();
// Request 5.
sync_then_assert_request_and_fake_response! {
[server, room_list, sync]
states = Recovering => Running,
assert request >= {},
respond with = {
"pos": "3",
},
after delay = request_5_delay, // Slow request!
};
sync_indicator_task.await.unwrap();
Ok(())
}

View File

@@ -60,6 +60,7 @@ macro_rules! sliding_sync_then_assert_request_and_fake_response {
[$server:ident, $stream:ident]
assert request $sign:tt { $( $request_json:tt )* },
respond with = $( ( code $code:expr ) )? { $( $response_json:tt )* }
$( , after delay = $response_delay:expr )?
$(,)?
) => {
sliding_sync_then_assert_request_and_fake_response! {
@@ -67,6 +68,7 @@ macro_rules! sliding_sync_then_assert_request_and_fake_response {
sync matches Some(Ok(_)),
assert request $sign { $( $request_json )* },
respond with = $( ( code $code ) )? { $( $response_json )* },
$( after delay = $response_delay, )?
}
};
@@ -75,6 +77,7 @@ macro_rules! sliding_sync_then_assert_request_and_fake_response {
sync matches $sync_result:pat,
assert request $sign:tt { $( $request_json:tt )* },
respond with = $( ( code $code:expr ) )? { $( $response_json:tt )* }
$( , after delay = $response_delay:expr )?
$(,)?
) => {
{
@@ -96,6 +99,7 @@ macro_rules! sliding_sync_then_assert_request_and_fake_response {
$( $response_json )*
})
)
$( .set_delay($response_delay) )?
})
.mount_as_scoped(&$server)
.await;