From dec4b2122b2bc843d596862d83317bcabb39aa0f Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 22 Feb 2023 16:51:42 +0100 Subject: [PATCH 1/2] doc(sdk): Improve documentation of `SlidingSyncMode`. --- crates/matrix-sdk/src/sliding_sync/mod.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/matrix-sdk/src/sliding_sync/mod.rs b/crates/matrix-sdk/src/sliding_sync/mod.rs index a35f0989f..418b834cc 100644 --- a/crates/matrix-sdk/src/sliding_sync/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/mod.rs @@ -690,11 +690,13 @@ pub enum SlidingSyncState { /// The mode by which the the [`SlidingSyncView`] is in fetching the data. #[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize)] pub enum SlidingSyncMode { - /// fully sync all rooms in the background, page by page of `batch_size` + /// Fully sync all rooms in the background, page by page of `batch_size`, + /// like `0..20`, `21..40`, 41..60` etc. assuming the `batch_size` is 20. #[serde(alias = "FullSync")] PagingFullSync, - /// fully sync all rooms in the background, with a growing window of - /// `batch_size`, + /// Fully sync all rooms in the background, with a growing window of + /// `batch_size`, like `0..20`, `0..40`, `0..60` etc. assuming the + /// `batch_size` is 20. GrowingFullSync, /// Only sync the specific windows defined #[default] From 5727726e5d2bf03722f057d6ba2bd91c06ec8dfc Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 22 Feb 2023 17:07:50 +0100 Subject: [PATCH 2/2] feat(sdk): Make `SlidingSync.pos` already private. So far, the `SlidingSync.pos` field was public to the crate. In order to avoid breaking the internal state of this type, its visibility is now private. However, we need to be able to change the value when testing the `SlidingSync` type itself. To achieve that, this patch removes the old `force_sliding_sync_pos` function, and implements 2 new functions: `set_pos` and `pos` directly on `SlidingSync` only when `#[cfg(any(test, feature ="testing"))]`. --- crates/matrix-sdk/src/sliding_sync/mod.rs | 23 ++++++++++++++++--- crates/matrix-sdk/src/test_utils.rs | 8 ------- .../sliding-sync-integration-test/src/lib.rs | 5 ++-- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/crates/matrix-sdk/src/sliding_sync/mod.rs b/crates/matrix-sdk/src/sliding_sync/mod.rs index 418b834cc..cd90467cf 100644 --- a/crates/matrix-sdk/src/sliding_sync/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/mod.rs @@ -766,8 +766,9 @@ pub struct SlidingSync { /// The storage key to keep this cache at and load it from storage_key: Option, - // ------ Internal state - pub(crate) pos: Mutable>, + /// The `pos` marker. + pos: Mutable>, + delta_token: Mutable>, /// The views of this sliding sync instance @@ -1153,11 +1154,14 @@ impl SlidingSync { match self.sync_once(&mut views).instrument(sync_span.clone()).await { Ok(Some(updates)) => { self.failure_count.store(0, Ordering::SeqCst); + yield Ok(updates) - }, + } + Ok(None) => { break; } + Err(e) => { if e.client_api_error_kind() == Some(&ErrorKind::UnknownPos) { // session expired, let's reset @@ -1189,6 +1193,19 @@ impl SlidingSync { } } +#[cfg(any(test, feature = "testing"))] +impl SlidingSync { + /// Get a copy of the `pos` value. + pub fn pos(&self) -> Option { + self.pos.get_cloned() + } + + /// Set a new value for `pos`. + pub fn set_pos(&self, new_pos: String) { + self.pos.set(Some(new_pos)) + } +} + #[cfg(test)] mod test { use ruma::room_id; diff --git a/crates/matrix-sdk/src/test_utils.rs b/crates/matrix-sdk/src/test_utils.rs index 30ebe50fe..53efd0d34 100644 --- a/crates/matrix-sdk/src/test_utils.rs +++ b/crates/matrix-sdk/src/test_utils.rs @@ -4,8 +4,6 @@ use matrix_sdk_base::Session; use ruma::{api::MatrixVersion, device_id, user_id}; -#[cfg(feature = "experimental-sliding-sync")] -use crate::sliding_sync::SlidingSync; use crate::{config::RequestConfig, Client, ClientBuilder}; pub(crate) fn test_client_builder(homeserver_url: Option) -> ClientBuilder { @@ -33,9 +31,3 @@ pub(crate) async fn logged_in_client(homeserver_url: Option) -> Client { client } - -/// Force a specific pos-value to be used for the given sliding-sync instance. -#[cfg(feature = "experimental-sliding-sync")] -pub fn force_sliding_sync_pos(sliding_sync: &SlidingSync, new_pos: String) { - sliding_sync.pos.set(Some(new_pos)); -} diff --git a/testing/sliding-sync-integration-test/src/lib.rs b/testing/sliding-sync-integration-test/src/lib.rs index 853c00e9a..52bc287e0 100644 --- a/testing/sliding-sync-integration-test/src/lib.rs +++ b/testing/sliding-sync-integration-test/src/lib.rs @@ -81,7 +81,6 @@ mod tests { api::client::error::ErrorKind as RumaError, events::room::message::RoomMessageEventContent, UInt, }, - test_utils::force_sliding_sync_pos, SlidingSyncMode, SlidingSyncState, SlidingSyncView, }; @@ -1042,7 +1041,7 @@ mod tests { ); // force the pos to be invalid and thus this being reset internally - force_sliding_sync_pos(&sync_proxy, "100".to_owned()); + sync_proxy.set_pos("100".to_string()); let mut error_seen = false; for _n in 0..2 { @@ -1249,7 +1248,7 @@ mod tests { assert!(room_updated, "Room update has not been seen"); // force the pos to be invalid and thus this being reset internally - force_sliding_sync_pos(&sync_proxy, "100".to_owned()); + sync_proxy.set_pos("100".to_owned()); let mut error_seen = false; let mut room_updated = false;