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 5 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
134 changes: 128 additions & 6 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,11 @@ 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::{
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 @@ -27,6 +31,7 @@ use ruma::{
};
use serde_json::json;
use stream_assert::{assert_next_matches, assert_pending};
use tempfile::TempDir;
use tokio::{spawn, sync::mpsc::channel, task::yield_now};
use wiremock::{
matchers::{header, method, path},
Expand All @@ -42,12 +47,30 @@ async fn new_room_list_service() -> Result<(Client, MockServer, RoomListService)
Ok((client, server, room_list))
}

async fn new_persistent_room_list_service(
store_path: &std::path::Path,
) -> Result<(MockServer, RoomListService), Error> {
let server = MockServer::start().await;
MatMaul marked this conversation as resolved.
Show resolved Hide resolved
let client = test_client_builder(Some(server.uri().to_string()))
.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((server, 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 $pos:expr, )?
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 +80,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 $pos, )?
assert request $assert_request { $( $request_json )* },
respond with = $( ( code $code ) )? { $( $response_json )* },
$( after delay = $response_delay, )?
Expand All @@ -67,6 +91,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 $pos:expr, )?
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 +109,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 $pos, )?
assert request $assert_request { $( $request_json )* },
respond with = $( ( code $code ) )? { $( $response_json )* },
$( after delay = $response_delay, )?
Expand Down Expand Up @@ -483,6 +509,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 None::<String>,
MatMaul marked this conversation as resolved.
Show resolved Hide resolved
assert request >= {
"lists": {
ALL_ROOMS: {
Expand Down Expand Up @@ -511,6 +538,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 Some("0"),
assert request >= {
"lists": {
ALL_ROOMS: {
Expand Down Expand Up @@ -539,6 +567,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 Some("1"),
assert request >= {
"lists": {
ALL_ROOMS: {
Expand All @@ -562,6 +591,101 @@ 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, room_list) = new_persistent_room_list_service(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!(all_rooms_loading_state, RoomListLoadingState::NotLoaded);
MatMaul marked this conversation as resolved.
Show resolved Hide resolved
assert_pending!(all_rooms_loading_state);

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

{
let (server, room_list) = new_persistent_room_list_service(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();

// Wait on Tokio to run all the tasks. Necessary only when testing.
yield_now().await;
MatMaul marked this conversation as resolved.
Show resolved Hide resolved

// We already have a state stored so the list should already be loaded
assert_next_matches!(
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 Some("0"),
assert request >= {
"lists": {
ALL_ROOMS: {
"ranges": [[0, 19]],
},
},
},
respond with = {
"pos": "1",
"lists": {
ALL_ROOMS: {
"count": 12,
},
},
"rooms": {},
},
};

// Wait on Tokio to run all the tasks. Necessary only when testing.
yield_now().await;
MatMaul marked this conversation as resolved.
Show resolved Hide resolved

// maximum_number_of_rooms changed so we should get a new loaded state
assert_next_matches!(
MatMaul marked this conversation as resolved.
Show resolved Hide resolved
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,8 +1172,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_pending!(all_rooms_loading_state);
MatMaul marked this conversation as resolved.
Show resolved Hide resolved
assert_next_matches!(all_rooms_loading_state, RoomListLoadingState::NotLoaded);
MatMaul marked this conversation as resolved.
Show resolved Hide resolved

sync_then_assert_request_and_fake_response! {
[server, room_list, sync]
Expand Down Expand Up @@ -1155,11 +1278,10 @@ 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!(
MatMaul marked this conversation as resolved.
Show resolved Hide resolved
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

sync_then_assert_request_and_fake_response! {
[server, room_list, sync]
Expand Down
11 changes: 11 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 $pos:expr, )?
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 $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 $pos:expr, )?
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 $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 Down
Loading