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 6f63c8255d8..87f49310376 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,15 +16,17 @@ use std::{collections::HashMap, num::NonZeroUsize, sync::RwLock as StdRwLock, ti use async_trait::async_trait; use matrix_sdk_common::{ - linked_chunk::{LinkedChunk, Update}, + linked_chunk::Update, ring_buffer::RingBuffer, - store_locks::memory_store_helper::{handle_linked_chunk_updates, try_take_leased_lock}, + store_locks::memory_store_helper::{ + relational_linked_chunk::RelationalLinkedChunk, try_take_leased_lock, + }, }; use ruma::{MxcUri, OwnedMxcUri}; use super::{EventCacheStore, EventCacheStoreError, Result}; use crate::{ - event_cache::{Event, Gap, DEFAULT_CHUNK_CAPACITY}, + event_cache::{Event, Gap}, media::{MediaRequestParameters, UniqueKey as _}, }; @@ -36,7 +38,7 @@ use crate::{ pub struct MemoryStore { media: StdRwLock)>>, leases: StdRwLock>, - events: StdRwLock>, + events: StdRwLock>, } // SAFETY: `new_unchecked` is safe because 20 is not zero. @@ -47,7 +49,7 @@ impl Default for MemoryStore { Self { media: StdRwLock::new(RingBuffer::new(NUMBER_OF_MEDIAS)), leases: Default::default(), - events: StdRwLock::new(LinkedChunk::new()), + events: StdRwLock::new(RelationalLinkedChunk::new()), } } } @@ -77,7 +79,7 @@ impl EventCacheStore for MemoryStore { &self, updates: &[Update], ) -> Result<(), Self::Error> { - Ok(handle_linked_chunk_updates(&self.events, updates)) + Ok(self.events.write().unwrap().apply_updates(updates)) } async fn add_media_content( diff --git a/crates/matrix-sdk-common/src/store_locks.rs b/crates/matrix-sdk-common/src/store_locks.rs index 6c638e881ab..95ba05b5e82 100644 --- a/crates/matrix-sdk-common/src/store_locks.rs +++ b/crates/matrix-sdk-common/src/store_locks.rs @@ -557,13 +557,14 @@ pub mod memory_store_helper { type Chunks = Vec<(Option, ChunkIdentifier, Option)>; - #[derive(Debug)] + #[derive(Debug, PartialEq)] enum Content { Item(Item), Gap(Gap), } - /// A [`LinkedChunk`] but with a relational layout, similar to what we would have in a database. + /// A [`LinkedChunk`] but with a relational layout, similar to what we + /// would have in a database. /// /// This is used by memory stores. #[derive(Debug)] @@ -576,148 +577,145 @@ pub mod memory_store_helper { pub fn new() -> Self { Self { chunks: Vec::new(), items: Vec::new() } } - } - - pub fn handle_linked_chunk_updates( - linked_chunk: &mut RelationalLinkedChunk, - updates: &[Update], - ) where - Item: Clone, - Gap: Clone, - { - for update in updates { - match update { - Update::NewItemsChunk { previous, new, next } => { - insert_chunk(&mut linked_chunk.chunks, previous, new, next); - } - Update::NewGapChunk { previous, new, next, gap } => { - insert_chunk(&mut linked_chunk.chunks, previous, new, next); - linked_chunk - .items - .push((Position::new(*new, 0), Content::Gap(gap.clone()))); - } - - Update::RemoveChunk(chunk_identifier) => { - remove_chunk(&mut linked_chunk.chunks, chunk_identifier); + pub fn apply_updates(&mut self, updates: &[Update]) + where + Item: Clone, + Gap: Clone, + { + for update in updates { + match update { + Update::NewItemsChunk { previous, new, next } => { + insert_chunk(&mut self.chunks, previous, new, next); + } - let indices_to_remove = linked_chunk - .items - .iter() - .enumerate() - .filter_map(|(nth, (position, _))| { - (position.chunk_identifier() == *chunk_identifier).then_some(nth) - }) - .collect::>(); + Update::NewGapChunk { previous, new, next, gap } => { + insert_chunk(&mut self.chunks, previous, new, next); + self.items.push((Position::new(*new, 0), Content::Gap(gap.clone()))); + } - for index_to_remove in indices_to_remove.into_iter().rev() { - linked_chunk.items.remove(index_to_remove); + Update::RemoveChunk(chunk_identifier) => { + remove_chunk(&mut self.chunks, chunk_identifier); + + let indices_to_remove = self + .items + .iter() + .enumerate() + .filter_map(|(nth, (position, _))| { + (position.chunk_identifier() == *chunk_identifier) + .then_some(nth) + }) + .collect::>(); + + 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.clone(); + Update::PushItems { at, items } => { + let mut at = at.clone(); - for item in items { - linked_chunk.items.push((at.clone(), Content::Item(item.clone()))); - at.increment_index(); + for item in items { + self.items.push((at.clone(), Content::Item(item.clone()))); + at.increment_index(); + } } - } - Update::RemoveItem { at } => { - let entry_to_remove = linked_chunk - .items - .iter() - .position(|(position, _)| position == at) - .expect("Remove an unkwnown item"); + Update::RemoveItem { at } => { + let entry_to_remove = self + .items + .iter() + .position(|(position, _)| position == at) + .expect("Remove an unkwnown item"); - linked_chunk.items.remove(entry_to_remove); - } + self.items.remove(entry_to_remove); + } - Update::DetachLastItems { at } => { - let indices_to_remove = linked_chunk - .items - .iter() - .enumerate() - .filter_map(|(nth, (position, _))| { - (position.chunk_identifier() == at.chunk_identifier() - && position.index() >= at.index()) - .then_some(nth) - }) - .collect::>(); - - for index_to_remove in indices_to_remove.into_iter().rev() { - linked_chunk.items.remove(index_to_remove); + Update::DetachLastItems { at } => { + let indices_to_remove = self + .items + .iter() + .enumerate() + .filter_map(|(nth, (position, _))| { + (position.chunk_identifier() == at.chunk_identifier() + && position.index() >= at.index()) + .then_some(nth) + }) + .collect::>(); + + for index_to_remove in indices_to_remove.into_iter().rev() { + self.items.remove(index_to_remove); + } } - } - Update::StartReattachItems | Update::EndReattachItems => { /* nothing */ } + Update::StartReattachItems | Update::EndReattachItems => { /* nothing */ } + } } - } - fn insert_chunk( - chunks: &mut Chunks, - previous: &Option, - new: &ChunkIdentifier, - next: &Option, - ) { - // Find the previous chunk, and update its next chunk. - if let Some(previous) = previous { - let entry_for_previous_chunk = chunks - .iter_mut() - .find(|(_, chunk_candidate, _)| chunk_candidate == previous) - .expect("Previous chunk should be present"); + fn insert_chunk( + chunks: &mut Chunks, + previous: &Option, + new: &ChunkIdentifier, + next: &Option, + ) { + // Find the previous chunk, and update its next chunk. + if let Some(previous) = previous { + let entry_for_previous_chunk = chunks + .iter_mut() + .find(|(_, chunk_candidate, _)| chunk_candidate == previous) + .expect("Previous chunk should be present"); + + // Insert the chunk. + entry_for_previous_chunk.2 = Some(*new); + } - // Insert the chunk. - entry_for_previous_chunk.2 = 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(|(_, chunk_candidate, _)| chunk_candidate == next) + .expect("Next chunk should be present"); - // Find the next chunk, and update its previous chunk. - if let Some(next) = next { - let entry_for_next_chunk = chunks - .iter_mut() - .find(|(_, chunk_candidate, _)| chunk_candidate == next) - .expect("Next chunk should be present"); + // Insert the chunk. + entry_for_next_chunk.0 = Some(*new); + } // Insert the chunk. - entry_for_next_chunk.0 = Some(*new); + chunks.push((*previous, *new, *next)); } - // Insert the chunk. - chunks.push((*previous, *new, *next)); - } - - fn remove_chunk(chunks: &mut Chunks, chunk: &ChunkIdentifier) { - let entry_nth_to_remove = chunks - .iter() - .enumerate() - .find_map(|(nth, (_, chunk_candidate, _))| { - (chunk_candidate == chunk).then_some(nth) - }) - .expect("Remove an unknown chunk"); - - let (previous, _, 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(|(_, chunk_candidate, _)| *chunk_candidate == previous) - .expect("Previous chunk should be present"); - - // Insert the chunk. - entry_for_previous_chunk.2 = next; - } + fn remove_chunk(chunks: &mut Chunks, chunk: &ChunkIdentifier) { + let entry_nth_to_remove = chunks + .iter() + .enumerate() + .find_map(|(nth, (_, chunk_candidate, _))| { + (chunk_candidate == chunk).then_some(nth) + }) + .expect("Remove an unknown chunk"); + + let (previous, _, 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(|(_, chunk_candidate, _)| *chunk_candidate == previous) + .expect("Previous chunk should be present"); + + // Insert the chunk. + entry_for_previous_chunk.2 = next; + } - // Find the next chunk, and update its previous chunk. - if let Some(next) = next { - let entry_for_next_chunk = chunks - .iter_mut() - .find(|(_, chunk_candidate, _)| *chunk_candidate == next) - .expect("Next chunk should be present"); + // Find the next chunk, and update its previous chunk. + if let Some(next) = next { + let entry_for_next_chunk = chunks + .iter_mut() + .find(|(_, chunk_candidate, _)| *chunk_candidate == next) + .expect("Next chunk should be present"); - // Insert the chunk. - entry_for_next_chunk.0 = previous; + // Insert the chunk. + entry_for_next_chunk.0 = previous; + } } } } @@ -736,24 +734,22 @@ pub mod memory_store_helper { fn test_new_items_chunk() { let mut relational_linked_chunk = RelationalLinkedChunk::::new(); - handle_linked_chunk_updates( - &mut relational_linked_chunk, - &[ - // 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(&[ + // 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, &[ @@ -761,16 +757,74 @@ pub mod memory_store_helper { (Some(cid!(0)), cid!(1), None), (None, cid!(2), Some(cid!(3))), (Some(cid!(2)), cid!(3), 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::::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, + &[ + (None, cid!(0), Some(cid!(1))), + (Some(cid!(0)), cid!(1), Some(cid!(2))), + (Some(cid!(1)), cid!(2), None), + ], + ); + // Items contains the gap. + assert_eq!( + relational_linked_chunk.items, + &[(Position::new(cid!(1), 0), Content::Gap(()))], + ); + } + + #[test] + fn test_remove_chunk() { + let mut relational_linked_chunk = RelationalLinkedChunk::::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, + &[(None, cid!(0), Some(cid!(2))), (Some(cid!(0)), cid!(2), None),], + ); + // Items no longer contains the gap. + assert!(relational_linked_chunk.items.is_empty()); } - */ } } }