From d99957d37028c4782d5c0912186266c05254de67 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 17 May 2023 16:06:14 +0200 Subject: [PATCH 01/11] feat(sdk): Implement `SlidingSyncList::set_sync_mode`. Changing the sync-mode of a list on-the-fly is necessary to optimise the new `RoomList` API. For example, we can start with a list in a selective mode, a range of `0..=50` and a `timeline_limit=0` to fetch the beginning of the room list, and then _change_ the sync-mode to growing to continue to sync the room list in the background. Today, this is done with 2 lists and a merge of the lists, but (i) this is error-prone, (ii) this is not optimal. Thank to `SlidingSyncList::set_sync_mode`, merging lists is no more necessary, thus removing a class of bugs in client's code. --- .../src/sliding_sync/list/builder.rs | 2 +- .../matrix-sdk/src/sliding_sync/list/mod.rs | 32 ++++++++++++++++--- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/crates/matrix-sdk/src/sliding_sync/list/builder.rs b/crates/matrix-sdk/src/sliding_sync/list/builder.rs index f3fc1154d..f04a928a6 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/builder.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/builder.rs @@ -202,7 +202,7 @@ impl SlidingSyncListBuilder { let list = SlidingSyncList { inner: Arc::new(SlidingSyncListInner { // From the builder - sync_mode: self.sync_mode.clone(), + sync_mode: StdRwLock::new(self.sync_mode.clone()), sort: self.sort, required_state: self.required_state, filters: self.filters, diff --git a/crates/matrix-sdk/src/sliding_sync/list/mod.rs b/crates/matrix-sdk/src/sliding_sync/list/mod.rs index e6a922613..16354e887 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/mod.rs @@ -60,9 +60,14 @@ impl SlidingSyncList { self.inner.name.as_str() } + /// Change the sync-mode. + pub fn set_sync_mode(&self, sync_mode: SlidingSyncMode) { + self.inner.set_sync_mode(sync_mode); + } + /// Set the ranges to fetch. pub fn set_ranges(&self, ranges: &[RangeInclusive]) -> Result<(), Error> { - if self.inner.sync_mode.ranges_can_be_modified_by_user().not() { + if self.inner.sync_mode.read().unwrap().ranges_can_be_modified_by_user().not() { return Err(Error::CannotModifyRanges(self.name().to_owned())); } @@ -74,7 +79,7 @@ impl SlidingSyncList { /// Reset the ranges to a particular set. pub fn set_range(&self, range: RangeInclusive) -> Result<(), Error> { - if self.inner.sync_mode.ranges_can_be_modified_by_user().not() { + if self.inner.sync_mode.read().unwrap().ranges_can_be_modified_by_user().not() { return Err(Error::CannotModifyRanges(self.name().to_owned())); } @@ -86,7 +91,7 @@ impl SlidingSyncList { /// Set the ranges to fetch. pub fn add_range(&self, range: RangeInclusive) -> Result<(), Error> { - if self.inner.sync_mode.ranges_can_be_modified_by_user().not() { + if self.inner.sync_mode.read().unwrap().ranges_can_be_modified_by_user().not() { return Err(Error::CannotModifyRanges(self.name().to_owned())); } @@ -103,7 +108,7 @@ impl SlidingSyncList { /// order but you will only receive updates when for rooms entering or /// leaving the set. pub fn reset_ranges(&self) -> Result<(), Error> { - if self.inner.sync_mode.ranges_can_be_modified_by_user().not() { + if self.inner.sync_mode.read().unwrap().ranges_can_be_modified_by_user().not() { return Err(Error::CannotModifyRanges(self.name().to_owned())); } @@ -258,7 +263,7 @@ pub(super) struct SlidingSyncListInner { state: StdRwLock>, /// Which [`SlidingSyncMode`] to start this list under. - sync_mode: SlidingSyncMode, + sync_mode: StdRwLock, /// Sort the room list by this. sort: Vec, @@ -300,6 +305,23 @@ pub(super) struct SlidingSyncListInner { } impl SlidingSyncListInner { + /// Change the sync-mode. + /// + /// This will change the sync-mode but also the request generator. + /// + /// [`Self::ranges`] will be updated when the next request will sent and a + /// response will be received. The [`Self::maximum_number_of_rooms`] + /// won't change. + pub fn set_sync_mode(&self, sync_mode: SlidingSyncMode) { + // Acquire both locks before modifying the values their hold. + let mut sync_mode_lock = self.sync_mode.write().unwrap(); + let mut request_generator_lock = self.request_generator.write().unwrap(); + + // Now we can modify both values. + *sync_mode_lock = sync_mode.clone(); + *request_generator_lock = SlidingSyncListRequestGenerator::new(sync_mode); + } + /// Reset and add new ranges. fn set_ranges(&self, ranges: &[RangeInclusive]) { Observable::set(&mut self.ranges.write().unwrap(), ranges.to_vec()); From 8bb04ed40bbafef852671ec4e101298e01a06e43 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 17 May 2023 16:10:02 +0200 Subject: [PATCH 02/11] test(sdk): Use the Rust inclusive range syntax for ranges. --- .../matrix-sdk/src/sliding_sync/list/mod.rs | 60 +++++++++---------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/crates/matrix-sdk/src/sliding_sync/list/mod.rs b/crates/matrix-sdk/src/sliding_sync/list/mod.rs index 16354e887..c0e6fafaf 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/mod.rs @@ -1223,7 +1223,7 @@ mod tests { maximum_number_of_rooms = $maximum_number_of_rooms:expr, $( next => { - ranges = $( [ $range_start:literal ; $range_end:literal ] ),* , + ranges = $( $range_start:literal ..= $range_end:literal ),* , is_fully_loaded = $is_fully_loaded:expr, list_state = $list_state:ident, } @@ -1276,29 +1276,29 @@ mod tests { list_state = NotLoaded, maximum_number_of_rooms = 25, next => { - ranges = [0; 9], + ranges = 0..=9, is_fully_loaded = false, list_state = PartiallyLoaded, }, next => { - ranges = [10; 19], + ranges = 10..=19, is_fully_loaded = false, list_state = PartiallyLoaded, }, // The maximum number of rooms is reached! next => { - ranges = [20; 24], + ranges = 20..=24, is_fully_loaded = true, list_state = FullyLoaded, }, // Now it's fully loaded, so the same request must be produced everytime. next => { - ranges = [0; 24], // the range starts at 0 now! + ranges = 0..=24, // the range starts at 0 now! is_fully_loaded = true, list_state = FullyLoaded, }, next => { - ranges = [0; 24], + ranges = 0..=24, is_fully_loaded = true, list_state = FullyLoaded, }, @@ -1318,29 +1318,29 @@ mod tests { list_state = NotLoaded, maximum_number_of_rooms = 25, next => { - ranges = [0; 9], + ranges = 0..=9, is_fully_loaded = false, list_state = PartiallyLoaded, }, next => { - ranges = [10; 19], + ranges = 10..=19, is_fully_loaded = false, list_state = PartiallyLoaded, }, // The maximum number of rooms to fetch is reached! next => { - ranges = [20; 21], + ranges = 20..=21, is_fully_loaded = true, list_state = FullyLoaded, }, // Now it's fully loaded, so the same request must be produced everytime. next => { - ranges = [0; 21], // the range starts at 0 now! + ranges = 0..=21, // the range starts at 0 now! is_fully_loaded = true, list_state = FullyLoaded, }, next => { - ranges = [0; 21], + ranges = 0..=21, is_fully_loaded = true, list_state = FullyLoaded, }, @@ -1360,29 +1360,29 @@ mod tests { list_state = NotLoaded, maximum_number_of_rooms = 25, next => { - ranges = [0; 9], + ranges = 0..=9, is_fully_loaded = false, list_state = PartiallyLoaded, }, next => { - ranges = [0; 19], + ranges = 0..=19, is_fully_loaded = false, list_state = PartiallyLoaded, }, // The maximum number of rooms is reached! next => { - ranges = [0; 24], + ranges = 0..=24, is_fully_loaded = true, list_state = FullyLoaded, }, // Now it's fully loaded, so the same request must be produced everytime. next => { - ranges = [0; 24], + ranges = 0..=24, is_fully_loaded = true, list_state = FullyLoaded, }, next => { - ranges = [0; 24], + ranges = 0..=24, is_fully_loaded = true, list_state = FullyLoaded, }, @@ -1402,29 +1402,29 @@ mod tests { list_state = NotLoaded, maximum_number_of_rooms = 25, next => { - ranges = [0; 9], + ranges = 0..=9, is_fully_loaded = false, list_state = PartiallyLoaded, }, next => { - ranges = [0; 19], + ranges = 0..=19, is_fully_loaded = false, list_state = PartiallyLoaded, }, // The maximum number of rooms is reached! next => { - ranges = [0; 21], + ranges = 0..=21, is_fully_loaded = true, list_state = FullyLoaded, }, // Now it's fully loaded, so the same request must be produced everytime. next => { - ranges = [0; 21], + ranges = 0..=21, is_fully_loaded = true, list_state = FullyLoaded, }, next => { - ranges = [0; 21], + ranges = 0..=21, is_fully_loaded = true, list_state = FullyLoaded, }, @@ -1446,18 +1446,18 @@ mod tests { maximum_number_of_rooms = 25, // The maximum number of rooms is reached directly! next => { - ranges = [0; 10], [42; 153], + ranges = 0..=10, 42..=153, is_fully_loaded = true, list_state = FullyLoaded, }, // Now it's fully loaded, so the same request must be produced everytime. next => { - ranges = [0; 10], [42; 153], + ranges = 0..=10, 42..=153, is_fully_loaded = true, list_state = FullyLoaded, }, next => { - ranges = [0; 10], [42; 153], + ranges = 0..=10, 42..=153, is_fully_loaded = true, list_state = FullyLoaded, } @@ -1479,18 +1479,18 @@ mod tests { maximum_number_of_rooms = 25, // The maximum number of rooms is reached directly! next => { - ranges = [0; 10], [42; 153], + ranges = 0..=10, 42..=153, is_fully_loaded = true, list_state = FullyLoaded, }, // Now it's fully loaded, so the same request must be produced everytime. next => { - ranges = [0; 10], [42; 153], + ranges = 0..=10, 42..=153, is_fully_loaded = true, list_state = FullyLoaded, }, next => { - ranges = [0; 10], [42; 153], + ranges = 0..=10, 42..=153, is_fully_loaded = true, list_state = FullyLoaded, } @@ -1503,7 +1503,7 @@ mod tests { list_state = PartiallyLoaded, maximum_number_of_rooms = 25, next => { - ranges = [3; 7], + ranges = 3..=7, is_fully_loaded = true, list_state = FullyLoaded, }, @@ -1516,7 +1516,7 @@ mod tests { list_state = PartiallyLoaded, maximum_number_of_rooms = 25, next => { - ranges = [3; 7], [10; 23], + ranges = 3..=7, 10..=23, is_fully_loaded = true, list_state = FullyLoaded, }, @@ -1529,7 +1529,7 @@ mod tests { list_state = PartiallyLoaded, maximum_number_of_rooms = 25, next => { - ranges = [42; 77], + ranges = 42..=77, is_fully_loaded = true, list_state = FullyLoaded, }, From f01e8dc9929e3fd8e0c8d16392db4df4426c6b66 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 17 May 2023 16:27:17 +0200 Subject: [PATCH 03/11] test(sdk): Test `SlidingSyncList::set_sync_mode`. --- .../matrix-sdk/src/sliding_sync/list/mod.rs | 151 +++++++++++++++++- 1 file changed, 148 insertions(+), 3 deletions(-) diff --git a/crates/matrix-sdk/src/sliding_sync/list/mod.rs b/crates/matrix-sdk/src/sliding_sync/list/mod.rs index c0e6fafaf..44dcd293e 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/mod.rs @@ -61,6 +61,14 @@ impl SlidingSyncList { } /// Change the sync-mode. + /// + /// It is sometimes necessary to change the sync-mode of a list on-the-fly. + /// + /// This will change the sync-mode but also the request generator. + /// + /// The ranges and the state will be updated when the next request will sent + /// and a response will be received. The maximum number of rooms won't + /// change. pub fn set_sync_mode(&self, sync_mode: SlidingSyncMode) { self.inner.set_sync_mode(sync_mode); } @@ -309,9 +317,9 @@ impl SlidingSyncListInner { /// /// This will change the sync-mode but also the request generator. /// - /// [`Self::ranges`] will be updated when the next request will sent and a - /// response will be received. The [`Self::maximum_number_of_rooms`] - /// won't change. + /// [`Self::ranges`] and [`Self::state`] will be updated when the next + /// request will sent and a response will be received. The + /// [`Self::maximum_number_of_rooms`] won't change. pub fn set_sync_mode(&self, sync_mode: SlidingSyncMode) { // Acquire both locks before modifying the values their hold. let mut sync_mode_lock = self.sync_mode.write().unwrap(); @@ -1549,6 +1557,143 @@ mod tests { }; } + #[test] + fn test_generator_changing_sync_mode_on_the_fly() { + let (sender, _receiver) = channel(4); + + let mut list = SlidingSyncList::builder("testing") + .sync_mode(SlidingSyncMode::new_selective()) + .ranges(vec![0..=10, 42..=153].to_vec()) + .build(sender); + + assert_ranges! { + list = list, + list_state = NotLoaded, + maximum_number_of_rooms = 25, + // The maximum number of rooms is reached directly! + next => { + ranges = 0..=10, 42..=153, + is_fully_loaded = true, + list_state = FullyLoaded, + }, + // Now it's fully loaded, so the same request must be produced everytime. + next => { + ranges = 0..=10, 42..=153, + is_fully_loaded = true, + list_state = FullyLoaded, + }, + next => { + ranges = 0..=10, 42..=153, + is_fully_loaded = true, + list_state = FullyLoaded, + } + }; + + // Changing from `Selective` to `Growing`. + list.set_sync_mode(SlidingSyncMode::new_growing(10, None)); + + assert_ranges! { + list = list, + list_state = FullyLoaded, // it inherits the state of the previous sync-mode since no sync has been made yet. + maximum_number_of_rooms = 25, + next => { + ranges = 0..=9, + is_fully_loaded = false, + list_state = PartiallyLoaded, + }, + next => { + ranges = 0..=19, + is_fully_loaded = false, + list_state = PartiallyLoaded, + }, + // The maximum number of rooms is reached! + next => { + ranges = 0..=24, + is_fully_loaded = true, + list_state = FullyLoaded, + }, + // Now it's fully loaded, so the same request must be produced everytime. + next => { + ranges = 0..=24, + is_fully_loaded = true, + list_state = FullyLoaded, + }, + next => { + ranges = 0..=24, + is_fully_loaded = true, + list_state = FullyLoaded, + }, + }; + + // Changing from `Growing` to `Paging`. + list.set_sync_mode(SlidingSyncMode::new_paging(10, None)); + + assert_ranges! { + list = list, + list_state = FullyLoaded, // it inherits the state of the previous sync-mode since no sync has been made yet. + maximum_number_of_rooms = 25, + next => { + ranges = 0..=9, + is_fully_loaded = false, + list_state = PartiallyLoaded, + }, + next => { + ranges = 10..=19, + is_fully_loaded = false, + list_state = PartiallyLoaded, + }, + // The maximum number of rooms is reached! + next => { + ranges = 20..=24, + is_fully_loaded = true, + list_state = FullyLoaded, + }, + // Now it's fully loaded, so the same request must be produced everytime. + next => { + ranges = 0..=24, // the range starts at 0 now! + is_fully_loaded = true, + list_state = FullyLoaded, + }, + next => { + ranges = 0..=24, + is_fully_loaded = true, + list_state = FullyLoaded, + }, + }; + + // Changing from `Paging` to `Selective`. + list.set_sync_mode(SlidingSyncMode::new_selective()); + + assert_eq!(list.state(), SlidingSyncState::FullyLoaded); // it inherits the state of the previous sync-mode. + + // We need to update the ranges, of course, as they are not managed + // automatically anymore. + list.set_range(0..=100).unwrap(); + + assert_ranges! { + list = list, + list_state = PartiallyLoaded, // changing the ranges make the state to go `PartiallyLoaded`. + maximum_number_of_rooms = 25, + // The maximum number of rooms is reached directly! + next => { + ranges = 0..=100, + is_fully_loaded = true, + list_state = FullyLoaded, + }, + // Now it's fully loaded, so the same request must be produced everytime. + next => { + ranges = 0..=100, + is_fully_loaded = true, + list_state = FullyLoaded, + }, + next => { + ranges = 0..=100, + is_fully_loaded = true, + list_state = FullyLoaded, + } + }; + } + #[tokio::test] #[allow(clippy::await_holding_lock)] async fn test_sliding_sync_inner_update_state_room_list_and_maximum_number_of_rooms() { From 2f29664fb88eaf9b2c00a1f3133bf55ed46d33a1 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Sat, 20 May 2023 17:57:11 +0200 Subject: [PATCH 04/11] doc(sdk): Fix a typo. Co-authored-by: Benjamin Bouvier --- crates/matrix-sdk/src/sliding_sync/list/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/matrix-sdk/src/sliding_sync/list/mod.rs b/crates/matrix-sdk/src/sliding_sync/list/mod.rs index 44dcd293e..3f5501ebc 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/mod.rs @@ -66,7 +66,7 @@ impl SlidingSyncList { /// /// This will change the sync-mode but also the request generator. /// - /// The ranges and the state will be updated when the next request will sent + /// The ranges and the state will be updated when the next request will be sent /// and a response will be received. The maximum number of rooms won't /// change. pub fn set_sync_mode(&self, sync_mode: SlidingSyncMode) { From 88ebd899374413bf37fead1686116379206bedd8 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Sat, 20 May 2023 17:57:21 +0200 Subject: [PATCH 05/11] doc(sdk): Fix a typo. Co-authored-by: Benjamin Bouvier --- crates/matrix-sdk/src/sliding_sync/list/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/matrix-sdk/src/sliding_sync/list/mod.rs b/crates/matrix-sdk/src/sliding_sync/list/mod.rs index 3f5501ebc..615cad5ac 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/mod.rs @@ -318,7 +318,7 @@ impl SlidingSyncListInner { /// This will change the sync-mode but also the request generator. /// /// [`Self::ranges`] and [`Self::state`] will be updated when the next - /// request will sent and a response will be received. The + /// request will be sent and a response will be received. The /// [`Self::maximum_number_of_rooms`] won't change. pub fn set_sync_mode(&self, sync_mode: SlidingSyncMode) { // Acquire both locks before modifying the values their hold. From 4ee2f2a44b248c35d4d4f64d1398ee87df8fe9eb Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 22 May 2023 08:43:10 +0200 Subject: [PATCH 06/11] chore(sdk): Run `rustfmt`. --- crates/matrix-sdk/src/sliding_sync/list/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/matrix-sdk/src/sliding_sync/list/mod.rs b/crates/matrix-sdk/src/sliding_sync/list/mod.rs index 615cad5ac..a2646d42a 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/mod.rs @@ -66,9 +66,9 @@ impl SlidingSyncList { /// /// This will change the sync-mode but also the request generator. /// - /// The ranges and the state will be updated when the next request will be sent - /// and a response will be received. The maximum number of rooms won't - /// change. + /// The ranges and the state will be updated when the next request will be + /// sent and a response will be received. The maximum number of rooms + /// won't change. pub fn set_sync_mode(&self, sync_mode: SlidingSyncMode) { self.inner.set_sync_mode(sync_mode); } From 82f5768df3c7d787b899d1cfe724109ae8015d5d Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 22 May 2023 09:37:32 +0200 Subject: [PATCH 07/11] feat(sdk): Rename `SlidingSyncInternalMessage` variants. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch renames the variant to be more explicit about what they do instead of re-using the “for loop vocabulary”. --- crates/matrix-sdk/src/sliding_sync/mod.rs | 24 ++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/crates/matrix-sdk/src/sliding_sync/mod.rs b/crates/matrix-sdk/src/sliding_sync/mod.rs index 0f92f5acb..7eea454f9 100644 --- a/crates/matrix-sdk/src/sliding_sync/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/mod.rs @@ -152,7 +152,9 @@ impl SlidingSync { .write() .unwrap() .insert(room_id, settings.unwrap_or_default()); - self.inner.internal_channel_send(SlidingSyncInternalMessage::ContinueSyncLoop).await?; + self.inner + .internal_channel_send(SlidingSyncInternalMessage::SyncLoopSkipOverCurrentIteration) + .await?; Ok(()) } @@ -163,7 +165,9 @@ impl SlidingSync { if self.inner.room_subscriptions.write().unwrap().remove(&room_id).is_some() { // … then keep the unsubscription for the next request. self.inner.room_unsubscriptions.write().unwrap().insert(room_id); - self.inner.internal_channel_send(SlidingSyncInternalMessage::ContinueSyncLoop).await?; + self.inner + .internal_channel_send(SlidingSyncInternalMessage::SyncLoopSkipOverCurrentIteration) + .await?; } Ok(()) @@ -230,7 +234,9 @@ impl SlidingSync { list_builder: SlidingSyncListBuilder, ) -> Result> { let list = list_builder.build(self.inner.internal_channel.0.clone()); - self.inner.internal_channel_send(SlidingSyncInternalMessage::ContinueSyncLoop).await?; + self.inner + .internal_channel_send(SlidingSyncInternalMessage::SyncLoopSkipOverCurrentIteration) + .await?; Ok(self.inner.lists.write().unwrap().insert(list.name().to_owned(), list)) } @@ -591,11 +597,11 @@ impl SlidingSync { use SlidingSyncInternalMessage::*; match internal_message { - None | Some(BreakSyncLoop) => { + None | Some(SyncLoopStop) => { break; } - Some(ContinueSyncLoop) => { + Some(SyncLoopSkipOverCurrentIteration) => { continue; } } @@ -683,9 +689,13 @@ impl SlidingSyncInner { #[derive(Debug)] enum SlidingSyncInternalMessage { + /// Instruct the sync loop to stop. #[allow(unused)] // temporary - BreakSyncLoop, - ContinueSyncLoop, + SyncLoopStop, + + /// Instruct the sync loop to skip over any remaining work in its iteration, + /// and to jump to the next iteration. + SyncLoopSkipOverCurrentIteration, } #[cfg(any(test, feature = "testing"))] From 6e77804070a1e38c57f7cfa2ed5c99ea56649841 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 22 May 2023 09:38:42 +0200 Subject: [PATCH 08/11] feat(sdk): `SlidingSyncList::set_sync_mode` sends an internal message. This patch updates `SlidingSyncList::set_sync_mode` to send a `SyncLoopSkipOverCurrentIteration` internal message to the sync loop. --- .../matrix-sdk/src/sliding_sync/list/mod.rs | 49 +++++++++++++------ 1 file changed, 35 insertions(+), 14 deletions(-) diff --git a/crates/matrix-sdk/src/sliding_sync/list/mod.rs b/crates/matrix-sdk/src/sliding_sync/list/mod.rs index a2646d42a..e0fcc078c 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/mod.rs @@ -64,13 +64,23 @@ impl SlidingSyncList { /// /// It is sometimes necessary to change the sync-mode of a list on-the-fly. /// - /// This will change the sync-mode but also the request generator. - /// - /// The ranges and the state will be updated when the next request will be - /// sent and a response will be received. The maximum number of rooms - /// won't change. - pub fn set_sync_mode(&self, sync_mode: SlidingSyncMode) { + /// This will change the sync-mode but also the request generator. A new + /// request generator is generated. Since requests are calculated based on + /// the request generator, changing the sync-mode is equivalent to + /// “resetting” the list. It's actually not calling [`Self::reset`], which + /// means that the state is not reset **purposely**. The ranges and the + /// state will be updated when the next request will be sent and a + /// response will be received. The maximum number of rooms won't change. + pub fn set_sync_mode(&self, sync_mode: SlidingSyncMode) -> Result<()> { self.inner.set_sync_mode(sync_mode); + + // When the sync mode is changed, the sync loop must skip over any work in its + // iteration and jump to the next iteration. + self.inner.internal_channel_blocking_send( + SlidingSyncInternalMessage::SyncLoopSkipOverCurrentIteration, + )?; + + Ok(()) } /// Set the ranges to fetch. @@ -242,11 +252,11 @@ impl SlidingSyncList { pub(super) fn reset(&self) -> Result<(), Error> { self.inner.reset(); - // When a list is reset, the sync loop must be “restarted”. - self.inner - .sliding_sync_internal_channel_sender - .blocking_send(SlidingSyncInternalMessage::ContinueSyncLoop) - .map_err(|_| Error::InternalChannelIsBroken)?; + // When a list is reset, the sync loop must skip over any work in its iteration + // and jump to the next iteration. + self.inner.internal_channel_blocking_send( + SlidingSyncInternalMessage::SyncLoopSkipOverCurrentIteration, + )?; Ok(()) } @@ -648,6 +658,17 @@ impl SlidingSyncListInner { Ok(()) } + + /// Send a message over the internal channel. + fn internal_channel_blocking_send( + &self, + message: SlidingSyncInternalMessage, + ) -> Result<(), Error> { + Ok(self + .sliding_sync_internal_channel_sender + .blocking_send(message) + .map_err(|_| Error::InternalChannelIsBroken)?) + } } #[instrument(skip(operations))] @@ -1590,7 +1611,7 @@ mod tests { }; // Changing from `Selective` to `Growing`. - list.set_sync_mode(SlidingSyncMode::new_growing(10, None)); + list.set_sync_mode(SlidingSyncMode::new_growing(10, None)).unwrap(); assert_ranges! { list = list, @@ -1626,7 +1647,7 @@ mod tests { }; // Changing from `Growing` to `Paging`. - list.set_sync_mode(SlidingSyncMode::new_paging(10, None)); + list.set_sync_mode(SlidingSyncMode::new_paging(10, None)).unwrap(); assert_ranges! { list = list, @@ -1662,7 +1683,7 @@ mod tests { }; // Changing from `Paging` to `Selective`. - list.set_sync_mode(SlidingSyncMode::new_selective()); + list.set_sync_mode(SlidingSyncMode::new_selective()).unwrap(); assert_eq!(list.state(), SlidingSyncState::FullyLoaded); // it inherits the state of the previous sync-mode. From fee1a50f383e530441f3b64e07028341f023cc1a Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 22 May 2023 09:39:43 +0200 Subject: [PATCH 09/11] doc(sdk): Improve documentation of `next_request`. --- crates/matrix-sdk/src/sliding_sync/list/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/matrix-sdk/src/sliding_sync/list/mod.rs b/crates/matrix-sdk/src/sliding_sync/list/mod.rs index e0fcc078c..a70c04090 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/mod.rs @@ -209,6 +209,9 @@ impl SlidingSyncList { } /// Calculate the next request and return it. + /// + /// The next request is entirely calculated based on the request generator + /// ([`SlidingSyncListRequestGenerator`]). pub(super) fn next_request(&mut self) -> Result { self.inner.next_request() } From e9399eb6358528f11d548e4fc60ddc96b0398bd1 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 22 May 2023 10:54:36 +0200 Subject: [PATCH 10/11] doc(sdk): Do no link to a private function. --- crates/matrix-sdk/src/sliding_sync/list/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/matrix-sdk/src/sliding_sync/list/mod.rs b/crates/matrix-sdk/src/sliding_sync/list/mod.rs index a70c04090..367214d5b 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/mod.rs @@ -67,7 +67,7 @@ impl SlidingSyncList { /// This will change the sync-mode but also the request generator. A new /// request generator is generated. Since requests are calculated based on /// the request generator, changing the sync-mode is equivalent to - /// “resetting” the list. It's actually not calling [`Self::reset`], which + /// “resetting” the list. It's actually not calling `Self::reset`, which /// means that the state is not reset **purposely**. The ranges and the /// state will be updated when the next request will be sent and a /// response will be received. The maximum number of rooms won't change. From abf8a50c0d5036a3e8d6acee8c427419d7e58a64 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 22 May 2023 11:24:11 +0200 Subject: [PATCH 11/11] chore(sdk): Make Clippy happy. --- crates/matrix-sdk/src/sliding_sync/list/mod.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/matrix-sdk/src/sliding_sync/list/mod.rs b/crates/matrix-sdk/src/sliding_sync/list/mod.rs index 367214d5b..81b5d66f8 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/mod.rs @@ -667,10 +667,9 @@ impl SlidingSyncListInner { &self, message: SlidingSyncInternalMessage, ) -> Result<(), Error> { - Ok(self - .sliding_sync_internal_channel_sender + self.sliding_sync_internal_channel_sender .blocking_send(message) - .map_err(|_| Error::InternalChannelIsBroken)?) + .map_err(|_| Error::InternalChannelIsBroken) } }