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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Nov 5, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a0523b0
Activate share_pos on the room-list sliding sync instance
MatMaul Sep 19, 2024
67f18d5
RoomListLoadingState now yields immediately with current value
MatMaul Oct 7, 2024
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
4 changes: 3 additions & 1 deletion crates/matrix-sdk-ui/src/room_list_service/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,9 @@ impl RoomListService {
}))
.with_typing_extension(assign!(http::request::Typing::default(), {
enabled: Some(true),
}));
}))
// We don't deal with encryption device messages here so this is safe
MatMaul marked this conversation as resolved.
Show resolved Hide resolved
.share_pos();

let sliding_sync = builder
.add_cached_list(
Expand Down
4 changes: 3 additions & 1 deletion crates/matrix-sdk-ui/src/room_list_service/room_list.rs
View file
Open in desktop
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()
MatMaul marked this conversation as resolved.
Show resolved Hide resolved
}

/// Get a stream of rooms.
Expand Down
134 changes: 128 additions & 6 deletions crates/matrix-sdk-ui/tests/integration/room_list_service.rs
View file
Open in desktop
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::sync::UnreadNotificationsCount;
use matrix_sdk_test::{async_test, mocks::mock_encryption_state};
use matrix_sdk_ui::{
Expand All @@ -23,6 +27,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 @@ -38,12 +43,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 @@ -53,6 +76,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 @@ -63,6 +87,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 @@ -80,6 +105,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 @@ -481,6 +507,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 @@ -509,6 +536,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 @@ -537,6 +565,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 @@ -560,6 +589,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 @@ -1046,8 +1170,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 @@ -1153,11 +1276,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
View file
Open in desktop
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