From 56a6b5e7ec757de21468177800aad1b1532c87fb Mon Sep 17 00:00:00 2001
From: Ivan Enderlin <ivan@mnt.io>
Date: Wed, 20 Nov 2024 16:06:16 +0100
Subject: [PATCH 1/9] feat(common): Implement `RelationalLinkedChunk`.

A `RelationalLinkedChunk` is like a `LinkedChunk` but with a relational
layout, similar to what we would have in a database.

This is used by memory stores. The idea is to have a data layout that
is similar for memory stores and for relational database stores, to
represent a `LinkedChunk`.

This type is also designed to receive `Update`. Applying `Update`s
directly on a `LinkedChunk` is not ideal and particularly not trivial
as the `Update`s do _not_ match the internal data layout of the
`LinkedChunk`, they have been designed for storages, like a relational
database for example.

This type is not as performant as `LinkedChunk` (in terms of memory
layout, CPU caches etc.). It is only designed to be used in memory
stores, which are mostly used for test purposes or light usages of the
SDK.
---
 .../matrix-sdk-common/src/linked_chunk/mod.rs |  15 +-
 .../src/linked_chunk/relational.rs            | 450 ++++++++++++++++++
 2 files changed, 463 insertions(+), 2 deletions(-)
 create mode 100644 crates/matrix-sdk-common/src/linked_chunk/relational.rs

diff --git a/crates/matrix-sdk-common/src/linked_chunk/mod.rs b/crates/matrix-sdk-common/src/linked_chunk/mod.rs
index eeba29be302..a250cbab4d6 100644
--- a/crates/matrix-sdk-common/src/linked_chunk/mod.rs
+++ b/crates/matrix-sdk-common/src/linked_chunk/mod.rs
@@ -93,6 +93,7 @@ macro_rules! assert_items_eq {
 }
 
 mod as_vector;
