feat(sdk): Mutation-based testing and property-based testing for LinkedChunk

feat(sdk): Mutation-based testing and property-based testing for `LinkedChunk`

Author: Hywan

Cargo.lock

@@ -151,6 +151,7 @@ tracing-subscriber = { version = "0.3.11", features = ["env-filter"] }
 wasm-bindgen-test = "0.3.33"
 
 [target.'cfg(not(target_arch = "wasm32"))'.dev-dependencies]
+proptest = "1.4.0"
 tokio = { workspace = true, features = ["rt-multi-thread", "macros"] }
 wiremock = { workspace = true }
 

crates/matrix-sdk/Cargo.toml

@@ -33,6 +33,7 @@ type ChunkLength = usize;
 /// `Vec<VectorDiff<Item>>` (this type). Basically, it helps to consume a
 /// [`LinkedChunk<CAP, Item, Gap>`](super::LinkedChunk) as if it was an
 /// [`eyeball_im::ObservableVector<Item>`].
+#[derive(Debug)]
 pub struct AsVector<Item, Gap> {
     /// Strong reference to [`UpdatesInner`].
     updates: Arc<RwLock<UpdatesInner<Item, Gap>>>,
@@ -58,14 +59,20 @@ impl<Item, Gap> AsVector<Item, Gap> {
         token: ReaderToken,
         chunk_iterator: Iter<'_, CAP, Item, Gap>,
     ) -> Self {
+        // Drain previous updates so that this type is synced with `Updates`.
+        {
+            let mut updates = updates.write().unwrap();
+            let _ = updates.take_with_token(token);
+        }
+
         Self { updates, token, mapper: UpdateToVectorDiff::new(chunk_iterator) }
     }
 
     /// Take the new updates as [`VectorDiff`].
     ///
     /// It returns an empty `Vec` if there is no new `VectorDiff` for the
     /// moment.
-    pub(super) fn take(&mut self) -> Vec<VectorDiff<Item>>
+    pub fn take(&mut self) -> Vec<VectorDiff<Item>>
     where
         Item: Clone,
     {
@@ -76,6 +83,7 @@ impl<Item, Gap> AsVector<Item, Gap> {
 }
 
 /// Internal type that converts [`Update`] into [`VectorDiff`].
+#[derive(Debug)]
 struct UpdateToVectorDiff {
     /// Pairs of all known chunks and their respective length. This is the only
     /// required data for this algorithm.
@@ -581,4 +589,105 @@ mod tests {
             ]
         );
     }
+
+    #[test]
+    fn updates_are_drained_when_constructing_as_vector() {
+        let mut linked_chunk = LinkedChunk::<10, char, ()>::new_with_update_history();
+
+        linked_chunk.push_items_back(['a']);
+
+        let mut as_vector = linked_chunk.as_vector().unwrap();
+        let diffs = as_vector.take();
+
+        // `diffs` are empty because `AsVector` is built _after_ `LinkedChunk`
+        // has been updated.
+        assert!(diffs.is_empty());
+
+        linked_chunk.push_items_back(['b']);
+
+        let diffs = as_vector.take();
+
+        // `diffs` is not empty because new updates are coming.
+        assert_eq!(diffs.len(), 1);
+    }
+
+    #[cfg(not(target_arch = "wasm32"))]
+    mod proptests {
+        use proptest::prelude::*;
+
+        use super::*;
+
+        #[derive(Debug, Clone)]
+        enum AsVectorOperation {
+            PushItems { items: Vec<char> },
+            PushGap,
+            ReplaceLastGap { items: Vec<char> },
+        }
+
+        fn as_vector_operation_strategy() -> impl Strategy<Value = AsVectorOperation> {
+            prop_oneof![
+                3 => prop::collection::vec(prop::char::ranges(vec!['a'..='z', 'A'..='Z'].into()), 0..=25)
+                    .prop_map(|items| AsVectorOperation::PushItems { items }),
+
+                2 => Just(AsVectorOperation::PushGap),
+
+                1 => prop::collection::vec(prop::char::ranges(vec!['a'..='z', 'A'..='Z'].into()), 0..=25)
+                    .prop_map(|items| AsVectorOperation::ReplaceLastGap { items }),
+            ]
+        }
+
+        proptest! {
+            #[test]
+            fn as_vector_is_correct(
+                operations in prop::collection::vec(as_vector_operation_strategy(), 10..=50)
+            ) {
+                let mut linked_chunk = LinkedChunk::<10, char, ()>::new_with_update_history();
+                let mut as_vector = linked_chunk.as_vector().unwrap();
+
+                for operation in operations {
+                    match operation {
+                        AsVectorOperation::PushItems { items } => {
+                            linked_chunk.push_items_back(items);
+                        }
+
+                        AsVectorOperation::PushGap => {
+                            linked_chunk.push_gap_back(());
+                        }
+
+                        AsVectorOperation::ReplaceLastGap { items } => {
+                            let Some(gap_identifier) = linked_chunk
+                                .rchunks()
+                                .find_map(|chunk| chunk.is_gap().then_some(chunk.identifier()))
+                            else {
+                                continue;
+                            };
+
+                            linked_chunk.replace_gap_at(items, gap_identifier).unwrap();
+                        }
+                    }
+                }
+
+                let mut vector_from_diffs = Vec::new();
+
+                // Read all updates as `VectorDiff` and rebuild a `Vec<char>`.
+                for diff in as_vector.take() {
+                    match diff {
+                        VectorDiff::Insert { index, value } => vector_from_diffs.insert(index, value),
+                        VectorDiff::Append { values } => {
+                            let mut values = values.iter().copied().collect();
+
+                            vector_from_diffs.append(&mut values);
+                        }
+                        _ => unreachable!(),
+                    }
+                }
+
+                // Iterate over all chunks and collect items as `Vec<char>`.
+                let vector_from_chunks = linked_chunk.items().map(|(_, item)| *item).collect::<Vec<_>>();
+
+                // Compare both `Vec`s.
+                assert_eq!(vector_from_diffs, vector_from_chunks);
+            }
+        }
+    }
 }

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

