feat(sdk): SlidingSyncList cannot be removed from SlidingSync.

`create_range` and `SlidingSyncList::next_request` return a `Result`
instead of an `Option`. It was a legacy from the old `impl Iterator` of
`SlidingSyncListRequestGenerator`. But actually, when `create_range`
fails, it must return a `Err` value, not a `None` value.

And since it's a regular error, `sync_once` can propagate the
error. Thus, it is no longer necessary to “update” the list of
`SlidingSyncList`. It's not necessary to remove some items in this list
when the `next_request` method can no return a value: it always returns
a value, except when a error happens.

crates/matrix-sdk/src/sliding_sync/error.rs

@@ -4,6 +4,7 @@ use thiserror::Error;
 
 /// Internal representation of errors in Sliding Sync.
 #[derive(Error, Debug)]
+#[cfg_attr(test, derive(PartialEq))]
 #[non_exhaustive]
 pub enum Error {
     /// The response we've received from the server can't be parsed or doesn't
@@ -24,4 +25,12 @@ pub enum Error {
     /// selected sync mode doesn't allow that.
     #[error("The chosen sync mode for the list `{0}` doesn't allow to modify the ranges")]
     CannotModifyRanges(String),
+    /// Ranges have a `start` bound greater than `end`.
+    #[error("Ranges have invalid bounds: `{start}..{end}`")]
+    InvalidRange {
+        /// Start bound.
+        start: u32,
+        /// End bound.
+        end: u32,
+    },
 }

crates/matrix-sdk/src/sliding_sync/list/mod.rs

@@ -204,7 +204,7 @@ impl SlidingSyncList {
     }
 
     /// Calculate the next request and return it.
-    pub(super) fn next_request(&mut self) -> Option<v4::SyncRequestList> {
+    pub(super) fn next_request(&mut self) -> Result<v4::SyncRequestList, Error> {
         self.inner.next_request()
     }
 
@@ -304,7 +304,7 @@ impl SlidingSyncListInner {
         Observable::set(&mut self.state.write().unwrap(), SlidingSyncState::NotLoaded);
     }
 
-    fn next_request(&self) -> Option<v4::SyncRequestList> {
+    fn next_request(&self) -> Result<v4::SyncRequestList, Error> {
         {
             // Use a dedicated scope to ensure the lock is released before continuing.
             let mut request_generator = self.request_generator.write().unwrap();
@@ -362,7 +362,7 @@ impl SlidingSyncListInner {
         }
 
         // Here we go.
-        Some(self.request())
+        Ok(self.request())
     }
 
     /// Build a [`SyncRequestList`][v4::SyncRequestList] based on the current

crates/matrix-sdk/src/sliding_sync/list/request_generator.rs

@@ -33,6 +33,8 @@ use std::cmp::min;
 
 use ruma::UInt;
 
+use crate::sliding_sync::Error;
+
 /// The kind of request generator.
 #[derive(Debug)]
 pub(super) enum SlidingSyncListRequestGeneratorKind {
@@ -114,8 +116,10 @@ impl SlidingSyncListRequestGenerator {
     }
 
     pub(super) fn reset(&mut self) {
+        // Reset the ranges.
         self.ranges.clear();
 
+        // Reset particular parts of the generator.
         match &mut self.kind {
             SlidingSyncListRequestGeneratorKind::Paging {
                 number_of_fetched_rooms,
@@ -150,7 +154,7 @@ pub(super) fn create_range(
     desired_size: u32,
     maximum_number_of_rooms_to_fetch: Option<u32>,
     maximum_number_of_rooms: Option<u32>,
-) -> Option<(UInt, UInt)> {
+) -> Result<(UInt, UInt), Error> {
     // Calculate the range.
     // The `start` bound is given. Let's calculate the `end` bound.
 
@@ -178,10 +182,10 @@ pub(super) fn create_range(
     // Make sure `start` is smaller than `end`. It can happen if `start` is greater
     // than `maximum_number_of_rooms_to_fetch` or `maximum_number_of_rooms`.
     if start > end {
-        return None;
+        return Err(Error::InvalidRange { start, end });
     }
 
-    Some((start.into(), end.into()))
+    Ok((start.into(), end.into()))
 }
 
 #[cfg(test)]
@@ -193,41 +197,47 @@ mod tests {
     #[test]
     fn test_create_range_from() {
         // From 0, we want 100 items.
-        assert_eq!(create_range(0, 100, None, None), Some((uint!(0), uint!(99))));
+        assert_eq!(create_range(0, 100, None, None), Ok((uint!(0), uint!(99))));
 
         // From 100, we want 100 items.
-        assert_eq!(create_range(100, 100, None, None), Some((uint!(100), uint!(199))));
+        assert_eq!(create_range(100, 100, None, None), Ok((uint!(100), uint!(199))));
 
         // From 0, we want 100 items, but there is a maximum number of rooms to fetch
         // defined at 50.
-        assert_eq!(create_range(0, 100, Some(50), None), Some((uint!(0), uint!(49))));
+        assert_eq!(create_range(0, 100, Some(50), None), Ok((uint!(0), uint!(49))));
 
         // From 49, we want 100 items, but there is a maximum number of rooms to fetch
         // defined at 50. There is 1 item to load.
-        assert_eq!(create_range(49, 100, Some(50), None), Some((uint!(49), uint!(49))));
+        assert_eq!(create_range(49, 100, Some(50), None), Ok((uint!(49), uint!(49))));
 
         // From 50, we want 100 items, but there is a maximum number of rooms to fetch
         // defined at 50.
-        assert_eq!(create_range(50, 100, Some(50), None), None);
+        assert_eq!(
+            create_range(50, 100, Some(50), None),
+            Err(Error::InvalidRange { start: 50, end: 49 })
+        );
 
         // From 0, we want 100 items, but there is a maximum number of rooms defined at
         // 50.
-        assert_eq!(create_range(0, 100, None, Some(50)), Some((uint!(0), uint!(49))));
+        assert_eq!(create_range(0, 100, None, Some(50)), Ok((uint!(0), uint!(49))));
 
         // From 49, we want 100 items, but there is a maximum number of rooms defined at
         // 50. There is 1 item to load.
-        assert_eq!(create_range(49, 100, None, Some(50)), Some((uint!(49), uint!(49))));
+        assert_eq!(create_range(49, 100, None, Some(50)), Ok((uint!(49), uint!(49))));
 
         // From 50, we want 100 items, but there is a maximum number of rooms defined at
         // 50.
-        assert_eq!(create_range(50, 100, None, Some(50)), None);
+        assert_eq!(
+            create_range(50, 100, None, Some(50)),
+            Err(Error::InvalidRange { start: 50, end: 49 })
+        );
 
         // From 0, we want 100 items, but there is a maximum number of rooms to fetch
         // defined at 75, and a maximum number of rooms defined at 50.
-        assert_eq!(create_range(0, 100, Some(75), Some(50)), Some((uint!(0), uint!(49))));
+        assert_eq!(create_range(0, 100, Some(75), Some(50)), Ok((uint!(0), uint!(49))));
 
         // From 0, we want 100 items, but there is a maximum number of rooms to fetch
         // defined at 50, and a maximum number of rooms defined at 75.
-        assert_eq!(create_range(0, 100, Some(50), Some(75)), Some((uint!(0), uint!(49))));
+        assert_eq!(create_range(0, 100, Some(50), Some(75)), Ok((uint!(0), uint!(49))));
     }
 }

crates/matrix-sdk/src/sliding_sync/mod.rs

@@ -429,23 +429,14 @@ impl SlidingSync {
         {
             let mut lists_lock = lists.lock().unwrap();
             let lists = lists_lock.borrow_mut();
-            let mut lists_to_remove = Vec::new();
-
-            for (name, list) in lists.iter_mut() {
-                if let Some(list_request) = list.next_request() {
-                    requests_lists.insert(name.clone(), list_request);
-                } else {
-                    lists_to_remove.push(name.clone());
-                }
-            }
-
-            for list_name in lists_to_remove {
-                lists.remove(&list_name);
-            }
 
             if lists.is_empty() {
                 return Ok(None);
             }
+
+            for (name, list) in lists.iter_mut() {
+                requests_lists.insert(name.clone(), list.next_request()?);
+            }
         }
 
         let (pos, delta_token) = {