feat(sdk): as_vectors are no longer unsafe.

This patch removes the `unsafe` part of `as_vector`. The idea is to
pass `Iter` (the forward iterator of `Chunk`) to `AsVector` so that it
internally computes `initial_chunk_lengths`. The shape of this data must
no longer be guaranteed by the caller.

This patch goes a bit further: `UpdateToVectorDiff` has
a new constructor which consumes this `Iter` and builds
`initial_chunk_lengths` itself. Even better!

Finally, `Updates::as_vector` is removed. It's clearly no longer
necessary and it was creating borrowing issues anyway with the new code
structure.

crates/matrix-sdk/src/event_cache/linked_chunk/as_vector.rs

@@ -22,21 +22,18 @@ use eyeball_im::VectorDiff;
 
 use super::{
     updates::{ReaderToken, Update, UpdatesInner},
-    ChunkIdentifier,
+    ChunkContent, ChunkIdentifier, Iter,
 };
 
 /// A type alias to represent a chunk's length. This is purely for commodity.
-pub(super) type ChunkLength = usize;
+type ChunkLength = usize;
 
 /// A type that transforms a `Vec<Update<Item, Gap>>` (given by
 /// [`Updates`](super::Update)) into a `Vec<VectorDiff<Item>>` (this
 /// type). Basically, it helps to consume a [`LinkedChunk<CAP, Item, Gap>`] as
 /// if it was an [`eyeball::ObservableVector<Item>`].
 pub struct AsVector<Item, Gap> {
-    /// Weak reference to [`UpdatesInner`].
-    ///
-    /// Using a weak reference allows [`Updates`] to be dropped
-    /// freely even if a subscriber exists.
+    /// Strong reference to [`UpdatesInner`].
     updates: Arc<RwLock<UpdatesInner<Item, Gap>>>,
 
     /// The token to read the updates.
@@ -48,22 +45,12 @@ pub struct AsVector<Item, Gap> {
 
 impl<Item, Gap> AsVector<Item, Gap> {
     /// Create a new [`Self`].
-    ///
-    /// `initial_chunk_lengths` must be pairs of all chunk identifiers with the
-    /// associated chunk length. The pairs must be in the exact same order than
-    /// in [`LinkedChunk`].
-    ///
-    /// # Safety
-    ///
-    /// This method is marked as unsafe only to attract the caller's attention
-    /// on the order of pairs. If the order of pairs are incorrect, the entire
-    /// algorithm will not work properly and is very likely to panic.
-    pub(super) unsafe fn new(
+    pub(super) fn new<const CAP: usize>(
         updates: Arc<RwLock<UpdatesInner<Item, Gap>>>,
         token: ReaderToken,
-        initial_chunk_lengths: VecDeque<(ChunkIdentifier, ChunkLength)>,
+        chunk_iterator: Iter<CAP, Item, Gap>,
     ) -> Self {
-        Self { updates, token, mapper: UpdateToVectorDiff { chunks: initial_chunk_lengths } }
+        Self { updates, token, mapper: UpdateToVectorDiff::new(chunk_iterator) }
     }
 
     /// Take the new updates as [`VectorDiff`].
@@ -89,6 +76,23 @@ struct UpdateToVectorDiff {
 }
 
 impl UpdateToVectorDiff {
+    /// Construct [`Self`].
+    fn new<const CAP: usize, Item, Gap>(chunk_iterator: Iter<CAP, Item, Gap>) -> Self {
+        let mut initial_chunk_lengths = VecDeque::new();
+
+        for chunk in chunk_iterator {
+            initial_chunk_lengths.push_front((
+                chunk.identifier(),
+                match chunk.content() {
+                    ChunkContent::Gap(_) => 0,
+                    ChunkContent::Items(items) => items.len(),
+                },
+            ))
+        }
+
+        Self { chunks: initial_chunk_lengths }
+    }
+
     /// Map several [`Update`] into [`VectorDiff`].
     ///
     /// How does this type transform `Update` into `VectorDiff`? There is no
@@ -97,7 +101,8 @@ impl UpdateToVectorDiff {
     /// computed manually.
     ///
     /// The only buffered data is pairs of [`ChunkIdentifier`] and
-    /// [`ChunkLength`]. The following rules must be respected:
+    /// [`ChunkLength`]. The following rules must be respected (they are defined
+    /// in [`Self::new`]):
     ///
     /// * A chunk of kind [`ChunkContent::Gap`] has a length of 0,
     /// * A chunk of kind [`ChunkContent::Items`] has a length equals to its

crates/matrix-sdk/src/event_cache/linked_chunk/mod.rs

@@ -95,7 +95,6 @@ mod as_vector;
 mod updates;
 
 use std::{
-    collections::VecDeque,
     fmt,
     marker::PhantomData,
     ops::Not,
@@ -742,23 +741,13 @@ impl<const CAP: usize, Item, Gap> LinkedChunk<CAP, Item, Gap> {
     /// been constructed with [`Self::new`], otherwise, if it's been constructed
     /// with [`Self::new_with_update_history`], it returns `Some(…)`.
     pub fn as_vector(&mut self) -> Option<AsVector<Item, Gap>> {
-        let mut initial_chunk_lengths = VecDeque::new();
+        let (updates, token) = self
+            .updates
+            .as_mut()
+            .map(|updates| (updates.inner.clone(), updates.new_reader_token()))?;
+        let chunk_iterator = self.chunks();
 
-        for chunk in self.chunks() {
-            initial_chunk_lengths.push_front((
-                chunk.identifier(),
-                match chunk.content() {
-                    ChunkContent::Gap(_) => 0,
-                    ChunkContent::Items(items) => items.len(),
-                },
-            ))
-        }
-
-        self.updates.as_mut().map(|updates| {
-            // SAFETY: The order of chunk pairs inside `initial_chunk_lengths` is exactly
-            // the same as chunks in `Self`.
-            unsafe { updates.as_vector(initial_chunk_lengths) }
-        })
+        Some(AsVector::new(updates, token, chunk_iterator))
     }
 }
 

crates/matrix-sdk/src/event_cache/linked_chunk/updates.rs

@@ -13,7 +13,7 @@
 // limitations under the License.
 
 use std::{
-    collections::{HashMap, VecDeque},
+    collections::HashMap,
     pin::Pin,
     sync::{Arc, RwLock, Weak},
     task::{Context, Poll, Waker},
@@ -21,7 +21,7 @@ use std::{
 
 use futures_core::Stream;
 
-use super::{AsVector, ChunkIdentifier, ChunkLength, Position};
+use super::{ChunkIdentifier, Position};
 
 /// Represent the updates that have happened inside a [`LinkedChunk`].
 ///
@@ -120,7 +120,7 @@ where
 ///
 /// Get a value for this type with [`LinkedChunk::updates`].
 pub struct Updates<Item, Gap> {
-    inner: Arc<RwLock<UpdatesInner<Item, Gap>>>,
+    pub(super) inner: Arc<RwLock<UpdatesInner<Item, Gap>>>,
 }
 
 /// A token used to represent readers that read the updates in
@@ -279,16 +279,7 @@ impl<Item, Gap> Updates<Item, Gap> {
         UpdatesSubscriber::new(Arc::downgrade(&self.inner), token)
     }
 
-    pub(super) unsafe fn as_vector(
-        &mut self,
-        initial_chunk_lengths: VecDeque<(ChunkIdentifier, ChunkLength)>,
-    ) -> AsVector<Item, Gap> {
-        // An `AsVector` is a new update reader, it needs its own token.
-        let token = self.new_reader_token();
-
-        AsVector::new(Arc::clone(&self.inner), token, initial_chunk_lengths)
-    }
-
+    /// Generate a new [`ReaderToken`].
     pub(super) fn new_reader_token(&mut self) -> ReaderToken {
         let mut inner = self.inner.write().unwrap();