fix(base): Check the lazy_previous of the first chunk matches the new first chunk.

This patch adds a new check when inserting a new first chunk. It makes
some tests to fail but because they were not realistic. This patch then
updates these tests.

Author: Hywan

crates/matrix-sdk-base/src/event_cache/store/integration_tests.rs

@@ -459,6 +459,7 @@ impl EventCacheStoreIntegrationTests for DynEventCacheStore {
             // A unique chunk.
             assert_matches!(rchunks.next(), Some(chunk) => {
                 assert_eq!(chunk.identifier(), 2);
+                assert_eq!(chunk.lazy_previous(), Some(CId::new(1)));
 
                 assert_matches!(chunk.content(), ChunkContent::Items(events) => {
                     assert_eq!(events.len(), 3);
@@ -476,19 +477,17 @@ impl EventCacheStoreIntegrationTests for DynEventCacheStore {
         // Load the previous chunk: this is a gap.
         {
             let first_chunk = linked_chunk.chunks().next().unwrap().identifier();
-            let mut previous_chunk =
+            let previous_chunk =
                 self.load_previous_chunk(room_id, first_chunk).await.unwrap().unwrap();
 
-            // Pretend it's the first chunk.
-            previous_chunk.previous = None;
-
             let _ = lazy_loader::insert_new_first_chunk(&mut linked_chunk, previous_chunk).unwrap();
 
             let mut rchunks = linked_chunk.rchunks();
 
             // The last chunk.
             assert_matches!(rchunks.next(), Some(chunk) => {
                 assert_eq!(chunk.identifier(), 2);
+                assert!(chunk.lazy_previous().is_none());
 
                 // Already asserted, but let's be sure nothing breaks.
                 assert_matches!(chunk.content(), ChunkContent::Items(events) => {
@@ -502,6 +501,7 @@ impl EventCacheStoreIntegrationTests for DynEventCacheStore {
             // The new chunk.
             assert_matches!(rchunks.next(), Some(chunk) => {
                 assert_eq!(chunk.identifier(), 1);
+                assert_eq!(chunk.lazy_previous(), Some(CId::new(0)));
 
                 assert_matches!(chunk.content(), ChunkContent::Gap(gap) => {
                     assert_eq!(gap.prev_token, "morbier");
@@ -524,6 +524,7 @@ impl EventCacheStoreIntegrationTests for DynEventCacheStore {
             // The last chunk.
             assert_matches!(rchunks.next(), Some(chunk) => {
                 assert_eq!(chunk.identifier(), 2);
+                assert!(chunk.lazy_previous().is_none());
 
                 // Already asserted, but let's be sure nothing breaks.
                 assert_matches!(chunk.content(), ChunkContent::Items(events) => {
@@ -537,6 +538,7 @@ impl EventCacheStoreIntegrationTests for DynEventCacheStore {
             // Its previous chunk.
             assert_matches!(rchunks.next(), Some(chunk) => {
                 assert_eq!(chunk.identifier(), 1);
+                assert!(chunk.lazy_previous().is_none());
 
                 // Already asserted, but let's be sure nothing breaks.
                 assert_matches!(chunk.content(), ChunkContent::Gap(gap) => {
@@ -547,6 +549,7 @@ impl EventCacheStoreIntegrationTests for DynEventCacheStore {
             // The new chunk.
             assert_matches!(rchunks.next(), Some(chunk) => {
                 assert_eq!(chunk.identifier(), 0);
+                assert!(chunk.lazy_previous().is_none());
 
                 assert_matches!(chunk.content(), ChunkContent::Items(events) => {
                     assert_eq!(events.len(), 2);
@@ -574,6 +577,7 @@ impl EventCacheStoreIntegrationTests for DynEventCacheStore {
             // The first chunk.
             assert_matches!(chunks.next(), Some(chunk) => {
                 assert_eq!(chunk.identifier(), 0);
+                assert!(chunk.lazy_previous().is_none());
 
                 assert_matches!(chunk.content(), ChunkContent::Items(events) => {
                     assert_eq!(events.len(), 2);
@@ -585,6 +589,7 @@ impl EventCacheStoreIntegrationTests for DynEventCacheStore {
             // The second chunk.
             assert_matches!(chunks.next(), Some(chunk) => {
                 assert_eq!(chunk.identifier(), 1);
+                assert!(chunk.lazy_previous().is_none());
 
                 assert_matches!(chunk.content(), ChunkContent::Gap(gap) => {
                     assert_eq!(gap.prev_token, "morbier");
@@ -594,6 +599,7 @@ impl EventCacheStoreIntegrationTests for DynEventCacheStore {
             // The third and last chunk.
             assert_matches!(chunks.next(), Some(chunk) => {
                 assert_eq!(chunk.identifier(), 2);
+                assert!(chunk.lazy_previous().is_none());
 
                 assert_matches!(chunk.content(), ChunkContent::Items(events) => {
                     assert_eq!(events.len(), 3);

crates/matrix-sdk-common/src/linked_chunk/lazy_loader.rs

@@ -96,7 +96,8 @@ where
             }
         }
 
-        let expected_next_chunk = linked_chunk.links.first_chunk().identifier();
+        let first_chunk = linked_chunk.links.first_chunk();
+        let expected_next_chunk = first_chunk.identifier();
 
         // New chunk has a next chunk.
         let Some(next_chunk) = new_first_chunk.next else {
@@ -111,15 +112,25 @@ where
             });
         }
 
+        // Same check as before, but in reverse: the first chunk has a `lazy_previous`
+        // to the new first chunk.
+        if first_chunk.lazy_previous() != Some(new_first_chunk.identifier) {
+            return Err(LazyLoaderError::CannotConnectTwoChunks {
+                new_chunk: first_chunk.identifier,
+                with_chunk: new_first_chunk.identifier,
+            });
+        }
+
         // Alright. All checks are made.
     }
 
     // Insert the new first chunk.
     {
         // Transform the `RawChunk` into a `Chunk`.
         let lazy_previous = new_first_chunk.previous.take();
+        let new_first_chunk_identifier = new_first_chunk.identifier;
         let mut new_first_chunk =
-            Chunk::new_leaked(new_first_chunk.identifier, new_first_chunk.content);
+            Chunk::new_leaked(new_first_chunk_identifier, new_first_chunk.content);
 
         let links = &mut linked_chunk.links;
 
@@ -524,6 +535,26 @@ mod tests {
         });
     }
 
+    #[test]
+    fn test_insert_new_first_chunk_err_cannot_connect_two_chunks_before_no_lazy_previous() {
+        let new_first_chunk = RawChunk {
+            previous: None,
+            identifier: ChunkIdentifier::new(1),
+            next: Some(ChunkIdentifier::new(0)),
+            content: ChunkContent::Gap(()),
+        };
+
+        let mut linked_chunk = LinkedChunk::<2, char, ()>::new();
+        linked_chunk.push_gap_back(());
+
+        let result = insert_new_first_chunk(&mut linked_chunk, new_first_chunk);
+
+        assert_matches!(result, Err(LazyLoaderError::CannotConnectTwoChunks { new_chunk, with_chunk }) => {
+            assert_eq!(new_chunk, 0);
+            assert_eq!(with_chunk, 1);
+        });
+    }
+
     #[test]
     fn test_insert_new_first_chunk_gap() {
         let new_first_chunk = RawChunk {
@@ -535,6 +566,7 @@ mod tests {
 
         let mut linked_chunk = LinkedChunk::<5, char, ()>::new_with_update_history();
         linked_chunk.push_items_back(vec!['a', 'b']);
+        linked_chunk.links.first_chunk_mut().lazy_previous = Some(ChunkIdentifier::new(1));
 
         // Drain initial updates.
         let _ = linked_chunk.updates().unwrap().take();
@@ -593,6 +625,7 @@ mod tests {
 
         let mut linked_chunk = LinkedChunk::<5, char, ()>::new_with_update_history();
         linked_chunk.push_items_back(vec!['a', 'b']);
+        linked_chunk.links.first_chunk_mut().lazy_previous = Some(ChunkIdentifier::new(1));
 
         // Drain initial updates.
         let _ = linked_chunk.updates().unwrap().take();