@@ -216,6 +216,12 @@ pub struct LinkedChunk<const CHUNK_CAPACITY: usize, Item, Gap> {
     marker: PhantomData<Box<Chunk<CHUNK_CAPACITY, Item, Gap>>>,
 }
 
+impl<const CAP: usize, Item, Gap> Default for LinkedChunk<CAP, Item, Gap> {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 impl<const CAP: usize, Item, Gap> LinkedChunk<CAP, Item, Gap> {
     /// Create a new [`Self`].
     pub fn new() -> Self {
@@ -252,6 +258,7 @@ impl<const CAP: usize, Item, Gap> LinkedChunk<CAP, Item, Gap> {
     }
 
     /// Get the number of items in this linked chunk.
+    #[allow(clippy::len_without_is_empty)]
     pub fn len(&self) -> usize {
         self.length
     }
@@ -865,6 +872,7 @@ impl Position {
 
 /// An iterator over a [`LinkedChunk`] that traverses the chunk in backward
 /// direction (i.e. it calls `previous` on each chunk to make progress).
+#[derive(Debug)]
 pub struct IterBackward<'a, const CAP: usize, Item, Gap> {
     chunk: Option<&'a Chunk<CAP, Item, Gap>>,
 }
@@ -890,6 +898,7 @@ impl<'a, const CAP: usize, Item, Gap> Iterator for IterBackward<'a, CAP, Item, G
 
 /// An iterator over a [`LinkedChunk`] that traverses the chunk in forward
 /// direction (i.e. it calls `next` on each chunk to make progress).
+#[derive(Debug)]
 pub struct Iter<'a, const CAP: usize, Item, Gap> {
     chunk: Option<&'a Chunk<CAP, Item, Gap>>,
 }
@@ -1248,6 +1257,8 @@ where
 
 #[cfg(test)]
 mod tests {
+    use std::ops::Not;
+
     use assert_matches::assert_matches;
 
     use super::{
@@ -2151,4 +2162,23 @@ mod tests {
             assert_eq!(chunk.last_position(), Position(ChunkIdentifier(3), 0));
         }
     }
+
+    #[test]
+    fn test_is_first_and_last_chunk() {
+        let mut linked_chunk = LinkedChunk::<3, char, ()>::new();
+
+        let mut chunks = linked_chunk.chunks().peekable();
+        assert!(chunks.peek().unwrap().is_first_chunk());
+        assert!(chunks.next().unwrap().is_last_chunk());
+        assert!(chunks.next().is_none());
+
+        linked_chunk.push_items_back(['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h']);
+
+        let mut chunks = linked_chunk.chunks().peekable();
+        assert!(chunks.next().unwrap().is_first_chunk());
+        assert!(chunks.peek().unwrap().is_first_chunk().not());
+        assert!(chunks.next().unwrap().is_last_chunk().not());
+        assert!(chunks.next().unwrap().is_last_chunk());
+        assert!(chunks.next().is_none());
+    }
 }

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

@@ -95,6 +95,7 @@ pub enum Update<Item, Gap> {
 /// A collection of [`Update`]s that can be observed.
 ///
 /// Get a value for this type with [`LinkedChunk::updates`].
+#[derive(Debug)]
 pub struct ObservableUpdates<Item, Gap> {
     pub(super) inner: Arc<RwLock<UpdatesInner<Item, Gap>>>,
 }
@@ -164,6 +165,7 @@ pub(super) type ReaderToken = usize;
 /// for example with [`UpdatesSubscriber`]. Of course, they can be multiple
 /// `UpdatesSubscriber`s at the same time. Hence the need of supporting multiple
 /// readers.
+#[derive(Debug)]
 pub(super) struct UpdatesInner<Item, Gap> {
     /// All the updates that have not been read by all readers.
     updates: Vec<Update<Item, Gap>>,