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

Jump to bottom

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4140bbd
Activate share_pos on the room-list sliding sync instance
MatMaul Sep 19, 2024
ac4d99c
Address comments
MatMaul Sep 30, 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
1 change: 1 addition & 0 deletions 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 @@ -116,6 +116,7 @@ impl RoomListService {
let mut builder = client
.sliding_sync("room-list")
.map_err(Error::SlidingSync)?
.share_pos()
MatMaul marked this conversation as resolved.
Show resolved Hide resolved
.with_account_data_extension(
assign!(http::request::AccountData::default(), { enabled: Some(true) }),
)
Expand Down
95 changes: 94 additions & 1 deletion 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::{
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: impl AsRef<std::path::Path>,
MatMaul marked this conversation as resolved.
Show resolved Hide resolved
) -> Result<(MockServer, RoomListService), Error> {
let server = MockServer::start().await;
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 @@ -479,6 +505,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>,
assert request >= {
"lists": {
ALL_ROOMS: {
Expand Down Expand Up @@ -506,6 +533,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 @@ -533,6 +561,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 @@ -555,6 +584,70 @@ 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);

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);

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": 10,
},
},
"rooms": {},
},
};
}

Ok(())
}

#[async_test]
async fn test_sync_resumes_from_error() -> Result<(), Error> {
let (_, server, room_list) = new_room_list_service().await?;
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