Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Activate share_pos on the room-list sliding sync instance #4035

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions crates/matrix-sdk-ui/src/room_list_service/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,9 @@ impl RoomListService {
.with_to_device_extension(
assign!(http::request::ToDevice::default(), { enabled: Some(true) }),
);
} else {
// TODO: This is racy with encryption, needs cross-process lock
builder = builder.share_pos();
}

let sliding_sync = builder
Expand Down
4 changes: 3 additions & 1 deletion crates/matrix-sdk-ui/src/room_list_service/room_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,10 @@ impl RoomList {
}

/// Get a subscriber to the room list loading state.
///
/// This method will send out the current loading state as the first update.
pub fn loading_state(&self) -> Subscriber<RoomListLoadingState> {
self.loading_state.subscribe()
self.loading_state.subscribe_reset()
}

/// Get a stream of rooms.
Expand Down
140 changes: 132 additions & 8 deletions crates/matrix-sdk-ui/tests/integration/room_list_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@ use std::{
use assert_matches::assert_matches;
use eyeball_im::VectorDiff;
use futures_util::{pin_mut, FutureExt, StreamExt};
use matrix_sdk::{test_utils::logged_in_client_with_server, Client};
use matrix_sdk::{
assert_next_matches_with_timeout,
config::RequestConfig,
test_utils::{logged_in_client_with_server, set_client_session, test_client_builder},
Client,
};
use matrix_sdk_base::{
sliding_sync::http::request::RoomSubscription, sync::UnreadNotificationsCount,
};
Expand All @@ -26,7 +31,8 @@ use ruma::{
mxc_uri, room_id, uint,
};
use serde_json::json;
use stream_assert::{assert_next_matches, assert_pending};
use stream_assert::assert_pending;
use tempfile::TempDir;
use tokio::{spawn, sync::mpsc::channel, task::yield_now};
use wiremock::{
matchers::{header, method, path},
Expand All @@ -42,12 +48,30 @@ async fn new_room_list_service() -> Result<(Client, MockServer, RoomListService)
Ok((client, server, room_list))
}

async fn new_persistent_room_list_service(
homeserver_url: String,
store_path: &std::path::Path,
) -> Result<RoomListService, Error> {
let client = test_client_builder(Some(homeserver_url))
.request_config(RequestConfig::new().disable_retry())
.sqlite_store(store_path, None)
.build()
.await
.unwrap();
set_client_session(&client).await;

let room_list = RoomListService::new(client.clone()).await?;

Ok(room_list)
}

// Same macro as in the main, with additional checking that the state
// before/after the sync loop match those we expect.
macro_rules! sync_then_assert_request_and_fake_response {
(
[$server:ident, $room_list:ident, $stream:ident]
$( states = $pre_state:pat => $post_state:pat, )?
$( assert pos is $pos:tt, )?
assert request $assert_request:tt { $( $request_json:tt )* },
respond with = $( ( code $code:expr ) )? { $( $response_json:tt )* }
$( , after delay = $response_delay:expr )?
Expand All @@ -57,6 +81,7 @@ macro_rules! sync_then_assert_request_and_fake_response {
[$server, $room_list, $stream]
sync matches Some(Ok(_)),
$( states = $pre_state => $post_state, )?
$( assert pos is $pos, )?
assert request $assert_request { $( $request_json )* },
respond with = $( ( code $code ) )? { $( $response_json )* },
$( after delay = $response_delay, )?
Expand All @@ -67,6 +92,7 @@ macro_rules! sync_then_assert_request_and_fake_response {
[$server:ident, $room_list:ident, $stream:ident]
sync matches $sync_result:pat,
$( states = $pre_state:pat => $post_state:pat, )?
$( assert pos is $pos:tt, )?
assert request $assert_request:tt { $( $request_json:tt )* },
respond with = $( ( code $code:expr ) )? { $( $response_json:tt )* }
$( , after delay = $response_delay:expr )?
Expand All @@ -84,6 +110,7 @@ macro_rules! sync_then_assert_request_and_fake_response {
let next = super::sliding_sync_then_assert_request_and_fake_response! {
[$server, $stream]
sync matches $sync_result,
$( assert pos is $pos, )?
assert request $assert_request { $( $request_json )* },
respond with = $( ( code $code ) )? { $( $response_json )* },
$( after delay = $response_delay, )?
Expand Down Expand Up @@ -483,6 +510,7 @@ async fn test_sync_resumes_from_previous_state() -> Result<(), Error> {
sync_then_assert_request_and_fake_response! {
[server, room_list, sync]
states = Init => SettingUp,
assert pos is none,
assert request >= {
"lists": {
ALL_ROOMS: {
Expand Down Expand Up @@ -511,6 +539,7 @@ async fn test_sync_resumes_from_previous_state() -> Result<(), Error> {
sync_then_assert_request_and_fake_response! {
[server, room_list, sync]
states = SettingUp => Running,
assert pos is "0",
assert request >= {
"lists": {
ALL_ROOMS: {
Expand Down Expand Up @@ -539,6 +568,7 @@ async fn test_sync_resumes_from_previous_state() -> Result<(), Error> {
sync_then_assert_request_and_fake_response! {
[server, room_list, sync]
states = Running => Running,
assert pos is "1",
assert request >= {
"lists": {
ALL_ROOMS: {
Expand All @@ -562,6 +592,97 @@ async fn test_sync_resumes_from_previous_state() -> Result<(), Error> {
Ok(())
}

#[async_test]
async fn test_sync_resumes_from_previous_state_after_restart() -> Result<(), Error> {
let tmp_dir = TempDir::new().unwrap();
let store_path = tmp_dir.path();

let server = MockServer::start().await;

{
let room_list = new_persistent_room_list_service(server.uri(), store_path).await?;
let sync = room_list.sync();
pin_mut!(sync);

let all_rooms = room_list.all_rooms().await?;
let mut all_rooms_loading_state = all_rooms.loading_state();

// The loading is not loaded.
assert_next_matches_with_timeout!(all_rooms_loading_state, RoomListLoadingState::NotLoaded);
assert_pending!(all_rooms_loading_state);

sync_then_assert_request_and_fake_response! {
[server, room_list, sync]
states = Init => SettingUp,
assert pos is none,
assert request >= {
"lists": {
ALL_ROOMS: {
"ranges": [[0, 19]],
},
},
},
respond with = {
"pos": "0",
"lists": {
ALL_ROOMS: {
"count": 10,
},
},
"rooms": {},
},
};
}

{
let room_list = new_persistent_room_list_service(server.uri(), store_path).await?;
let sync = room_list.sync();
pin_mut!(sync);

let all_rooms = room_list.all_rooms().await?;
let mut all_rooms_loading_state = all_rooms.loading_state();

// We already have a state stored so the list should already be loaded
assert_next_matches_with_timeout!(
all_rooms_loading_state,
RoomListLoadingState::Loaded { maximum_number_of_rooms: Some(10) }
);
assert_pending!(all_rooms_loading_state);

// pos has been restored and is used when doing the req
sync_then_assert_request_and_fake_response! {
[server, room_list, sync]
states = Init => SettingUp,
assert pos is "0",
assert request >= {
"lists": {
ALL_ROOMS: {
"ranges": [[0, 19]],
},
},
},
respond with = {
"pos": "1",
"lists": {
ALL_ROOMS: {
"count": 12,
},
},
"rooms": {},
},
};

// maximum_number_of_rooms changed so we should get a new loaded state
assert_next_matches_with_timeout!(
all_rooms_loading_state,
RoomListLoadingState::Loaded { maximum_number_of_rooms: Some(12) }
);
assert_pending!(all_rooms_loading_state);
}

Ok(())
}

#[async_test]
async fn test_sync_resumes_from_error() -> Result<(), Error> {
let (_, server, room_list) = new_room_list_service().await?;
Expand Down Expand Up @@ -1048,7 +1169,7 @@ async fn test_loading_states() -> Result<(), Error> {
let mut all_rooms_loading_state = all_rooms.loading_state();

// The loading is not loaded.
assert_matches!(all_rooms_loading_state.get(), RoomListLoadingState::NotLoaded);
assert_next_matches_with_timeout!(all_rooms_loading_state, RoomListLoadingState::NotLoaded);
assert_pending!(all_rooms_loading_state);
MatMaul marked this conversation as resolved.
Show resolved Hide resolved

sync_then_assert_request_and_fake_response! {
Expand Down Expand Up @@ -1077,10 +1198,11 @@ async fn test_loading_states() -> Result<(), Error> {
yield_now().await;

// There is a loading state update, it's loaded now!
assert_next_matches!(
assert_next_matches_with_timeout!(
all_rooms_loading_state,
RoomListLoadingState::Loaded { maximum_number_of_rooms: Some(10) }
);
assert_pending!(all_rooms_loading_state);

sync_then_assert_request_and_fake_response! {
[server, room_list, sync]
Expand Down Expand Up @@ -1108,10 +1230,11 @@ async fn test_loading_states() -> Result<(), Error> {
yield_now().await;

// There is a loading state update because the number of rooms has been updated.
assert_next_matches!(
assert_next_matches_with_timeout!(
all_rooms_loading_state,
RoomListLoadingState::Loaded { maximum_number_of_rooms: Some(12) }
);
assert_pending!(all_rooms_loading_state);

sync_then_assert_request_and_fake_response! {
[server, room_list, sync]
Expand Down Expand Up @@ -1155,8 +1278,8 @@ async fn test_loading_states() -> Result<(), Error> {
pin_mut!(sync);

// The loading state is loaded! Indeed, there is data loaded from the cache.
assert_matches!(
all_rooms_loading_state.get(),
assert_next_matches_with_timeout!(
all_rooms_loading_state,
RoomListLoadingState::Loaded { maximum_number_of_rooms: Some(12) }
);
assert_pending!(all_rooms_loading_state);
MatMaul marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -1187,10 +1310,11 @@ async fn test_loading_states() -> Result<(), Error> {
yield_now().await;

// The loading state has been updated.
assert_next_matches!(
assert_next_matches_with_timeout!(
all_rooms_loading_state,
RoomListLoadingState::Loaded { maximum_number_of_rooms: Some(13) }
);
assert_pending!(all_rooms_loading_state);
}

Ok(())
Expand Down
14 changes: 14 additions & 0 deletions crates/matrix-sdk-ui/tests/integration/sliding_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ impl Match for SlidingSyncMatcher {
macro_rules! sliding_sync_then_assert_request_and_fake_response {
(
[$server:ident, $stream:ident]
$( assert pos is $pos:tt, )?
assert request $sign:tt { $( $request_json:tt )* },
respond with = $( ( code $code:expr ) )? { $( $response_json:tt )* }
$( , after delay = $response_delay:expr )?
Expand All @@ -66,6 +67,7 @@ macro_rules! sliding_sync_then_assert_request_and_fake_response {
sliding_sync_then_assert_request_and_fake_response! {
[$server, $stream]
sync matches Some(Ok(_)),
$( assert pos is $pos, )?
assert request $sign { $( $request_json )* },
respond with = $( ( code $code ) )? { $( $response_json )* },
$( after delay = $response_delay, )?
Expand All @@ -75,6 +77,7 @@ macro_rules! sliding_sync_then_assert_request_and_fake_response {
(
[$server:ident, $stream:ident]
sync matches $sync_result:pat,
$( assert pos is $pos:tt, )?
assert request $sign:tt { $( $request_json:tt )* },
respond with = $( ( code $code:expr ) )? { $( $response_json:tt )* }
$( , after delay = $response_delay:expr )?
Expand Down Expand Up @@ -117,6 +120,14 @@ macro_rules! sliding_sync_then_assert_request_and_fake_response {
root.remove("txn_id");
}

// Validate `pos` from the query parameter if specified.
$(
match $crate::sliding_sync_then_assert_request_and_fake_response!(@assertion_pos $pos) {
Some(pos) => assert!(wiremock::matchers::query_param("pos", pos).matches(request)),
None => assert!(wiremock::matchers::query_param_is_missing("pos").matches(request)),
}
)?

if let Err(error) = assert_json_diff::assert_json_matches_no_panic(
&json_value,
&json!({ $( $request_json )* }),
Expand All @@ -139,4 +150,7 @@ macro_rules! sliding_sync_then_assert_request_and_fake_response {

(@assertion_config >=) => { assert_json_diff::Config::new(assert_json_diff::CompareMode::Inclusive) };
(@assertion_config =) => { assert_json_diff::Config::new(assert_json_diff::CompareMode::Strict) };

(@assertion_pos none) => { None::<String> };
(@assertion_pos $val:expr) => { Some($val) };
}
Loading