+pub mod relational;
 mod updates;
 
 use std::{
@@ -933,7 +934,7 @@ impl ChunkIdentifierGenerator {
 /// Learn more with [`ChunkIdentifierGenerator`].
 #[derive(Copy, Clone, Debug, PartialEq)]
 #[repr(transparent)]
-pub struct ChunkIdentifier(u64);
+pub struct ChunkIdentifier(pub(super) u64);
 
 impl PartialEq<u64> for ChunkIdentifier {
     fn eq(&self, other: &u64) -> bool {
@@ -945,7 +946,7 @@ impl PartialEq<u64> for ChunkIdentifier {
 ///
 /// It's a pair of a chunk position and an item index.
 #[derive(Copy, Clone, Debug, PartialEq)]
-pub struct Position(ChunkIdentifier, usize);
+pub struct Position(pub(super) ChunkIdentifier, pub(super) usize);
 
 impl Position {
     /// Get the chunk identifier of the item.
@@ -966,6 +967,16 @@ impl Position {
     pub fn decrement_index(&mut self) {
         self.1 = self.1.checked_sub(1).expect("Cannot decrement the index because it's already 0");
     }
+
+    /// Increment the index part (see [`Self::index`]), i.e. add 1.
+    ///
+    /// # Panic
+    ///
+    /// This method will panic if it will overflow, i.e. if the index is larger
+    /// than `usize::MAX`.
+    pub fn increment_index(&mut self) {
+        self.1 = self.1.checked_add(1).expect("Cannot increment the index because it's too large");
+    }
 }
 
 /// An iterator over a [`LinkedChunk`] that traverses the chunk in backward
diff --git a/crates/matrix-sdk-common/src/linked_chunk/relational.rs b/crates/matrix-sdk-common/src/linked_chunk/relational.rs
new file mode 100644
index 00000000000..f4d528bff23
--- /dev/null
+++ b/crates/matrix-sdk-common/src/linked_chunk/relational.rs
@@ -0,0 +1,450 @@
+// Copyright 2024 The Matrix.org Foundation C.I.C.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//! Implementation for a _relational linked chunk_, see
+//! [`RelationalLinkedChunk`].
+
+use crate::linked_chunk::{ChunkIdentifier, Position, Update};
+
+/// A row of the [`RelationalLinkedChunk::chunks`].
+#[derive(Debug, PartialEq)]
+struct ChunkRow {
+    previous_chunk: Option<ChunkIdentifier>,
+    chunk: ChunkIdentifier,
+    next_chunk: Option<ChunkIdentifier>,
+}
+
+/// A row of the [`RelationalLinkedChunk::items`].
+#[derive(Debug, PartialEq)]
+struct ItemRow<Item, Gap> {
+    position: Position,
+    item: Either<Item, Gap>,
+}
+
+/// Kind of item.
+#[derive(Debug, PartialEq)]
+enum Either<Item, Gap> {
+    /// The content is an item.
+    Item(Item),
+
+    /// The content is a gap.
+    Gap(Gap),
+}
+
+/// A [`LinkedChunk`] but with a relational layout, similar to what we
+/// would have in a database.
+///
+/// This is used by memory stores. The idea is to have a data layout that is
+/// similar for memory stores and for relational database stores, to represent a
+/// [`LinkedChunk`].
+///
+/// This type is also designed to receive [`Update`]. Applying `Update`s
+/// directly on a [`LinkedChunk`] is not ideal and particularly not trivial as
+/// the `Update`s do _not_ match the internal data layout of the `LinkedChunk`,
+/// they are been designed for storages, like a relational database for example.
+///
+/// This type is not as performant as [`LinkedChunk`] (in terms of memory
+/// layout, CPU caches etc.). It is only designed to be used in memory stores,
+/// which are mostly used for test purposes or light usage of the SDK.
+///
+/// [`LinkedChunk`]: super::LinkedChunk
+#[derive(Debug)]
+pub struct RelationalLinkedChunk<Item, Gap> {
+    /// Chunks.
+    chunks: Vec<ChunkRow>,
+
+    /// Items.
+    items: Vec<ItemRow<Item, Gap>>,
+}
+
+impl<Item, Gap> RelationalLinkedChunk<Item, Gap> {
+    /// Create a new relational linked chunk.
+    pub fn new() -> Self {
+        Self { chunks: Vec::new(), items: Vec::new() }
+    }
+
+    /// Apply [`Update`]s. That's the only way to write data inside this
+    /// relational linked chunk.
+    pub fn apply_updates(&mut self, updates: &[Update<Item, Gap>])
+    where
+        Item: Clone,
+        Gap: Clone,
+    {
+        for update in updates {
+            match update {
+                Update::NewItemsChunk { previous, new, next } => {
+                    insert_chunk(&mut self.chunks, previous, new, next);
+                }
+
+                Update::NewGapChunk { previous, new, next, gap } => {
+                    insert_chunk(&mut self.chunks, previous, new, next);
+                    self.items.push(ItemRow {
+                        position: Position(*new, 0),
+                        item: Either::Gap(gap.clone()),
+                    });
+                }
+
+                Update::RemoveChunk(chunk_identifier) => {
+                    remove_chunk(&mut self.chunks, chunk_identifier);
+
+                    let indices_to_remove = self
+                        .items
+                        .iter()
+                        .enumerate()
+                        .filter_map(|(nth, ItemRow { position, .. })| {
+                            (position.chunk_identifier() == *chunk_identifier).then_some(nth)
+                        })
+                        .collect::<Vec<_>>();
+
+                    for index_to_remove in indices_to_remove.into_iter().rev() {
+                        self.items.remove(index_to_remove);
+                    }
+                }
+
+                Update::PushItems { at, items } => {
+                    let mut at = *at;
+
+                    for item in items {
+                        self.items.push(ItemRow { position: at, item: Either::Item(item.clone()) });
+                        at.increment_index();
+                    }
+                }
+
+                Update::RemoveItem { at } => {
+                    let mut entry_to_remove = None;
+
+                    for (nth, ItemRow { position, .. }) in self.items.iter_mut().enumerate() {
+                        // Find the item to remove.
+                        if position == at {
+                            debug_assert!(entry_to_remove.is_none(), "Found the same entry twice");
+
+                            entry_to_remove = Some(nth);
+                        }
+
+                        // Update all items that come _after_ `at` to shift their index.
+                        if position.chunk_identifier() == at.chunk_identifier()
+                            && position.index() > at.index()
+                        {
+                            position.decrement_index();
+                        }
+                    }
+
+                    self.items.remove(entry_to_remove.expect("Remove an unknown item"));
+                }
+
+                Update::DetachLastItems { at } => {
+                    let indices_to_remove = self
+                        .items
+                        .iter()
+                        .enumerate()
+                        .filter_map(|(nth, ItemRow { position, .. })| {
+                            (position.chunk_identifier() == at.chunk_identifier()
+                                && position.index() >= at.index())
+                            .then_some(nth)
+                        })
+                        .collect::<Vec<_>>();
+
+                    for index_to_remove in indices_to_remove.into_iter().rev() {
+                        self.items.remove(index_to_remove);
+                    }
+                }
+
+                Update::StartReattachItems | Update::EndReattachItems => { /* nothing */ }
+            }
+        }
+
+        fn insert_chunk(
+            chunks: &mut Vec<ChunkRow>,
+            previous: &Option<ChunkIdentifier>,
+            new: &ChunkIdentifier,
+            next: &Option<ChunkIdentifier>,
+        ) {
+            // Find the previous chunk, and update its next chunk.
+            if let Some(previous) = previous {
+                let entry_for_previous_chunk = chunks
+                    .iter_mut()
+                    .find(|ChunkRow { chunk, .. }| chunk == previous)
+                    .expect("Previous chunk should be present");
+
+                // Insert the chunk.
+                entry_for_previous_chunk.next_chunk = Some(*new);
+            }
+
+            // Find the next chunk, and update its previous chunk.
+            if let Some(next) = next {
+                let entry_for_next_chunk = chunks
+                    .iter_mut()
+                    .find(|ChunkRow { chunk, .. }| chunk == next)
+                    .expect("Next chunk should be present");
+
+                // Insert the chunk.
+                entry_for_next_chunk.previous_chunk = Some(*new);
+            }
+
+            // Insert the chunk.
+            chunks.push(ChunkRow { previous_chunk: *previous, chunk: *new, next_chunk: *next });
+        }
+
+        fn remove_chunk(chunks: &mut Vec<ChunkRow>, chunk_to_remove: &ChunkIdentifier) {
+            let entry_nth_to_remove = chunks
+                .iter()
+                .enumerate()
+                .find_map(|(nth, ChunkRow { chunk, .. })| (chunk == chunk_to_remove).then_some(nth))
+                .expect("Remove an unknown chunk");
+
+            let ChunkRow { previous_chunk: previous, next_chunk: next, .. } =
+                chunks.remove(entry_nth_to_remove);
+
+            // Find the previous chunk, and update its next chunk.
+            if let Some(previous) = previous {
+                let entry_for_previous_chunk = chunks
+                    .iter_mut()
+                    .find(|ChunkRow { chunk, .. }| *chunk == previous)
+                    .expect("Previous chunk should be present");
+
+                // Insert the chunk.
+                entry_for_previous_chunk.next_chunk = next;
+            }
+
+            // Find the next chunk, and update its previous chunk.
+            if let Some(next) = next {
+                let entry_for_next_chunk = chunks
+                    .iter_mut()
+                    .find(|ChunkRow { chunk, .. }| *chunk == next)
+                    .expect("Next chunk should be present");
+
+                // Insert the chunk.
+                entry_for_next_chunk.previous_chunk = previous;
+            }
+        }
+    }
+}
+
+impl<Item, Gap> Default for RelationalLinkedChunk<Item, Gap> {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::{ChunkIdentifier as CId, *};
+
+    #[test]
+    fn test_new_items_chunk() {
+        let mut relational_linked_chunk = RelationalLinkedChunk::<char, ()>::new();
+
+        relational_linked_chunk.apply_updates(&[
+            // 0
+            Update::NewItemsChunk { previous: None, new: CId(0), next: None },
+            // 1 after 0
+            Update::NewItemsChunk { previous: Some(CId(0)), new: CId(1), next: None },
+            // 2 before 0
+            Update::NewItemsChunk { previous: None, new: CId(2), next: Some(CId(0)) },
+            // 3 between 2 and 0
+            Update::NewItemsChunk { previous: Some(CId(2)), new: CId(3), next: Some(CId(0)) },
+        ]);
+
+        // Chunks are correctly linked.
+        assert_eq!(
+            relational_linked_chunk.chunks,
+            &[
+                ChunkRow { previous_chunk: Some(CId(3)), chunk: CId(0), next_chunk: Some(CId(1)) },
+                ChunkRow { previous_chunk: Some(CId(0)), chunk: CId(1), next_chunk: None },
+                ChunkRow { previous_chunk: None, chunk: CId(2), next_chunk: Some(CId(3)) },
+                ChunkRow { previous_chunk: Some(CId(2)), chunk: CId(3), next_chunk: Some(CId(0)) },
+            ],
+        );
+        // Items have not been modified.
+        assert!(relational_linked_chunk.items.is_empty());
+    }
+
+    #[test]
+    fn test_new_gap_chunk() {
+        let mut relational_linked_chunk = RelationalLinkedChunk::<char, ()>::new();
+
+        relational_linked_chunk.apply_updates(&[
+            // 0
+            Update::NewItemsChunk { previous: None, new: CId(0), next: None },
+            // 1 after 0
+            Update::NewGapChunk { previous: Some(CId(0)), new: CId(1), next: None, gap: () },
+            // 2 after 1
+            Update::NewItemsChunk { previous: Some(CId(1)), new: CId(2), next: None },
+        ]);
+
+        // Chunks are correctly links.
+        assert_eq!(
+            relational_linked_chunk.chunks,
+            &[
+                ChunkRow { previous_chunk: None, chunk: CId(0), next_chunk: Some(CId(1)) },
+                ChunkRow { previous_chunk: Some(CId(0)), chunk: CId(1), next_chunk: Some(CId(2)) },
+                ChunkRow { previous_chunk: Some(CId(1)), chunk: CId(2), next_chunk: None },
+            ],
+        );
+        // Items contains the gap.
+        assert_eq!(
+            relational_linked_chunk.items,
+            &[ItemRow { position: Position(CId(1), 0), item: Either::Gap(()) }],
+        );
+    }
+
+    #[test]
+    fn test_remove_chunk() {
+        let mut relational_linked_chunk = RelationalLinkedChunk::<char, ()>::new();
+
+        relational_linked_chunk.apply_updates(&[
+            // 0
+            Update::NewItemsChunk { previous: None, new: CId(0), next: None },
+            // 1 after 0
+            Update::NewGapChunk { previous: Some(CId(0)), new: CId(1), next: None, gap: () },
+            // 2 after 1
+            Update::NewItemsChunk { previous: Some(CId(1)), new: CId(2), next: None },
+            // remove 1
+            Update::RemoveChunk(CId(1)),
+        ]);
+
+        // Chunks are correctly links.
+        assert_eq!(
+            relational_linked_chunk.chunks,
+            &[
+                ChunkRow { previous_chunk: None, chunk: CId(0), next_chunk: Some(CId(2)) },
+                ChunkRow { previous_chunk: Some(CId(0)), chunk: CId(2), next_chunk: None },
+            ],
+        );
+        // Items no longer contains the gap.
+        assert!(relational_linked_chunk.items.is_empty());
+    }
+
+    #[test]
+    fn test_push_items() {
+        let mut relational_linked_chunk = RelationalLinkedChunk::<char, ()>::new();
+
+        relational_linked_chunk.apply_updates(&[
+            // new chunk (this is not mandatory for this test, but let's try to be realistic)
+            Update::NewItemsChunk { previous: None, new: CId(0), next: None },
+            // new items on 0
+            Update::PushItems { at: Position(CId(0), 0), items: vec!['a', 'b', 'c'] },
+            // new chunk (to test new items are pushed in the correct chunk)
+            Update::NewItemsChunk { previous: Some(CId(0)), new: CId(1), next: None },
+            // new items on 1
+            Update::PushItems { at: Position(CId(1), 0), items: vec!['x', 'y', 'z'] },
+            // new items on 0 again
+            Update::PushItems { at: Position(CId(0), 3), items: vec!['d', 'e'] },
+        ]);
+
+        // Chunks are correctly links.
+        assert_eq!(
+            relational_linked_chunk.chunks,
+            &[
+                ChunkRow { previous_chunk: None, chunk: CId(0), next_chunk: Some(CId(1)) },
+                ChunkRow { previous_chunk: Some(CId(0)), chunk: CId(1), next_chunk: None },
+            ],
+        );
+        // Items contains the pushed items.
+        assert_eq!(
+            relational_linked_chunk.items,
+            &[
+                ItemRow { position: Position(CId(0), 0), item: Either::Item('a') },
+                ItemRow { position: Position(CId(0), 1), item: Either::Item('b') },
+                ItemRow { position: Position(CId(0), 2), item: Either::Item('c') },
+                ItemRow { position: Position(CId(1), 0), item: Either::Item('x') },
+                ItemRow { position: Position(CId(1), 1), item: Either::Item('y') },
+                ItemRow { position: Position(CId(1), 2), item: Either::Item('z') },
+                ItemRow { position: Position(CId(0), 3), item: Either::Item('d') },
+                ItemRow { position: Position(CId(0), 4), item: Either::Item('e') },
+            ],
+        );
+    }
+
+    #[test]
+    fn test_remove_item() {
+        let mut relational_linked_chunk = RelationalLinkedChunk::<char, ()>::new();
+
+        relational_linked_chunk.apply_updates(&[
+            // new chunk (this is not mandatory for this test, but let's try to be realistic)
+            Update::NewItemsChunk { previous: None, new: CId(0), next: None },
+            // new items on 0
+            Update::PushItems { at: Position(CId(0), 0), items: vec!['a', 'b', 'c', 'd', 'e'] },
+            // remove an item: 'a'
+            Update::RemoveItem { at: Position(CId(0), 0) },
+            // remove an item: 'd'
+            Update::RemoveItem { at: Position(CId(0), 2) },
+        ]);
+
+        // Chunks are correctly links.
+        assert_eq!(
+            relational_linked_chunk.chunks,
+            &[ChunkRow { previous_chunk: None, chunk: CId(0), next_chunk: None }],
+        );
+        // Items contains the pushed items.
+        assert_eq!(
+            relational_linked_chunk.items,
+            &[
+                ItemRow { position: Position(CId(0), 0), item: Either::Item('b') },
+                ItemRow { position: Position(CId(0), 1), item: Either::Item('c') },
+                ItemRow { position: Position(CId(0), 2), item: Either::Item('e') },
+            ],
+        );
+    }
+
+    #[test]
+    fn test_detach_last_items() {
+        let mut relational_linked_chunk = RelationalLinkedChunk::<char, ()>::new();
+
+        relational_linked_chunk.apply_updates(&[
+            // new chunk
+            Update::NewItemsChunk { previous: None, new: CId(0), next: None },
+            // new chunk
+            Update::NewItemsChunk { previous: Some(CId(0)), new: CId(1), next: None },
+            // new items on 0
+            Update::PushItems { at: Position(CId(0), 0), items: vec!['a', 'b', 'c', 'd', 'e'] },
+            // new items on 1
+            Update::PushItems { at: Position(CId(1), 0), items: vec!['x', 'y', 'z'] },
+            // detach last items on 0
+            Update::DetachLastItems { at: Position(CId(0), 2) },
+        ]);
+
+        // Chunks are correctly links.
+        assert_eq!(
+            relational_linked_chunk.chunks,
+            &[
+                ChunkRow { previous_chunk: None, chunk: CId(0), next_chunk: Some(CId(1)) },
+                ChunkRow { previous_chunk: Some(CId(0)), chunk: CId(1), next_chunk: None },
+            ],
+        );
+        // Items contains the pushed items.
+        assert_eq!(
+            relational_linked_chunk.items,
+            &[
+                ItemRow { position: Position(CId(0), 0), item: Either::Item('a') },
+                ItemRow { position: Position(CId(0), 1), item: Either::Item('b') },
+                ItemRow { position: Position(CId(1), 0), item: Either::Item('x') },
+                ItemRow { position: Position(CId(1), 1), item: Either::Item('y') },
+                ItemRow { position: Position(CId(1), 2), item: Either::Item('z') },
+            ],
+        );
+    }
+
+    #[test]
+    fn test_start_and_end_reattach_items() {
+        let mut relational_linked_chunk = RelationalLinkedChunk::<char, ()>::new();
+
+        relational_linked_chunk
+            .apply_updates(&[Update::StartReattachItems, Update::EndReattachItems]);
+
+        // Nothing happened.
+        assert!(relational_linked_chunk.chunks.is_empty());
+        assert!(relational_linked_chunk.items.is_empty());
+    }
+}

From 676155a5a8c63a6a4d4c47a8fab50dd869e3abf4 Mon Sep 17 00:00:00 2001
From: Ivan Enderlin <ivan@mnt.io>
Date: Wed, 20 Nov 2024 16:09:03 +0100
Subject: [PATCH 2/9] feat(base): `MemoryStore` uses `RelationalLinkedChunk` to
 store events.

That's it.
---
 .../src/event_cache/store/memory_store.rs             | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/crates/matrix-sdk-base/src/event_cache/store/memory_store.rs b/crates/matrix-sdk-base/src/event_cache/store/memory_store.rs
index bd764cad0db..8b8b14356b0 100644
--- a/crates/matrix-sdk-base/src/event_cache/store/memory_store.rs
+++ b/crates/matrix-sdk-base/src/event_cache/store/memory_store.rs
@@ -16,7 +16,8 @@ use std::{collections::HashMap, num::NonZeroUsize, sync::RwLock as StdRwLock, ti
 
 use async_trait::async_trait;
 use matrix_sdk_common::{
-    linked_chunk::Update, ring_buffer::RingBuffer,
+    linked_chunk::{relational::RelationalLinkedChunk, Update},
+    ring_buffer::RingBuffer,
     store_locks::memory_store_helper::try_take_leased_lock,
 };
 use ruma::{MxcUri, OwnedMxcUri};
@@ -35,6 +36,7 @@ use crate::{
 pub struct MemoryStore {
     media: StdRwLock<RingBuffer<(OwnedMxcUri, String /* unique key */, Vec<u8>)>>,
     leases: StdRwLock<HashMap<String, (String, Instant)>>,
+    events: StdRwLock<RelationalLinkedChunk<Event, Gap>>,
 }
 
 // SAFETY: `new_unchecked` is safe because 20 is not zero.
@@ -45,6 +47,7 @@ impl Default for MemoryStore {
         Self {
             media: StdRwLock::new(RingBuffer::new(NUMBER_OF_MEDIAS)),
             leases: Default::default(),
+            events: StdRwLock::new(RelationalLinkedChunk::new()),
         }
     }
 }
@@ -72,9 +75,11 @@ impl EventCacheStore for MemoryStore {
 
     async fn handle_linked_chunk_updates(
         &self,
-        _updates: &[Update<Event, Gap>],
+        updates: &[Update<Event, Gap>],
     ) -> Result<(), Self::Error> {
-        todo!()
+        self.events.write().unwrap().apply_updates(updates);
+
+        Ok(())
     }
 
     async fn add_media_content(

From f063296b3757ab552d0bead2331df3c4eb81cebd Mon Sep 17 00:00:00 2001
From: Ivan Enderlin <ivan@mnt.io>
Date: Wed, 20 Nov 2024 16:13:50 +0100
Subject: [PATCH 3/9] doc(common): Fix a typo.

---
 crates/matrix-sdk-common/src/linked_chunk/mod.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/crates/matrix-sdk-common/src/linked_chunk/mod.rs b/crates/matrix-sdk-common/src/linked_chunk/mod.rs
index a250cbab4d6..479474c83e1 100644
--- a/crates/matrix-sdk-common/src/linked_chunk/mod.rs
+++ b/crates/matrix-sdk-common/src/linked_chunk/mod.rs
@@ -872,12 +872,12 @@ impl<const CAP: usize, Item, Gap> Drop for LinkedChunk<CAP, Item, Gap> {
 }
 
 /// A [`LinkedChunk`] can be safely sent over thread boundaries if `Item: Send`
-/// and `Gap: Send`. The only unsafe part if around the `NonNull`, but the API
+/// and `Gap: Send`. The only unsafe part is around the `NonNull`, but the API
 /// and the lifetimes to deref them are designed safely.
 unsafe impl<const CAP: usize, Item: Send, Gap: Send> Send for LinkedChunk<CAP, Item, Gap> {}
 
 /// A [`LinkedChunk`] can be safely share between threads if `Item: Sync` and
-/// `Gap: Sync`. The only unsafe part if around the `NonNull`, but the API and
+/// `Gap: Sync`. The only unsafe part is around the `NonNull`, but the API and
 /// the lifetimes to deref them are designed safely.
 unsafe impl<const CAP: usize, Item: Sync, Gap: Sync> Sync for LinkedChunk<CAP, Item, Gap> {}
 

From fa96c95c3743429cd86a52cbd9b708fe95aa1902 Mon Sep 17 00:00:00 2001
From: Ivan Enderlin <ivan@mnt.io>
Date: Mon, 25 Nov 2024 09:51:25 +0100
Subject: [PATCH 4/9] feat(common):
 `EventCacheStore::handle_linked_chunk_updates` takes a `&RoomId`.

---
 .../src/event_cache/store/memory_store.rs     |   5 +-
 .../src/event_cache/store/traits.rs           |   6 +-
 .../src/linked_chunk/relational.rs            | 157 +++++++++++-------
 .../src/event_cache_store.rs                  |   3 +-
 4 files changed, 102 insertions(+), 69 deletions(-)

diff --git a/crates/matrix-sdk-base/src/event_cache/store/memory_store.rs b/crates/matrix-sdk-base/src/event_cache/store/memory_store.rs
index 8b8b14356b0..c7cf9f7501f 100644
--- a/crates/matrix-sdk-base/src/event_cache/store/memory_store.rs
+++ b/crates/matrix-sdk-base/src/event_cache/store/memory_store.rs
@@ -20,7 +20,7 @@ use matrix_sdk_common::{
     ring_buffer::RingBuffer,
     store_locks::memory_store_helper::try_take_leased_lock,
 };
-use ruma::{MxcUri, OwnedMxcUri};
+use ruma::{MxcUri, OwnedMxcUri, RoomId};
 
 use super::{EventCacheStore, EventCacheStoreError, Result};
 use crate::{
@@ -75,9 +75,10 @@ impl EventCacheStore for MemoryStore {
 
     async fn handle_linked_chunk_updates(
         &self,
+        room_id: &RoomId,
         updates: &[Update<Event, Gap>],
     ) -> Result<(), Self::Error> {
-        self.events.write().unwrap().apply_updates(updates);
+        self.events.write().unwrap().apply_updates(room_id, updates);
 
         Ok(())
     }
diff --git a/crates/matrix-sdk-base/src/event_cache/store/traits.rs b/crates/matrix-sdk-base/src/event_cache/store/traits.rs
index dde80830973..2584b8881ed 100644
--- a/crates/matrix-sdk-base/src/event_cache/store/traits.rs
+++ b/crates/matrix-sdk-base/src/event_cache/store/traits.rs
@@ -16,7 +16,7 @@ use std::{fmt, sync::Arc};
 
 use async_trait::async_trait;
 use matrix_sdk_common::{linked_chunk::Update, AsyncTraitDeps};
-use ruma::MxcUri;
+use ruma::{MxcUri, RoomId};
 
 use super::EventCacheStoreError;
 use crate::{
@@ -45,6 +45,7 @@ pub trait EventCacheStore: AsyncTraitDeps {
     /// in-memory. This method aims at forwarding this update inside this store.
     async fn handle_linked_chunk_updates(
         &self,
+        room_id: &RoomId,
         updates: &[Update<Event, Gap>],
     ) -> Result<(), Self::Error>;
 
@@ -144,9 +145,10 @@ impl<T: EventCacheStore> EventCacheStore for EraseEventCacheStoreError<T> {
 
     async fn handle_linked_chunk_updates(
         &self,
+        room_id: &RoomId,
         updates: &[Update<Event, Gap>],
     ) -> Result<(), Self::Error> {
-        self.0.handle_linked_chunk_updates(updates).await.map_err(Into::into)
+        self.0.handle_linked_chunk_updates(room_id, updates).await.map_err(Into::into)
     }
 
     async fn add_media_content(
diff --git a/crates/matrix-sdk-common/src/linked_chunk/relational.rs b/crates/matrix-sdk-common/src/linked_chunk/relational.rs
index f4d528bff23..d410b2fa8db 100644
--- a/crates/matrix-sdk-common/src/linked_chunk/relational.rs
+++ b/crates/matrix-sdk-common/src/linked_chunk/relational.rs
@@ -15,6 +15,8 @@
 //! Implementation for a _relational linked chunk_, see
 //! [`RelationalLinkedChunk`].
 
+use ruma::RoomId;
+
 use crate::linked_chunk::{ChunkIdentifier, Position, Update};
 
 /// A row of the [`RelationalLinkedChunk::chunks`].
@@ -76,7 +78,7 @@ impl<Item, Gap> RelationalLinkedChunk<Item, Gap> {
 
     /// Apply [`Update`]s. That's the only way to write data inside this
     /// relational linked chunk.
-    pub fn apply_updates(&mut self, updates: &[Update<Item, Gap>])
+    pub fn apply_updates(&mut self, _room_id: &RoomId, updates: &[Update<Item, Gap>])
     where
         Item: Clone,
         Gap: Clone,
@@ -239,22 +241,28 @@ impl<Item, Gap> Default for RelationalLinkedChunk<Item, Gap> {
 
 #[cfg(test)]
 mod tests {
+    use ruma::room_id;
+
     use super::{ChunkIdentifier as CId, *};
 
     #[test]
     fn test_new_items_chunk() {
+        let room_id = room_id!("!r0:matrix.org");
         let mut relational_linked_chunk = RelationalLinkedChunk::<char, ()>::new();
 
-        relational_linked_chunk.apply_updates(&[
-            // 0
-            Update::NewItemsChunk { previous: None, new: CId(0), next: None },
-            // 1 after 0
-            Update::NewItemsChunk { previous: Some(CId(0)), new: CId(1), next: None },
-            // 2 before 0
-            Update::NewItemsChunk { previous: None, new: CId(2), next: Some(CId(0)) },
-            // 3 between 2 and 0
-            Update::NewItemsChunk { previous: Some(CId(2)), new: CId(3), next: Some(CId(0)) },
-        ]);
+        relational_linked_chunk.apply_updates(
+            room_id,
+            &[
+                // 0
+                Update::NewItemsChunk { previous: None, new: CId(0), next: None },
+                // 1 after 0
+                Update::NewItemsChunk { previous: Some(CId(0)), new: CId(1), next: None },
+                // 2 before 0
+                Update::NewItemsChunk { previous: None, new: CId(2), next: Some(CId(0)) },
+                // 3 between 2 and 0
+                Update::NewItemsChunk { previous: Some(CId(2)), new: CId(3), next: Some(CId(0)) },
+            ],
+        );
 
         // Chunks are correctly linked.
         assert_eq!(
@@ -272,16 +280,20 @@ mod tests {
 
     #[test]
     fn test_new_gap_chunk() {
+        let room_id = room_id!("!r0:matrix.org");
         let mut relational_linked_chunk = RelationalLinkedChunk::<char, ()>::new();
 
-        relational_linked_chunk.apply_updates(&[
-            // 0
-            Update::NewItemsChunk { previous: None, new: CId(0), next: None },
-            // 1 after 0
-            Update::NewGapChunk { previous: Some(CId(0)), new: CId(1), next: None, gap: () },
-            // 2 after 1
-            Update::NewItemsChunk { previous: Some(CId(1)), new: CId(2), next: None },
-        ]);
+        relational_linked_chunk.apply_updates(
+            room_id,
+            &[
+                // 0
+                Update::NewItemsChunk { previous: None, new: CId(0), next: None },
+                // 1 after 0
+                Update::NewGapChunk { previous: Some(CId(0)), new: CId(1), next: None, gap: () },
+                // 2 after 1
+                Update::NewItemsChunk { previous: Some(CId(1)), new: CId(2), next: None },
+            ],
+        );
 
         // Chunks are correctly links.
         assert_eq!(
@@ -301,18 +313,22 @@ mod tests {
 
     #[test]
     fn test_remove_chunk() {
+        let room_id = room_id!("!r0:matrix.org");
         let mut relational_linked_chunk = RelationalLinkedChunk::<char, ()>::new();
 
-        relational_linked_chunk.apply_updates(&[
-            // 0
-            Update::NewItemsChunk { previous: None, new: CId(0), next: None },
-            // 1 after 0
-            Update::NewGapChunk { previous: Some(CId(0)), new: CId(1), next: None, gap: () },
-            // 2 after 1
-            Update::NewItemsChunk { previous: Some(CId(1)), new: CId(2), next: None },
-            // remove 1
-            Update::RemoveChunk(CId(1)),
-        ]);
+        relational_linked_chunk.apply_updates(
+            room_id,
+            &[
+                // 0
+                Update::NewItemsChunk { previous: None, new: CId(0), next: None },
+                // 1 after 0
+                Update::NewGapChunk { previous: Some(CId(0)), new: CId(1), next: None, gap: () },
+                // 2 after 1
+                Update::NewItemsChunk { previous: Some(CId(1)), new: CId(2), next: None },
+                // remove 1
+                Update::RemoveChunk(CId(1)),
+            ],
+        );
 
         // Chunks are correctly links.
         assert_eq!(
@@ -328,20 +344,24 @@ mod tests {
 
     #[test]
     fn test_push_items() {
+        let room_id = room_id!("!r0:matrix.org");
         let mut relational_linked_chunk = RelationalLinkedChunk::<char, ()>::new();
 
-        relational_linked_chunk.apply_updates(&[
-            // new chunk (this is not mandatory for this test, but let's try to be realistic)
-            Update::NewItemsChunk { previous: None, new: CId(0), next: None },
-            // new items on 0
-            Update::PushItems { at: Position(CId(0), 0), items: vec!['a', 'b', 'c'] },
-            // new chunk (to test new items are pushed in the correct chunk)
-            Update::NewItemsChunk { previous: Some(CId(0)), new: CId(1), next: None },
-            // new items on 1
-            Update::PushItems { at: Position(CId(1), 0), items: vec!['x', 'y', 'z'] },
-            // new items on 0 again
-            Update::PushItems { at: Position(CId(0), 3), items: vec!['d', 'e'] },
-        ]);
+        relational_linked_chunk.apply_updates(
+            room_id,
+            &[
+                // new chunk (this is not mandatory for this test, but let's try to be realistic)
+                Update::NewItemsChunk { previous: None, new: CId(0), next: None },
+                // new items on 0
+                Update::PushItems { at: Position(CId(0), 0), items: vec!['a', 'b', 'c'] },
+                // new chunk (to test new items are pushed in the correct chunk)
+                Update::NewItemsChunk { previous: Some(CId(0)), new: CId(1), next: None },
+                // new items on 1
+                Update::PushItems { at: Position(CId(1), 0), items: vec!['x', 'y', 'z'] },
+                // new items on 0 again
+                Update::PushItems { at: Position(CId(0), 3), items: vec!['d', 'e'] },
+            ],
+        );
 
         // Chunks are correctly links.
         assert_eq!(
@@ -369,18 +389,22 @@ mod tests {
 
     #[test]
     fn test_remove_item() {
+        let room_id = room_id!("!r0:matrix.org");
         let mut relational_linked_chunk = RelationalLinkedChunk::<char, ()>::new();
 
-        relational_linked_chunk.apply_updates(&[
-            // new chunk (this is not mandatory for this test, but let's try to be realistic)
-            Update::NewItemsChunk { previous: None, new: CId(0), next: None },
-            // new items on 0
-            Update::PushItems { at: Position(CId(0), 0), items: vec!['a', 'b', 'c', 'd', 'e'] },
-            // remove an item: 'a'
-            Update::RemoveItem { at: Position(CId(0), 0) },
-            // remove an item: 'd'
-            Update::RemoveItem { at: Position(CId(0), 2) },
-        ]);
+        relational_linked_chunk.apply_updates(
+            room_id,
+            &[
+                // new chunk (this is not mandatory for this test, but let's try to be realistic)
+                Update::NewItemsChunk { previous: None, new: CId(0), next: None },
+                // new items on 0
+                Update::PushItems { at: Position(CId(0), 0), items: vec!['a', 'b', 'c', 'd', 'e'] },
+                // remove an item: 'a'
+                Update::RemoveItem { at: Position(CId(0), 0) },
+                // remove an item: 'd'
+                Update::RemoveItem { at: Position(CId(0), 2) },
+            ],
+        );
 
         // Chunks are correctly links.
         assert_eq!(
@@ -400,20 +424,24 @@ mod tests {
 
     #[test]
     fn test_detach_last_items() {
+        let room_id = room_id!("!r0:matrix.org");
         let mut relational_linked_chunk = RelationalLinkedChunk::<char, ()>::new();
 
-        relational_linked_chunk.apply_updates(&[
-            // new chunk
-            Update::NewItemsChunk { previous: None, new: CId(0), next: None },
-            // new chunk
-            Update::NewItemsChunk { previous: Some(CId(0)), new: CId(1), next: None },
-            // new items on 0
-            Update::PushItems { at: Position(CId(0), 0), items: vec!['a', 'b', 'c', 'd', 'e'] },
-            // new items on 1
-            Update::PushItems { at: Position(CId(1), 0), items: vec!['x', 'y', 'z'] },
-            // detach last items on 0
-            Update::DetachLastItems { at: Position(CId(0), 2) },
-        ]);
+        relational_linked_chunk.apply_updates(
+            room_id,
+            &[
+                // new chunk
+                Update::NewItemsChunk { previous: None, new: CId(0), next: None },
+                // new chunk
+                Update::NewItemsChunk { previous: Some(CId(0)), new: CId(1), next: None },
+                // new items on 0
+                Update::PushItems { at: Position(CId(0), 0), items: vec!['a', 'b', 'c', 'd', 'e'] },
+                // new items on 1
+                Update::PushItems { at: Position(CId(1), 0), items: vec!['x', 'y', 'z'] },
+                // detach last items on 0
+                Update::DetachLastItems { at: Position(CId(0), 2) },
+            ],
+        );
 
         // Chunks are correctly links.
         assert_eq!(
@@ -438,10 +466,11 @@ mod tests {
 
     #[test]
     fn test_start_and_end_reattach_items() {
+        let room_id = room_id!("!r0:matrix.org");
         let mut relational_linked_chunk = RelationalLinkedChunk::<char, ()>::new();
 
         relational_linked_chunk
-            .apply_updates(&[Update::StartReattachItems, Update::EndReattachItems]);
+            .apply_updates(room_id, &[Update::StartReattachItems, Update::EndReattachItems]);
 
         // Nothing happened.
         assert!(relational_linked_chunk.chunks.is_empty());
diff --git a/crates/matrix-sdk-sqlite/src/event_cache_store.rs b/crates/matrix-sdk-sqlite/src/event_cache_store.rs
index dc74999e227..f6b62e76216 100644
--- a/crates/matrix-sdk-sqlite/src/event_cache_store.rs
+++ b/crates/matrix-sdk-sqlite/src/event_cache_store.rs
@@ -8,7 +8,7 @@ use matrix_sdk_base::{
     media::{MediaRequestParameters, UniqueKey},
 };
 use matrix_sdk_store_encryption::StoreCipher;
-use ruma::MilliSecondsSinceUnixEpoch;
+use ruma::{MilliSecondsSinceUnixEpoch, RoomId};
 use rusqlite::OptionalExtension;
 use tokio::fs;
 use tracing::debug;
@@ -185,6 +185,7 @@ impl EventCacheStore for SqliteEventCacheStore {
 
     async fn handle_linked_chunk_updates(
         &self,
+        _room_id: &RoomId,
         _updates: &[Update<Event, Gap>],
     ) -> Result<(), Self::Error> {
         todo!()

From 3ebc31bff7ae372049369f39a9a5568d1ae086d1 Mon Sep 17 00:00:00 2001
From: Ivan Enderlin <ivan@mnt.io>
Date: Mon, 25 Nov 2024 10:16:00 +0100
Subject: [PATCH 5/9] feat(common): `RelationalLinkedChunk` stores the
 `RoomId`.

---
 .../src/linked_chunk/relational.rs            | 287 ++++++++++++++----
 1 file changed, 233 insertions(+), 54 deletions(-)

diff --git a/crates/matrix-sdk-common/src/linked_chunk/relational.rs b/crates/matrix-sdk-common/src/linked_chunk/relational.rs
index d410b2fa8db..4418bf3e654 100644
--- a/crates/matrix-sdk-common/src/linked_chunk/relational.rs
+++ b/crates/matrix-sdk-common/src/linked_chunk/relational.rs
@@ -15,13 +15,14 @@
 //! Implementation for a _relational linked chunk_, see
 //! [`RelationalLinkedChunk`].
 
-use ruma::RoomId;
+use ruma::{OwnedRoomId, RoomId};
 
 use crate::linked_chunk::{ChunkIdentifier, Position, Update};
 
 /// A row of the [`RelationalLinkedChunk::chunks`].
 #[derive(Debug, PartialEq)]
 struct ChunkRow {
+    room_id: OwnedRoomId,
     previous_chunk: Option<ChunkIdentifier>,
     chunk: ChunkIdentifier,
     next_chunk: Option<ChunkIdentifier>,
@@ -30,6 +31,7 @@ struct ChunkRow {
 /// A row of the [`RelationalLinkedChunk::items`].
 #[derive(Debug, PartialEq)]
 struct ItemRow<Item, Gap> {
+    room_id: OwnedRoomId,
     position: Position,
     item: Either<Item, Gap>,
 }
@@ -78,7 +80,7 @@ impl<Item, Gap> RelationalLinkedChunk<Item, Gap> {
 
     /// Apply [`Update`]s. That's the only way to write data inside this
     /// relational linked chunk.
-    pub fn apply_updates(&mut self, _room_id: &RoomId, updates: &[Update<Item, Gap>])
+    pub fn apply_updates(&mut self, room_id: &RoomId, updates: &[Update<Item, Gap>])
     where
         Item: Clone,
         Gap: Clone,
@@ -86,27 +88,32 @@ impl<Item, Gap> RelationalLinkedChunk<Item, Gap> {
         for update in updates {
             match update {
                 Update::NewItemsChunk { previous, new, next } => {
-                    insert_chunk(&mut self.chunks, previous, new, next);
+                    insert_chunk(&mut self.chunks, room_id, previous, new, next);
                 }
 
                 Update::NewGapChunk { previous, new, next, gap } => {
-                    insert_chunk(&mut self.chunks, previous, new, next);
+                    insert_chunk(&mut self.chunks, room_id, previous, new, next);
                     self.items.push(ItemRow {
+                        room_id: room_id.to_owned(),
                         position: Position(*new, 0),
                         item: Either::Gap(gap.clone()),
                     });
                 }
 
                 Update::RemoveChunk(chunk_identifier) => {
-                    remove_chunk(&mut self.chunks, chunk_identifier);
+                    remove_chunk(&mut self.chunks, room_id, chunk_identifier);
 
                     let indices_to_remove = self
                         .items
                         .iter()
                         .enumerate()
-                        .filter_map(|(nth, ItemRow { position, .. })| {
-                            (position.chunk_identifier() == *chunk_identifier).then_some(nth)
-                        })
+                        .filter_map(
+                            |(nth, ItemRow { room_id: room_id_candidate, position, .. })| {
+                                (room_id == room_id_candidate
+                                    && position.chunk_identifier() == *chunk_identifier)
+                                    .then_some(nth)
+                            },
+                        )
                         .collect::<Vec<_>>();
 
                     for index_to_remove in indices_to_remove.into_iter().rev() {
@@ -118,7 +125,11 @@ impl<Item, Gap> RelationalLinkedChunk<Item, Gap> {
                     let mut at = *at;
 
                     for item in items {
-                        self.items.push(ItemRow { position: at, item: Either::Item(item.clone()) });
+                        self.items.push(ItemRow {
+                            room_id: room_id.to_owned(),
+                            position: at,
+                            item: Either::Item(item.clone()),
+                        });
                         at.increment_index();
                     }
                 }
@@ -126,7 +137,14 @@ impl<Item, Gap> RelationalLinkedChunk<Item, Gap> {
                 Update::RemoveItem { at } => {
                     let mut entry_to_remove = None;
 
-                    for (nth, ItemRow { position, .. }) in self.items.iter_mut().enumerate() {
+                    for (nth, ItemRow { room_id: room_id_candidate, position, .. }) in
+                        self.items.iter_mut().enumerate()
+                    {
+                        // Filter by room ID.
+                        if room_id != room_id_candidate {
+                            continue;
+                        }
+
                         // Find the item to remove.
                         if position == at {
                             debug_assert!(entry_to_remove.is_none(), "Found the same entry twice");
@@ -150,11 +168,14 @@ impl<Item, Gap> RelationalLinkedChunk<Item, Gap> {
                         .items
                         .iter()
                         .enumerate()
-                        .filter_map(|(nth, ItemRow { position, .. })| {
-                            (position.chunk_identifier() == at.chunk_identifier()
-                                && position.index() >= at.index())
-                            .then_some(nth)
-                        })
+                        .filter_map(
+                            |(nth, ItemRow { room_id: room_id_candidate, position, .. })| {
+                                (room_id == room_id_candidate
+                                    && position.chunk_identifier() == at.chunk_identifier()
+                                    && position.index() >= at.index())
+                                .then_some(nth)
+                            },
+                        )
                         .collect::<Vec<_>>();
 
                     for index_to_remove in indices_to_remove.into_iter().rev() {
@@ -168,6 +189,7 @@ impl<Item, Gap> RelationalLinkedChunk<Item, Gap> {
 
         fn insert_chunk(
             chunks: &mut Vec<ChunkRow>,
+            room_id: &RoomId,
             previous: &Option<ChunkIdentifier>,
             new: &ChunkIdentifier,
             next: &Option<ChunkIdentifier>,
@@ -176,7 +198,9 @@ impl<Item, Gap> RelationalLinkedChunk<Item, Gap> {
             if let Some(previous) = previous {
                 let entry_for_previous_chunk = chunks
                     .iter_mut()
-                    .find(|ChunkRow { chunk, .. }| chunk == previous)
+                    .find(|ChunkRow { room_id: room_id_candidate, chunk, .. }| {
+                        room_id == room_id_candidate && chunk == previous
+                    })
                     .expect("Previous chunk should be present");
 
                 // Insert the chunk.
@@ -187,7 +211,9 @@ impl<Item, Gap> RelationalLinkedChunk<Item, Gap> {
             if let Some(next) = next {
                 let entry_for_next_chunk = chunks
                     .iter_mut()
-                    .find(|ChunkRow { chunk, .. }| chunk == next)
+                    .find(|ChunkRow { room_id: room_id_candidate, chunk, .. }| {
+                        room_id == room_id_candidate && chunk == next
+                    })
                     .expect("Next chunk should be present");
 
                 // Insert the chunk.
@@ -195,24 +221,37 @@ impl<Item, Gap> RelationalLinkedChunk<Item, Gap> {
             }
 
             // Insert the chunk.
-            chunks.push(ChunkRow { previous_chunk: *previous, chunk: *new, next_chunk: *next });
+            chunks.push(ChunkRow {
+                room_id: room_id.to_owned(),
+                previous_chunk: *previous,
+                chunk: *new,
+                next_chunk: *next,
+            });
         }
 
-        fn remove_chunk(chunks: &mut Vec<ChunkRow>, chunk_to_remove: &ChunkIdentifier) {
+        fn remove_chunk(
+            chunks: &mut Vec<ChunkRow>,
+            room_id: &RoomId,
+            chunk_to_remove: &ChunkIdentifier,
+        ) {
             let entry_nth_to_remove = chunks
                 .iter()
                 .enumerate()
-                .find_map(|(nth, ChunkRow { chunk, .. })| (chunk == chunk_to_remove).then_some(nth))
+                .find_map(|(nth, ChunkRow { room_id: room_id_candidate, chunk, .. })| {
+                    (room_id == room_id_candidate && chunk == chunk_to_remove).then_some(nth)
+                })
                 .expect("Remove an unknown chunk");
 
-            let ChunkRow { previous_chunk: previous, next_chunk: next, .. } =
+            let ChunkRow { room_id, previous_chunk: previous, next_chunk: next, .. } =
                 chunks.remove(entry_nth_to_remove);
 
             // Find the previous chunk, and update its next chunk.
             if let Some(previous) = previous {
                 let entry_for_previous_chunk = chunks
                     .iter_mut()
-                    .find(|ChunkRow { chunk, .. }| *chunk == previous)
+                    .find(|ChunkRow { room_id: room_id_candidate, chunk, .. }| {
+                        &room_id == room_id_candidate && *chunk == previous
+                    })
                     .expect("Previous chunk should be present");
 
                 // Insert the chunk.
@@ -223,7 +262,9 @@ impl<Item, Gap> RelationalLinkedChunk<Item, Gap> {
             if let Some(next) = next {
                 let entry_for_next_chunk = chunks
                     .iter_mut()
-                    .find(|ChunkRow { chunk, .. }| *chunk == next)
+                    .find(|ChunkRow { room_id: room_id_candidate, chunk, .. }| {
+                        &room_id == room_id_candidate && *chunk == next
+                    })
                     .expect("Next chunk should be present");
 
                 // Insert the chunk.
@@ -268,10 +309,30 @@ mod tests {
         assert_eq!(
             relational_linked_chunk.chunks,
             &[
-                ChunkRow { previous_chunk: Some(CId(3)), chunk: CId(0), next_chunk: Some(CId(1)) },
-                ChunkRow { previous_chunk: Some(CId(0)), chunk: CId(1), next_chunk: None },
-                ChunkRow { previous_chunk: None, chunk: CId(2), next_chunk: Some(CId(3)) },
-                ChunkRow { previous_chunk: Some(CId(2)), chunk: CId(3), next_chunk: Some(CId(0)) },
+                ChunkRow {
+                    room_id: room_id.to_owned(),
+                    previous_chunk: Some(CId(3)),
+                    chunk: CId(0),
+                    next_chunk: Some(CId(1))
+                },
+                ChunkRow {
+                    room_id: room_id.to_owned(),
+                    previous_chunk: Some(CId(0)),
+                    chunk: CId(1),
+                    next_chunk: None
+                },
+                ChunkRow {
+                    room_id: room_id.to_owned(),
+                    previous_chunk: None,
+                    chunk: CId(2),
+                    next_chunk: Some(CId(3))
+                },
+                ChunkRow {
+                    room_id: room_id.to_owned(),
+                    previous_chunk: Some(CId(2)),
+                    chunk: CId(3),
+                    next_chunk: Some(CId(0))
+                },
             ],
         );
         // Items have not been modified.
@@ -299,15 +360,34 @@ mod tests {
         assert_eq!(
             relational_linked_chunk.chunks,
             &[
-                ChunkRow { previous_chunk: None, chunk: CId(0), next_chunk: Some(CId(1)) },
-                ChunkRow { previous_chunk: Some(CId(0)), chunk: CId(1), next_chunk: Some(CId(2)) },
-                ChunkRow { previous_chunk: Some(CId(1)), chunk: CId(2), next_chunk: None },
+                ChunkRow {
+                    room_id: room_id.to_owned(),
+                    previous_chunk: None,
+                    chunk: CId(0),
+                    next_chunk: Some(CId(1))
+                },
+                ChunkRow {
+                    room_id: room_id.to_owned(),
+                    previous_chunk: Some(CId(0)),
+                    chunk: CId(1),
+                    next_chunk: Some(CId(2))
+                },
+                ChunkRow {
+                    room_id: room_id.to_owned(),
+                    previous_chunk: Some(CId(1)),
+                    chunk: CId(2),
+                    next_chunk: None
+                },
             ],
         );
         // Items contains the gap.
         assert_eq!(
             relational_linked_chunk.items,
-            &[ItemRow { position: Position(CId(1), 0), item: Either::Gap(()) }],
+            &[ItemRow {
+                room_id: room_id.to_owned(),
+                position: Position(CId(1), 0),
+                item: Either::Gap(())
+            }],
         );
     }
 
@@ -334,8 +414,18 @@ mod tests {
         assert_eq!(
             relational_linked_chunk.chunks,
             &[
-                ChunkRow { previous_chunk: None, chunk: CId(0), next_chunk: Some(CId(2)) },
-                ChunkRow { previous_chunk: Some(CId(0)), chunk: CId(2), next_chunk: None },
+                ChunkRow {
+                    room_id: room_id.to_owned(),
+                    previous_chunk: None,
+                    chunk: CId(0),
+                    next_chunk: Some(CId(2))
+                },
+                ChunkRow {
+                    room_id: room_id.to_owned(),
+                    previous_chunk: Some(CId(0)),
+                    chunk: CId(2),
+                    next_chunk: None
+                },
             ],
         );
         // Items no longer contains the gap.
@@ -367,22 +457,64 @@ mod tests {
         assert_eq!(
             relational_linked_chunk.chunks,
             &[
-                ChunkRow { previous_chunk: None, chunk: CId(0), next_chunk: Some(CId(1)) },
-                ChunkRow { previous_chunk: Some(CId(0)), chunk: CId(1), next_chunk: None },
+                ChunkRow {
+                    room_id: room_id.to_owned(),
+                    previous_chunk: None,
+                    chunk: CId(0),
+                    next_chunk: Some(CId(1))
+                },
+                ChunkRow {
+                    room_id: room_id.to_owned(),
+                    previous_chunk: Some(CId(0)),
+                    chunk: CId(1),
+                    next_chunk: None
+                },
             ],
         );
         // Items contains the pushed items.
         assert_eq!(
             relational_linked_chunk.items,
             &[
-                ItemRow { position: Position(CId(0), 0), item: Either::Item('a') },
-                ItemRow { position: Position(CId(0), 1), item: Either::Item('b') },
-                ItemRow { position: Position(CId(0), 2), item: Either::Item('c') },
-                ItemRow { position: Position(CId(1), 0), item: Either::Item('x') },
-                ItemRow { position: Position(CId(1), 1), item: Either::Item('y') },
-                ItemRow { position: Position(CId(1), 2), item: Either::Item('z') },
-                ItemRow { position: Position(CId(0), 3), item: Either::Item('d') },
-                ItemRow { position: Position(CId(0), 4), item: Either::Item('e') },
+                ItemRow {
+                    room_id: room_id.to_owned(),
+                    position: Position(CId(0), 0),
+                    item: Either::Item('a')
+                },
+                ItemRow {
+                    room_id: room_id.to_owned(),
+                    position: Position(CId(0), 1),
+                    item: Either::Item('b')
+                },
+                ItemRow {
+                    room_id: room_id.to_owned(),
+                    position: Position(CId(0), 2),
+                    item: Either::Item('c')
+                },
+                ItemRow {
+                    room_id: room_id.to_owned(),
+                    position: Position(CId(1), 0),
+                    item: Either::Item('x')
+                },
+                ItemRow {
+                    room_id: room_id.to_owned(),
+                    position: Position(CId(1), 1),
+                    item: Either::Item('y')
+                },
+                ItemRow {
+                    room_id: room_id.to_owned(),
+                    position: Position(CId(1), 2),
+                    item: Either::Item('z')
+                },
+                ItemRow {
+                    room_id: room_id.to_owned(),
+                    position: Position(CId(0), 3),
+                    item: Either::Item('d')
+                },
+                ItemRow {
+                    room_id: room_id.to_owned(),
+                    position: Position(CId(0), 4),
+                    item: Either::Item('e')
+                },
             ],
         );
     }
@@ -409,15 +541,32 @@ mod tests {
         // Chunks are correctly links.
         assert_eq!(
             relational_linked_chunk.chunks,
-            &[ChunkRow { previous_chunk: None, chunk: CId(0), next_chunk: None }],
+            &[ChunkRow {
+                room_id: room_id.to_owned(),
+                previous_chunk: None,
+                chunk: CId(0),
+                next_chunk: None
+            }],
         );
         // Items contains the pushed items.
         assert_eq!(
             relational_linked_chunk.items,
             &[
-                ItemRow { position: Position(CId(0), 0), item: Either::Item('b') },
-                ItemRow { position: Position(CId(0), 1), item: Either::Item('c') },
-                ItemRow { position: Position(CId(0), 2), item: Either::Item('e') },
+                ItemRow {
+                    room_id: room_id.to_owned(),
+                    position: Position(CId(0), 0),
+                    item: Either::Item('b')
+                },
+                ItemRow {
+                    room_id: room_id.to_owned(),
+                    position: Position(CId(0), 1),
+                    item: Either::Item('c')
+                },
+                ItemRow {
+                    room_id: room_id.to_owned(),
+                    position: Position(CId(0), 2),
+                    item: Either::Item('e')
+                },
             ],
         );
     }
@@ -447,19 +596,49 @@ mod tests {
         assert_eq!(
             relational_linked_chunk.chunks,
             &[
-                ChunkRow { previous_chunk: None, chunk: CId(0), next_chunk: Some(CId(1)) },
-                ChunkRow { previous_chunk: Some(CId(0)), chunk: CId(1), next_chunk: None },
+                ChunkRow {
+                    room_id: room_id.to_owned(),
+                    previous_chunk: None,
+                    chunk: CId(0),
+                    next_chunk: Some(CId(1))
+                },
+                ChunkRow {
+                    room_id: room_id.to_owned(),
+                    previous_chunk: Some(CId(0)),
+                    chunk: CId(1),
+                    next_chunk: None
+                },
             ],
         );
         // Items contains the pushed items.
         assert_eq!(
             relational_linked_chunk.items,
             &[
-                ItemRow { position: Position(CId(0), 0), item: Either::Item('a') },
-                ItemRow { position: Position(CId(0), 1), item: Either::Item('b') },
-                ItemRow { position: Position(CId(1), 0), item: Either::Item('x') },
-                ItemRow { position: Position(CId(1), 1), item: Either::Item('y') },
-                ItemRow { position: Position(CId(1), 2), item: Either::Item('z') },
+                ItemRow {
+                    room_id: room_id.to_owned(),
+                    position: Position(CId(0), 0),
+                    item: Either::Item('a')
+                },
+                ItemRow {
+                    room_id: room_id.to_owned(),
+                    position: Position(CId(0), 1),
+                    item: Either::Item('b')
+                },
+                ItemRow {
+                    room_id: room_id.to_owned(),
+                    position: Position(CId(1), 0),
+                    item: Either::Item('x')
+                },
+                ItemRow {
+                    room_id: room_id.to_owned(),
+                    position: Position(CId(1), 1),
+                    item: Either::Item('y')
+                },
+                ItemRow {
+                    room_id: room_id.to_owned(),
+                    position: Position(CId(1), 2),
+                    item: Either::Item('z')
+                },
             ],
         );
     }

From ba73489adf7de24fac446651f0b966b8bf7d5f08 Mon Sep 17 00:00:00 2001
From: Ivan Enderlin <ivan@mnt.io>
Date: Mon, 25 Nov 2024 15:43:09 +0100
Subject: [PATCH 6/9] refactor: Add constructors for `Position` and
 `ChunkIdentifier`.

This patch adds constructors for `Position` and `ChunkIdentifier` so
that we keep their inner values private.
---
 .../matrix-sdk-common/src/linked_chunk/mod.rs |  21 ++-
 .../src/linked_chunk/relational.rs            | 169 ++++++++++--------
 2 files changed, 114 insertions(+), 76 deletions(-)

diff --git a/crates/matrix-sdk-common/src/linked_chunk/mod.rs b/crates/matrix-sdk-common/src/linked_chunk/mod.rs
index 479474c83e1..0aae2ecc942 100644
--- a/crates/matrix-sdk-common/src/linked_chunk/mod.rs
+++ b/crates/matrix-sdk-common/src/linked_chunk/mod.rs
@@ -934,7 +934,19 @@ impl ChunkIdentifierGenerator {
 /// Learn more with [`ChunkIdentifierGenerator`].
 #[derive(Copy, Clone, Debug, PartialEq)]
 #[repr(transparent)]
-pub struct ChunkIdentifier(pub(super) u64);
+pub struct ChunkIdentifier(u64);
+
+impl ChunkIdentifier {
+    /// Create a new [`ChunkIdentifier`].
+    pub(super) fn new(identifier: u64) -> Self {
+        Self(identifier)
+    }
+
+    /// Get the underlying identifier.
+    fn index(&self) -> u64 {
+        self.0
+    }
+}
 
 impl PartialEq<u64> for ChunkIdentifier {
     fn eq(&self, other: &u64) -> bool {
@@ -946,9 +958,14 @@ impl PartialEq<u64> for ChunkIdentifier {
 ///
 /// It's a pair of a chunk position and an item index.
 #[derive(Copy, Clone, Debug, PartialEq)]
-pub struct Position(pub(super) ChunkIdentifier, pub(super) usize);
+pub struct Position(ChunkIdentifier, usize);
 
 impl Position {
+    /// Create a new [`Position`].
+    pub(super) fn new(chunk_identifier: ChunkIdentifier, index: usize) -> Self {
+        Self(chunk_identifier, index)
+    }
+
     /// Get the chunk identifier of the item.
     pub fn chunk_identifier(&self) -> ChunkIdentifier {
         self.0
diff --git a/crates/matrix-sdk-common/src/linked_chunk/relational.rs b/crates/matrix-sdk-common/src/linked_chunk/relational.rs
index 4418bf3e654..f7c2f9c7e24 100644
--- a/crates/matrix-sdk-common/src/linked_chunk/relational.rs
+++ b/crates/matrix-sdk-common/src/linked_chunk/relational.rs
@@ -56,7 +56,8 @@ enum Either<Item, Gap> {
 /// This type is also designed to receive [`Update`]. Applying `Update`s
 /// directly on a [`LinkedChunk`] is not ideal and particularly not trivial as
 /// the `Update`s do _not_ match the internal data layout of the `LinkedChunk`,
-/// they are been designed for storages, like a relational database for example.
+/// they have been designed for storages, like a relational database for
+/// example.
 ///
 /// This type is not as performant as [`LinkedChunk`] (in terms of memory
 /// layout, CPU caches etc.). It is only designed to be used in memory stores,
@@ -95,7 +96,7 @@ impl<Item, Gap> RelationalLinkedChunk<Item, Gap> {
                     insert_chunk(&mut self.chunks, room_id, previous, new, next);
                     self.items.push(ItemRow {
                         room_id: room_id.to_owned(),
-                        position: Position(*new, 0),
+                        position: Position::new(*new, 0),
                         item: Either::Gap(gap.clone()),
                     });
                 }
@@ -295,13 +296,17 @@ mod tests {
             room_id,
             &[
                 // 0
-                Update::NewItemsChunk { previous: None, new: CId(0), next: None },
+                Update::NewItemsChunk { previous: None, new: CId::new(0), next: None },
                 // 1 after 0
-                Update::NewItemsChunk { previous: Some(CId(0)), new: CId(1), next: None },
+                Update::NewItemsChunk { previous: Some(CId::new(0)), new: CId::new(1), next: None },
                 // 2 before 0
-                Update::NewItemsChunk { previous: None, new: CId(2), next: Some(CId(0)) },
+                Update::NewItemsChunk { previous: None, new: CId::new(2), next: Some(CId::new(0)) },
                 // 3 between 2 and 0
-                Update::NewItemsChunk { previous: Some(CId(2)), new: CId(3), next: Some(CId(0)) },
+                Update::NewItemsChunk {
+                    previous: Some(CId::new(2)),
+                    new: CId::new(3),
+                    next: Some(CId::new(0)),
+                },
             ],
         );
 
@@ -311,27 +316,27 @@ mod tests {
             &[
                 ChunkRow {
                     room_id: room_id.to_owned(),
-                    previous_chunk: Some(CId(3)),
-                    chunk: CId(0),
-                    next_chunk: Some(CId(1))
+                    previous_chunk: Some(CId::new(3)),
+                    chunk: CId::new(0),
+                    next_chunk: Some(CId::new(1))
                 },
                 ChunkRow {
                     room_id: room_id.to_owned(),
-                    previous_chunk: Some(CId(0)),
-                    chunk: CId(1),
+                    previous_chunk: Some(CId::new(0)),
+                    chunk: CId::new(1),
                     next_chunk: None
                 },
                 ChunkRow {
                     room_id: room_id.to_owned(),
                     previous_chunk: None,
-                    chunk: CId(2),
-                    next_chunk: Some(CId(3))
+                    chunk: CId::new(2),
+                    next_chunk: Some(CId::new(3))
                 },
                 ChunkRow {
                     room_id: room_id.to_owned(),
-                    previous_chunk: Some(CId(2)),
-                    chunk: CId(3),
-                    next_chunk: Some(CId(0))
+                    previous_chunk: Some(CId::new(2)),
+                    chunk: CId::new(3),
+                    next_chunk: Some(CId::new(0))
                 },
             ],
         );
@@ -348,11 +353,16 @@ mod tests {
             room_id,
             &[
                 // 0
-                Update::NewItemsChunk { previous: None, new: CId(0), next: None },
+                Update::NewItemsChunk { previous: None, new: CId::new(0), next: None },
                 // 1 after 0
-                Update::NewGapChunk { previous: Some(CId(0)), new: CId(1), next: None, gap: () },
+                Update::NewGapChunk {
+                    previous: Some(CId::new(0)),
+                    new: CId::new(1),
+                    next: None,
+                    gap: (),
+                },
                 // 2 after 1
-                Update::NewItemsChunk { previous: Some(CId(1)), new: CId(2), next: None },
+                Update::NewItemsChunk { previous: Some(CId::new(1)), new: CId::new(2), next: None },
             ],
         );
 
@@ -363,19 +373,19 @@ mod tests {
                 ChunkRow {
                     room_id: room_id.to_owned(),
                     previous_chunk: None,
-                    chunk: CId(0),
-                    next_chunk: Some(CId(1))
+                    chunk: CId::new(0),
+                    next_chunk: Some(CId::new(1))
                 },
                 ChunkRow {
                     room_id: room_id.to_owned(),
-                    previous_chunk: Some(CId(0)),
-                    chunk: CId(1),
-                    next_chunk: Some(CId(2))
+                    previous_chunk: Some(CId::new(0)),
+                    chunk: CId::new(1),
+                    next_chunk: Some(CId::new(2))
                 },
                 ChunkRow {
                     room_id: room_id.to_owned(),
-                    previous_chunk: Some(CId(1)),
-                    chunk: CId(2),
+                    previous_chunk: Some(CId::new(1)),
+                    chunk: CId::new(2),
                     next_chunk: None
                 },
             ],
@@ -385,7 +395,7 @@ mod tests {
             relational_linked_chunk.items,
             &[ItemRow {
                 room_id: room_id.to_owned(),
-                position: Position(CId(1), 0),
+                position: Position::new(CId::new(1), 0),
                 item: Either::Gap(())
             }],
         );
@@ -400,13 +410,18 @@ mod tests {
             room_id,
             &[
                 // 0
-                Update::NewItemsChunk { previous: None, new: CId(0), next: None },
+                Update::NewItemsChunk { previous: None, new: CId::new(0), next: None },
                 // 1 after 0
-                Update::NewGapChunk { previous: Some(CId(0)), new: CId(1), next: None, gap: () },
+                Update::NewGapChunk {
+                    previous: Some(CId::new(0)),
+                    new: CId::new(1),
+                    next: None,
+                    gap: (),
+                },
                 // 2 after 1
-                Update::NewItemsChunk { previous: Some(CId(1)), new: CId(2), next: None },
+                Update::NewItemsChunk { previous: Some(CId::new(1)), new: CId::new(2), next: None },
                 // remove 1
-                Update::RemoveChunk(CId(1)),
+                Update::RemoveChunk(CId::new(1)),
             ],
         );
 
@@ -417,13 +432,13 @@ mod tests {
                 ChunkRow {
                     room_id: room_id.to_owned(),
                     previous_chunk: None,
-                    chunk: CId(0),
-                    next_chunk: Some(CId(2))
+                    chunk: CId::new(0),
+                    next_chunk: Some(CId::new(2))
                 },
                 ChunkRow {
                     room_id: room_id.to_owned(),
-                    previous_chunk: Some(CId(0)),
-                    chunk: CId(2),
+                    previous_chunk: Some(CId::new(0)),
+                    chunk: CId::new(2),
                     next_chunk: None
                 },
             ],
@@ -441,15 +456,15 @@ mod tests {
             room_id,
             &[
                 // new chunk (this is not mandatory for this test, but let's try to be realistic)
-                Update::NewItemsChunk { previous: None, new: CId(0), next: None },
+                Update::NewItemsChunk { previous: None, new: CId::new(0), next: None },
                 // new items on 0
-                Update::PushItems { at: Position(CId(0), 0), items: vec!['a', 'b', 'c'] },
+                Update::PushItems { at: Position::new(CId::new(0), 0), items: vec!['a', 'b', 'c'] },
                 // new chunk (to test new items are pushed in the correct chunk)
-                Update::NewItemsChunk { previous: Some(CId(0)), new: CId(1), next: None },
+                Update::NewItemsChunk { previous: Some(CId::new(0)), new: CId::new(1), next: None },
                 // new items on 1
-                Update::PushItems { at: Position(CId(1), 0), items: vec!['x', 'y', 'z'] },
+                Update::PushItems { at: Position::new(CId::new(1), 0), items: vec!['x', 'y', 'z'] },
                 // new items on 0 again
-                Update::PushItems { at: Position(CId(0), 3), items: vec!['d', 'e'] },
+                Update::PushItems { at: Position::new(CId::new(0), 3), items: vec!['d', 'e'] },
             ],
         );
 
@@ -460,13 +475,13 @@ mod tests {
                 ChunkRow {
                     room_id: room_id.to_owned(),
                     previous_chunk: None,
-                    chunk: CId(0),
-                    next_chunk: Some(CId(1))
+                    chunk: CId::new(0),
+                    next_chunk: Some(CId::new(1))
                 },
                 ChunkRow {
                     room_id: room_id.to_owned(),
-                    previous_chunk: Some(CId(0)),
-                    chunk: CId(1),
+                    previous_chunk: Some(CId::new(0)),
+                    chunk: CId::new(1),
                     next_chunk: None
                 },
             ],
@@ -477,42 +492,42 @@ mod tests {
             &[
                 ItemRow {
                     room_id: room_id.to_owned(),
-                    position: Position(CId(0), 0),
+                    position: Position::new(CId::new(0), 0),
                     item: Either::Item('a')
                 },
                 ItemRow {
                     room_id: room_id.to_owned(),
-                    position: Position(CId(0), 1),
+                    position: Position::new(CId::new(0), 1),
                     item: Either::Item('b')
                 },
                 ItemRow {
                     room_id: room_id.to_owned(),
-                    position: Position(CId(0), 2),
+                    position: Position::new(CId::new(0), 2),
                     item: Either::Item('c')
                 },
                 ItemRow {
                     room_id: room_id.to_owned(),
-                    position: Position(CId(1), 0),
+                    position: Position::new(CId::new(1), 0),
                     item: Either::Item('x')
                 },
                 ItemRow {
                     room_id: room_id.to_owned(),
-                    position: Position(CId(1), 1),
+                    position: Position::new(CId::new(1), 1),
                     item: Either::Item('y')
                 },
                 ItemRow {
                     room_id: room_id.to_owned(),
-                    position: Position(CId(1), 2),
+                    position: Position::new(CId::new(1), 2),
                     item: Either::Item('z')
                 },
                 ItemRow {
                     room_id: room_id.to_owned(),
-                    position: Position(CId(0), 3),
+                    position: Position::new(CId::new(0), 3),
                     item: Either::Item('d')
                 },
                 ItemRow {
                     room_id: room_id.to_owned(),
-                    position: Position(CId(0), 4),
+                    position: Position::new(CId::new(0), 4),
                     item: Either::Item('e')
                 },
             ],
@@ -528,13 +543,16 @@ mod tests {
             room_id,
             &[
                 // new chunk (this is not mandatory for this test, but let's try to be realistic)
-                Update::NewItemsChunk { previous: None, new: CId(0), next: None },
+                Update::NewItemsChunk { previous: None, new: CId::new(0), next: None },
                 // new items on 0
-                Update::PushItems { at: Position(CId(0), 0), items: vec!['a', 'b', 'c', 'd', 'e'] },
+                Update::PushItems {
+                    at: Position::new(CId::new(0), 0),
+                    items: vec!['a', 'b', 'c', 'd', 'e'],
+                },
                 // remove an item: 'a'
-                Update::RemoveItem { at: Position(CId(0), 0) },
+                Update::RemoveItem { at: Position::new(CId::new(0), 0) },
                 // remove an item: 'd'
-                Update::RemoveItem { at: Position(CId(0), 2) },
+                Update::RemoveItem { at: Position::new(CId::new(0), 2) },
             ],
         );
 
@@ -544,7 +562,7 @@ mod tests {
             &[ChunkRow {
                 room_id: room_id.to_owned(),
                 previous_chunk: None,
-                chunk: CId(0),
+                chunk: CId::new(0),
                 next_chunk: None
             }],
         );
@@ -554,17 +572,17 @@ mod tests {
             &[
                 ItemRow {
                     room_id: room_id.to_owned(),
-                    position: Position(CId(0), 0),
+                    position: Position::new(CId::new(0), 0),
                     item: Either::Item('b')
                 },
                 ItemRow {
                     room_id: room_id.to_owned(),
-                    position: Position(CId(0), 1),
+                    position: Position::new(CId::new(0), 1),
                     item: Either::Item('c')
                 },
                 ItemRow {
                     room_id: room_id.to_owned(),
-                    position: Position(CId(0), 2),
+                    position: Position::new(CId::new(0), 2),
                     item: Either::Item('e')
                 },
             ],
@@ -580,15 +598,18 @@ mod tests {
             room_id,
             &[
                 // new chunk
-                Update::NewItemsChunk { previous: None, new: CId(0), next: None },
+                Update::NewItemsChunk { previous: None, new: CId::new(0), next: None },
                 // new chunk
-                Update::NewItemsChunk { previous: Some(CId(0)), new: CId(1), next: None },
+                Update::NewItemsChunk { previous: Some(CId::new(0)), new: CId::new(1), next: None },
                 // new items on 0
-                Update::PushItems { at: Position(CId(0), 0), items: vec!['a', 'b', 'c', 'd', 'e'] },
+                Update::PushItems {
+                    at: Position::new(CId::new(0), 0),
+                    items: vec!['a', 'b', 'c', 'd', 'e'],
+                },
                 // new items on 1
-                Update::PushItems { at: Position(CId(1), 0), items: vec!['x', 'y', 'z'] },
+                Update::PushItems { at: Position::new(CId::new(1), 0), items: vec!['x', 'y', 'z'] },
                 // detach last items on 0
-                Update::DetachLastItems { at: Position(CId(0), 2) },
+                Update::DetachLastItems { at: Position::new(CId::new(0), 2) },
             ],
         );
 
@@ -599,13 +620,13 @@ mod tests {
                 ChunkRow {
                     room_id: room_id.to_owned(),
                     previous_chunk: None,
-                    chunk: CId(0),
-                    next_chunk: Some(CId(1))
+                    chunk: CId::new(0),
+                    next_chunk: Some(CId::new(1))
                 },
                 ChunkRow {
                     room_id: room_id.to_owned(),
-                    previous_chunk: Some(CId(0)),
-                    chunk: CId(1),
+                    previous_chunk: Some(CId::new(0)),
+                    chunk: CId::new(1),
                     next_chunk: None
                 },
             ],
@@ -616,27 +637,27 @@ mod tests {
             &[
                 ItemRow {
                     room_id: room_id.to_owned(),
-                    position: Position(CId(0), 0),
+                    position: Position::new(CId::new(0), 0),
                     item: Either::Item('a')
                 },
                 ItemRow {
                     room_id: room_id.to_owned(),
-                    position: Position(CId(0), 1),
+                    position: Position::new(CId::new(0), 1),
                     item: Either::Item('b')
                 },
                 ItemRow {
                     room_id: room_id.to_owned(),
-                    position: Position(CId(1), 0),
+                    position: Position::new(CId::new(1), 0),
                     item: Either::Item('x')
                 },
                 ItemRow {
                     room_id: room_id.to_owned(),
-                    position: Position(CId(1), 1),
+                    position: Position::new(CId::new(1), 1),
                     item: Either::Item('y')
                 },
                 ItemRow {
                     room_id: room_id.to_owned(),
-                    position: Position(CId(1), 2),
+                    position: Position::new(CId::new(1), 2),
                     item: Either::Item('z')
                 },
             ],

From bfad2835ac8492e8167018c3e07ba22b100dedc8 Mon Sep 17 00:00:00 2001
From: Ivan Enderlin <ivan@mnt.io>
Date: Mon, 25 Nov 2024 15:53:28 +0100
Subject: [PATCH 7/9] fix(base): Move all fields of `MemoryStore` inside a
 `StdRwLock<_>`.

This patch creates a new `MemoryStoreInner` and moves all fields from
`MemoryStore` into this new type. All locks are removed, but a new lock
is added around `MemoryStoreInner`. That way we have a single lock.
---
 .../src/event_cache/store/memory_store.rs     | 58 ++++++++++++-------
 crates/matrix-sdk-common/src/store_locks.rs   |  7 +--
 .../src/store/memorystore.rs                  |  2 +-
 3 files changed, 42 insertions(+), 25 deletions(-)

diff --git a/crates/matrix-sdk-base/src/event_cache/store/memory_store.rs b/crates/matrix-sdk-base/src/event_cache/store/memory_store.rs
index c7cf9f7501f..ba9c3d24b80 100644
--- a/crates/matrix-sdk-base/src/event_cache/store/memory_store.rs
+++ b/crates/matrix-sdk-base/src/event_cache/store/memory_store.rs
@@ -34,9 +34,14 @@ use crate::{
 #[allow(clippy::type_complexity)]
 #[derive(Debug)]
 pub struct MemoryStore {
-    media: StdRwLock<RingBuffer<(OwnedMxcUri, String /* unique key */, Vec<u8>)>>,
-    leases: StdRwLock<HashMap<String, (String, Instant)>>,
-    events: StdRwLock<RelationalLinkedChunk<Event, Gap>>,
+    inner: StdRwLock<MemoryStoreInner>,
+}
+
+#[derive(Debug)]
+struct MemoryStoreInner {
+    media: RingBuffer<(OwnedMxcUri, String /* unique key */, Vec<u8>)>,
+    leases: HashMap<String, (String, Instant)>,
+    events: RelationalLinkedChunk<Event, Gap>,
 }
 
 // SAFETY: `new_unchecked` is safe because 20 is not zero.
@@ -45,9 +50,11 @@ const NUMBER_OF_MEDIAS: NonZeroUsize = unsafe { NonZeroUsize::new_unchecked(20)
 impl Default for MemoryStore {
     fn default() -> Self {
         Self {
-            media: StdRwLock::new(RingBuffer::new(NUMBER_OF_MEDIAS)),
-            leases: Default::default(),
-            events: StdRwLock::new(RelationalLinkedChunk::new()),
+            inner: StdRwLock::new(MemoryStoreInner {
+                media: RingBuffer::new(NUMBER_OF_MEDIAS),
+                leases: Default::default(),
+                events: RelationalLinkedChunk::new(),
+            }),
         }
     }
 }
@@ -70,7 +77,9 @@ impl EventCacheStore for MemoryStore {
         key: &str,
         holder: &str,
     ) -> Result<bool, Self::Error> {
-        Ok(try_take_leased_lock(&self.leases, lease_duration_ms, key, holder))
+        let mut inner = self.inner.write().unwrap();
+
+        Ok(try_take_leased_lock(&mut inner.leases, lease_duration_ms, key, holder))
     }
 
     async fn handle_linked_chunk_updates(
@@ -78,9 +87,9 @@ impl EventCacheStore for MemoryStore {
         room_id: &RoomId,
         updates: &[Update<Event, Gap>],
     ) -> Result<(), Self::Error> {
-        self.events.write().unwrap().apply_updates(room_id, updates);
+        let mut inner = self.inner.write().unwrap();
 
-        Ok(())
+        Ok(inner.events.apply_updates(updates))
     }
 
     async fn add_media_content(
@@ -90,8 +99,10 @@ impl EventCacheStore for MemoryStore {
     ) -> Result<()> {
         // Avoid duplication. Let's try to remove it first.
         self.remove_media_content(request).await?;
+
         // Now, let's add it.
-        self.media.write().unwrap().push((request.uri().to_owned(), request.unique_key(), data));
+        let mut inner = self.inner.write().unwrap();
+        inner.media.push((request.uri().to_owned(), request.unique_key(), data));
 
         Ok(())
     }
@@ -103,8 +114,10 @@ impl EventCacheStore for MemoryStore {
     ) -> Result<(), Self::Error> {
         let expected_key = from.unique_key();
 
-        let mut medias = self.media.write().unwrap();
-        if let Some((mxc, key, _)) = medias.iter_mut().find(|(_, key, _)| *key == expected_key) {
+        let mut inner = self.inner.write().unwrap();
+
+        if let Some((mxc, key, _)) = inner.media.iter_mut().find(|(_, key, _)| *key == expected_key)
+        {
             *mxc = to.uri().to_owned();
             *key = to.unique_key();
         }
@@ -115,8 +128,9 @@ impl EventCacheStore for MemoryStore {
     async fn get_media_content(&self, request: &MediaRequestParameters) -> Result<Option<Vec<u8>>> {
         let expected_key = request.unique_key();
 
-        let media = self.media.read().unwrap();
-        Ok(media.iter().find_map(|(_media_uri, media_key, media_content)| {
+        let inner = self.inner.write().unwrap();
+
+        Ok(inner.media.iter().find_map(|(_media_uri, media_key, media_content)| {
             (media_key == &expected_key).then(|| media_content.to_owned())
         }))
     }
@@ -124,23 +138,27 @@ impl EventCacheStore for MemoryStore {
     async fn remove_media_content(&self, request: &MediaRequestParameters) -> Result<()> {
         let expected_key = request.unique_key();
 
-        let mut media = self.media.write().unwrap();
-        let Some(index) = media
+        let mut inner = self.inner.write().unwrap();
+
+        let Some(index) = inner
+            .media
             .iter()
             .position(|(_media_uri, media_key, _media_content)| media_key == &expected_key)
         else {
             return Ok(());
         };
 
-        media.remove(index);
+        inner.media.remove(index);
 
         Ok(())
     }
 
     async fn remove_media_content_for_uri(&self, uri: &MxcUri) -> Result<()> {
-        let mut media = self.media.write().unwrap();
+        let mut inner = self.inner.write().unwrap();
+
         let expected_key = uri.to_owned();
-        let positions = media
+        let positions = inner
+            .media
             .iter()
             .enumerate()
             .filter_map(|(position, (media_uri, _media_key, _media_content))| {
@@ -150,7 +168,7 @@ impl EventCacheStore for MemoryStore {
 
         // Iterate in reverse-order so that positions stay valid after first removals.
         for position in positions.into_iter().rev() {
-            media.remove(position);
+            inner.media.remove(position);
         }
 
         Ok(())
diff --git a/crates/matrix-sdk-common/src/store_locks.rs b/crates/matrix-sdk-common/src/store_locks.rs
index e31acc5ca0b..08af735ed22 100644
--- a/crates/matrix-sdk-common/src/store_locks.rs
+++ b/crates/matrix-sdk-common/src/store_locks.rs
@@ -361,7 +361,7 @@ mod tests {
 
     impl TestStore {
         fn try_take_leased_lock(&self, lease_duration_ms: u32, key: &str, holder: &str) -> bool {
-            try_take_leased_lock(&self.leases, lease_duration_ms, key, holder)
+            try_take_leased_lock(&mut self.leases.write().unwrap(), lease_duration_ms, key, holder)
         }
     }
 
@@ -502,12 +502,11 @@ mod tests {
 pub mod memory_store_helper {
     use std::{
         collections::{hash_map::Entry, HashMap},
-        sync::RwLock,
         time::{Duration, Instant},
     };
 
     pub fn try_take_leased_lock(
-        leases: &RwLock<HashMap<String, (String, Instant)>>,
+        leases: &mut HashMap<String, (String, Instant)>,
         lease_duration_ms: u32,
         key: &str,
         holder: &str,
@@ -515,7 +514,7 @@ pub mod memory_store_helper {
         let now = Instant::now();
         let expiration = now + Duration::from_millis(lease_duration_ms.into());
 
-        match leases.write().unwrap().entry(key.to_owned()) {
+        match leases.entry(key.to_owned()) {
             // There is an existing holder.
             Entry::Occupied(mut entry) => {
                 let (current_holder, current_expiration) = entry.get_mut();
diff --git a/crates/matrix-sdk-crypto/src/store/memorystore.rs b/crates/matrix-sdk-crypto/src/store/memorystore.rs
index 6b177ea2641..8d5da350300 100644
--- a/crates/matrix-sdk-crypto/src/store/memorystore.rs
+++ b/crates/matrix-sdk-crypto/src/store/memorystore.rs
@@ -632,7 +632,7 @@ impl CryptoStore for MemoryStore {
         key: &str,
         holder: &str,
     ) -> Result<bool> {
-        Ok(try_take_leased_lock(&self.leases, lease_duration_ms, key, holder))
+        Ok(try_take_leased_lock(&mut self.leases.write().unwrap(), lease_duration_ms, key, holder))
     }
 }
 

From a178f62b7d66c503f72c5c21bdf2f21c35bd004b Mon Sep 17 00:00:00 2001
From: Ivan Enderlin <ivan@mnt.io>
Date: Mon, 25 Nov 2024 16:37:47 +0100
Subject: [PATCH 8/9] refactor: `EventCacheStore::handle_linked_chunk_updates`
 takes a `Vec<Update>`.

This patch updates `EventCacheStore::handle_linked_chunk_updates` to
take a `Vec<Update<Item, Gap>>` instead of `&[Update<Item, Gap>]`.
In fact, `linked_chunk::ObservableUpdates::take()` already returns a
`Vec<Update<Item, Gap>>`; we can simply forward this `Vec` up to here
without any further clones.
---
 .../src/event_cache/store/memory_store.rs     |  7 ++-
 .../src/event_cache/store/traits.rs           |  4 +-
 .../src/linked_chunk/relational.rs            | 58 +++++++++----------
 .../src/event_cache_store.rs                  |  2 +-
 4 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/crates/matrix-sdk-base/src/event_cache/store/memory_store.rs b/crates/matrix-sdk-base/src/event_cache/store/memory_store.rs
index ba9c3d24b80..92109029d3f 100644
--- a/crates/matrix-sdk-base/src/event_cache/store/memory_store.rs
+++ b/crates/matrix-sdk-base/src/event_cache/store/memory_store.rs
@@ -85,11 +85,12 @@ impl EventCacheStore for MemoryStore {
     async fn handle_linked_chunk_updates(
         &self,
         room_id: &RoomId,
-        updates: &[Update<Event, Gap>],
+        updates: Vec<Update<Event, Gap>>,
     ) -> Result<(), Self::Error> {
         let mut inner = self.inner.write().unwrap();
+        inner.events.apply_updates(room_id, updates);
 
-        Ok(inner.events.apply_updates(updates))
+        Ok(())
     }
 
     async fn add_media_content(
@@ -128,7 +129,7 @@ impl EventCacheStore for MemoryStore {
     async fn get_media_content(&self, request: &MediaRequestParameters) -> Result<Option<Vec<u8>>> {
         let expected_key = request.unique_key();
 
-        let inner = self.inner.write().unwrap();
+        let inner = self.inner.read().unwrap();
 
         Ok(inner.media.iter().find_map(|(_media_uri, media_key, media_content)| {
             (media_key == &expected_key).then(|| media_content.to_owned())
diff --git a/crates/matrix-sdk-base/src/event_cache/store/traits.rs b/crates/matrix-sdk-base/src/event_cache/store/traits.rs
index 2584b8881ed..3c35862b3c5 100644
--- a/crates/matrix-sdk-base/src/event_cache/store/traits.rs
+++ b/crates/matrix-sdk-base/src/event_cache/store/traits.rs
@@ -46,7 +46,7 @@ pub trait EventCacheStore: AsyncTraitDeps {
     async fn handle_linked_chunk_updates(
         &self,
         room_id: &RoomId,
-        updates: &[Update<Event, Gap>],
+        updates: Vec<Update<Event, Gap>>,
     ) -> Result<(), Self::Error>;
 
     /// Add a media file's content in the media store.
@@ -146,7 +146,7 @@ impl<T: EventCacheStore> EventCacheStore for EraseEventCacheStoreError<T> {
     async fn handle_linked_chunk_updates(
         &self,
         room_id: &RoomId,
-        updates: &[Update<Event, Gap>],
+        updates: Vec<Update<Event, Gap>>,
     ) -> Result<(), Self::Error> {
         self.0.handle_linked_chunk_updates(room_id, updates).await.map_err(Into::into)
     }
diff --git a/crates/matrix-sdk-common/src/linked_chunk/relational.rs b/crates/matrix-sdk-common/src/linked_chunk/relational.rs
index f7c2f9c7e24..56979ced5be 100644
--- a/crates/matrix-sdk-common/src/linked_chunk/relational.rs
+++ b/crates/matrix-sdk-common/src/linked_chunk/relational.rs
@@ -81,11 +81,7 @@ impl<Item, Gap> RelationalLinkedChunk<Item, Gap> {
 
     /// Apply [`Update`]s. That's the only way to write data inside this
     /// relational linked chunk.
-    pub fn apply_updates(&mut self, room_id: &RoomId, updates: &[Update<Item, Gap>])
-    where
-        Item: Clone,
-        Gap: Clone,
-    {
+    pub fn apply_updates(&mut self, room_id: &RoomId, updates: Vec<Update<Item, Gap>>) {
         for update in updates {
             match update {
                 Update::NewItemsChunk { previous, new, next } => {
@@ -96,8 +92,8 @@ impl<Item, Gap> RelationalLinkedChunk<Item, Gap> {
                     insert_chunk(&mut self.chunks, room_id, previous, new, next);
                     self.items.push(ItemRow {
                         room_id: room_id.to_owned(),
-                        position: Position::new(*new, 0),
-                        item: Either::Gap(gap.clone()),
+                        position: Position::new(new, 0),
+                        item: Either::Gap(gap),
                     });
                 }
 
@@ -111,7 +107,7 @@ impl<Item, Gap> RelationalLinkedChunk<Item, Gap> {
                         .filter_map(
                             |(nth, ItemRow { room_id: room_id_candidate, position, .. })| {
                                 (room_id == room_id_candidate
-                                    && position.chunk_identifier() == *chunk_identifier)
+                                    && position.chunk_identifier() == chunk_identifier)
                                     .then_some(nth)
                             },
                         )
@@ -122,14 +118,12 @@ impl<Item, Gap> RelationalLinkedChunk<Item, Gap> {
                     }
                 }
 
-                Update::PushItems { at, items } => {
-                    let mut at = *at;
-
+                Update::PushItems { mut at, items } => {
                     for item in items {
                         self.items.push(ItemRow {
                             room_id: room_id.to_owned(),
                             position: at,
-                            item: Either::Item(item.clone()),
+                            item: Either::Item(item),
                         });
                         at.increment_index();
                     }
@@ -147,7 +141,7 @@ impl<Item, Gap> RelationalLinkedChunk<Item, Gap> {
                         }
 
                         // Find the item to remove.
-                        if position == at {
+                        if *position == at {
                             debug_assert!(entry_to_remove.is_none(), "Found the same entry twice");
 
                             entry_to_remove = Some(nth);
@@ -191,21 +185,21 @@ impl<Item, Gap> RelationalLinkedChunk<Item, Gap> {
         fn insert_chunk(
             chunks: &mut Vec<ChunkRow>,
             room_id: &RoomId,
-            previous: &Option<ChunkIdentifier>,
-            new: &ChunkIdentifier,
-            next: &Option<ChunkIdentifier>,
+            previous: Option<ChunkIdentifier>,
+            new: ChunkIdentifier,
+            next: Option<ChunkIdentifier>,
         ) {
             // Find the previous chunk, and update its next chunk.
             if let Some(previous) = previous {
                 let entry_for_previous_chunk = chunks
                     .iter_mut()
                     .find(|ChunkRow { room_id: room_id_candidate, chunk, .. }| {
-                        room_id == room_id_candidate && chunk == previous
+                        room_id == room_id_candidate && *chunk == previous
                     })
                     .expect("Previous chunk should be present");
 
                 // Insert the chunk.
-                entry_for_previous_chunk.next_chunk = Some(*new);
+                entry_for_previous_chunk.next_chunk = Some(new);
             }
 
             // Find the next chunk, and update its previous chunk.
@@ -213,33 +207,33 @@ impl<Item, Gap> RelationalLinkedChunk<Item, Gap> {
                 let entry_for_next_chunk = chunks
                     .iter_mut()
                     .find(|ChunkRow { room_id: room_id_candidate, chunk, .. }| {
-                        room_id == room_id_candidate && chunk == next
+                        room_id == room_id_candidate && *chunk == next
                     })
                     .expect("Next chunk should be present");
 
                 // Insert the chunk.
-                entry_for_next_chunk.previous_chunk = Some(*new);
+                entry_for_next_chunk.previous_chunk = Some(new);
             }
 
             // Insert the chunk.
             chunks.push(ChunkRow {
                 room_id: room_id.to_owned(),
-                previous_chunk: *previous,
-                chunk: *new,
-                next_chunk: *next,
+                previous_chunk: previous,
+                chunk: new,
+                next_chunk: next,
             });
         }
 
         fn remove_chunk(
             chunks: &mut Vec<ChunkRow>,
             room_id: &RoomId,
-            chunk_to_remove: &ChunkIdentifier,
+            chunk_to_remove: ChunkIdentifier,
         ) {
             let entry_nth_to_remove = chunks
                 .iter()
                 .enumerate()
                 .find_map(|(nth, ChunkRow { room_id: room_id_candidate, chunk, .. })| {
-                    (room_id == room_id_candidate && chunk == chunk_to_remove).then_some(nth)
+                    (room_id == room_id_candidate && *chunk == chunk_to_remove).then_some(nth)
                 })
                 .expect("Remove an unknown chunk");
 
@@ -294,7 +288,7 @@ mod tests {
 
         relational_linked_chunk.apply_updates(
             room_id,
-            &[
+            vec![
                 // 0
                 Update::NewItemsChunk { previous: None, new: CId::new(0), next: None },
                 // 1 after 0
@@ -351,7 +345,7 @@ mod tests {
 
         relational_linked_chunk.apply_updates(
             room_id,
-            &[
+            vec![
                 // 0
                 Update::NewItemsChunk { previous: None, new: CId::new(0), next: None },
                 // 1 after 0
@@ -408,7 +402,7 @@ mod tests {
 
         relational_linked_chunk.apply_updates(
             room_id,
-            &[
+            vec![
                 // 0
                 Update::NewItemsChunk { previous: None, new: CId::new(0), next: None },
                 // 1 after 0
@@ -454,7 +448,7 @@ mod tests {
 
         relational_linked_chunk.apply_updates(
             room_id,
-            &[
+            vec![
                 // new chunk (this is not mandatory for this test, but let's try to be realistic)
                 Update::NewItemsChunk { previous: None, new: CId::new(0), next: None },
                 // new items on 0
@@ -541,7 +535,7 @@ mod tests {
 
         relational_linked_chunk.apply_updates(
             room_id,
-            &[
+            vec![
                 // new chunk (this is not mandatory for this test, but let's try to be realistic)
                 Update::NewItemsChunk { previous: None, new: CId::new(0), next: None },
                 // new items on 0
@@ -596,7 +590,7 @@ mod tests {
 
         relational_linked_chunk.apply_updates(
             room_id,
-            &[
+            vec![
                 // new chunk
                 Update::NewItemsChunk { previous: None, new: CId::new(0), next: None },
                 // new chunk
@@ -670,7 +664,7 @@ mod tests {
         let mut relational_linked_chunk = RelationalLinkedChunk::<char, ()>::new();
 
         relational_linked_chunk
-            .apply_updates(room_id, &[Update::StartReattachItems, Update::EndReattachItems]);
+            .apply_updates(room_id, vec![Update::StartReattachItems, Update::EndReattachItems]);
 
         // Nothing happened.
         assert!(relational_linked_chunk.chunks.is_empty());
diff --git a/crates/matrix-sdk-sqlite/src/event_cache_store.rs b/crates/matrix-sdk-sqlite/src/event_cache_store.rs
index f6b62e76216..5ee8d92324e 100644
--- a/crates/matrix-sdk-sqlite/src/event_cache_store.rs
+++ b/crates/matrix-sdk-sqlite/src/event_cache_store.rs
@@ -186,7 +186,7 @@ impl EventCacheStore for SqliteEventCacheStore {
     async fn handle_linked_chunk_updates(
         &self,
         _room_id: &RoomId,
-        _updates: &[Update<Event, Gap>],
+        _updates: Vec<Update<Event, Gap>>,
     ) -> Result<(), Self::Error> {
         todo!()
     }

From c788221da32534db49a8859f6ca50711c346c6e9 Mon Sep 17 00:00:00 2001
From: Ivan Enderlin <ivan@mnt.io>
Date: Mon, 25 Nov 2024 16:40:58 +0100
Subject: [PATCH 9/9] doc(common): Fix typos.

---
 .../src/linked_chunk/relational.rs                 | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/crates/matrix-sdk-common/src/linked_chunk/relational.rs b/crates/matrix-sdk-common/src/linked_chunk/relational.rs
index 56979ced5be..7f4983ca270 100644
--- a/crates/matrix-sdk-common/src/linked_chunk/relational.rs
+++ b/crates/matrix-sdk-common/src/linked_chunk/relational.rs
@@ -198,7 +198,7 @@ impl<Item, Gap> RelationalLinkedChunk<Item, Gap> {
                     })
                     .expect("Previous chunk should be present");
 
-                // Insert the chunk.
+                // Link the chunk.
                 entry_for_previous_chunk.next_chunk = Some(new);
             }
 
@@ -211,7 +211,7 @@ impl<Item, Gap> RelationalLinkedChunk<Item, Gap> {
                     })
                     .expect("Next chunk should be present");
 
-                // Insert the chunk.
+                // Link the chunk.
                 entry_for_next_chunk.previous_chunk = Some(new);
             }
 
@@ -360,7 +360,7 @@ mod tests {
             ],
         );
 
-        // Chunks are correctly links.
+        // Chunks are correctly linked.
         assert_eq!(
             relational_linked_chunk.chunks,
             &[
@@ -419,7 +419,7 @@ mod tests {
             ],
         );
 
-        // Chunks are correctly links.
+        // Chunks are correctly linked.
         assert_eq!(
             relational_linked_chunk.chunks,
             &[
@@ -462,7 +462,7 @@ mod tests {
             ],
         );
 
-        // Chunks are correctly links.
+        // Chunks are correctly linked.
         assert_eq!(
             relational_linked_chunk.chunks,
             &[
@@ -550,7 +550,7 @@ mod tests {
             ],
         );
 
-        // Chunks are correctly links.
+        // Chunks are correctly linked.
         assert_eq!(
             relational_linked_chunk.chunks,
             &[ChunkRow {
@@ -607,7 +607,7 @@ mod tests {
             ],
         );
 
-        // Chunks are correctly links.
+        // Chunks are correctly linked.
         assert_eq!(
             relational_linked_chunk.chunks,
             &[