diff --git a/crates/matrix-sdk-ui/src/room_list_service/mod.rs b/crates/matrix-sdk-ui/src/room_list_service/mod.rs index bd6a046b53d..05935be8420 100644 --- a/crates/matrix-sdk-ui/src/room_list_service/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list_service/mod.rs @@ -220,28 +220,26 @@ impl RoomListService { // returned. If true, only invited rooms are returned. is_invite: None, }))) - .requires_timeout(move |request_generator| { + .requires_timeout(move |_request_generator| { // We want Sliding Sync to apply the poll + network timeout —i.e. to do the // long-polling— in some particular cases. Let's define them. match observable_state.get() { - // These are the states where we want an immediate response from the - // server, with no long-polling. + // Pre-`Running` states must respond immediately: the session is still + // being established, recovering, or has terminated. `SyncIndicator` + // also derives from the time spent outside `Running`, so long-polling + // here would falsely trigger the loading hint. State::Init | State::SettingUp | State::Recovering | State::Error { .. } | State::Terminated { .. } => PollTimeout::Some(0), - // Otherwise we want long-polling if the list is fully-loaded. - State::Running => { - if request_generator.is_fully_loaded() { - // Long-polling. - PollTimeout::Default - } else { - // No long-polling yet. - PollTimeout::Some(0) - } - } + // Once we are `Running`, let the server long-poll regardless of + // whether the list is fully loaded. If the next Growing batch is + // ready (or any update is queued) the server can still answer + // immediately; otherwise it holds the connection instead of forcing + // the client to spin on empty responses. + State::Running => PollTimeout::Default, } }), ) @@ -562,7 +560,13 @@ pub enum SyncIndicator { #[cfg(test)] mod tests { - use std::future::ready; + use std::{ + future::ready, + sync::{ + Arc, + atomic::{AtomicUsize, Ordering}, + }, + }; use futures_util::{StreamExt, pin_mut}; use matrix_sdk::{ @@ -677,4 +681,73 @@ mod tests { Ok(()) } + + #[async_test] + async fn test_long_poll_once_running() -> Result<(), Error> { + let (client, server) = new_client().await; + let room_list = RoomListService::new(client).await?; + + let sync = room_list.sync(); + pin_mut!(sync); + + let pos = Arc::new(AtomicUsize::new(0)); + let _mock_guard = Mock::given(SlidingSyncMatcher) + .respond_with(move |_request: &Request| { + let next_pos = pos.fetch_add(1, Ordering::SeqCst).to_string(); + ResponseTemplate::new(200).set_body_json(json!({ + "pos": next_pos, + "lists": { + ALL_ROOMS_LIST_NAME: { + "count": 0, + "ops": [], + }, + }, + "rooms": {}, + })) + }) + .mount_as_scoped(&server) + .await; + + // Drive the state machine: Init -> SettingUp -> Running -> Running. + let _ = sync.next().await; + assert_eq!(room_list.state().get(), State::SettingUp); + let _ = sync.next().await; + assert_eq!(room_list.state().get(), State::Running); + let _ = sync.next().await; + + let requests = server.received_requests().await.unwrap(); + let sliding_sync_requests: Vec<_> = + requests.iter().filter(|request| SlidingSyncMatcher.matches(request)).collect(); + + let timeout_of = |index: usize| -> Option { + sliding_sync_requests[index] + .url + .query_pairs() + .find(|(key, _)| key == "timeout") + .map(|(_, value)| value.into_owned()) + }; + + // The `SettingUp` request must keep `timeout=0`: `SyncIndicator` is derived + // from how long we stay outside `Running`, so long-polling here would falsely + // trigger the loading hint. + assert_eq!( + timeout_of(1).as_deref(), + Some("0"), + "request issued in `SettingUp` must not long-poll: {:?}", + sliding_sync_requests[1].url, + ); + + // The first request issued while already in `Running` must long-poll, even + // though the list is not yet fully loaded — otherwise an idle homeserver + // gets flooded with back-to-back `timeout=0` requests carrying the same + // `pos`. + assert_eq!( + timeout_of(2).as_deref(), + Some("30000"), + "request issued in `Running` must long-poll: {:?}", + sliding_sync_requests[2].url, + ); + + Ok(()) + } } diff --git a/crates/matrix-sdk-ui/tests/integration/room_list_service.rs b/crates/matrix-sdk-ui/tests/integration/room_list_service.rs index bd51ea944fc..ef06a1e2e58 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list_service.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list_service.rs @@ -415,7 +415,9 @@ async fn test_sync_all_states() -> Result<(), Error> { states = SettingUp => Running, // The previous `pos`. assert pos Some("0"), - // Still no long-polling because the list isn't fully-loaded. + // `SettingUp` keeps `timeout=0` to preserve `SyncIndicator` semantics: + // long-polling here would extend the time spent outside `Running` and + // falsely trigger the loading hint. assert timeout Some(0), assert request >= { "conn_id": "room-list", @@ -443,8 +445,8 @@ async fn test_sync_all_states() -> Result<(), Error> { [server, room_list, sync] states = Running => Running, assert pos Some("1"), - // Still no long-polling because the list isn't fully-loaded. - assert timeout Some(0), + // Long-polling is enabled in `Running`, regardless of fully-loaded. + assert timeout Some(30000), assert request >= { "conn_id": "room-list", "lists": { @@ -471,9 +473,8 @@ async fn test_sync_all_states() -> Result<(), Error> { [server, room_list, sync] states = Running => Running, assert pos Some("2"), - // Still no long-polling because the list isn't fully-loaded, - // but it's about to be! - assert timeout Some(0), + // Long-polling is enabled in `Running`, regardless of fully-loaded. + assert timeout Some(30000), assert request >= { "conn_id": "room-list", "lists": { @@ -500,7 +501,7 @@ async fn test_sync_all_states() -> Result<(), Error> { [server, room_list, sync] states = Running => Running, assert pos Some("3"), - // The list is fully-loaded, we can start long-polling. + // Long-polling is enabled in `Running`, regardless of fully-loaded. assert timeout Some(30000), assert request >= { "conn_id": "room-list",