diff --git a/crates/matrix-sdk-base/CHANGELOG.md b/crates/matrix-sdk-base/CHANGELOG.md index d8c752b80d9..0441ecef6ab 100644 --- a/crates/matrix-sdk-base/CHANGELOG.md +++ b/crates/matrix-sdk-base/CHANGELOG.md @@ -20,6 +20,9 @@ All notable changes to this project will be documented in this file. ([#5390](https://github.com/matrix-org/matrix-rust-sdk/pull/5390)) ### Refactor +- [**breaking**] `RelationalLinkedChunk::items` now takes a `RoomId` instead of an + `&OwnedLinkedChunkId` parameter. + ([#5445](https://github.com/matrix-org/matrix-rust-sdk/pull/5445)) - [**breaking**] Add an `IsPrefix = False` bound to the `get_state_event_static()`, `get_state_event_static_for_key()` and `get_state_events_static()`, `get_account_data_event_static()` and diff --git a/crates/matrix-sdk-base/src/event_cache/store/integration_tests.rs b/crates/matrix-sdk-base/src/event_cache/store/integration_tests.rs index e656c8cd1fa..e6f2ab0155b 100644 --- a/crates/matrix-sdk-base/src/event_cache/store/integration_tests.rs +++ b/crates/matrix-sdk-base/src/event_cache/store/integration_tests.rs @@ -17,6 +17,7 @@ use std::{collections::BTreeMap, sync::Arc}; use assert_matches::assert_matches; +use assert_matches2::assert_let; use matrix_sdk_common::{ deserialized_responses::{ AlgorithmInfo, DecryptedRoomEvent, EncryptionInfo, TimelineEvent, TimelineEventKind, @@ -154,6 +155,10 @@ pub trait EventCacheStoreIntegrationTests { /// Test that saving an event works as expected. async fn test_save_event(&self); + + /// Test multiple things related to distinguishing a thread linked chunk + /// from a room linked chunk. + async fn test_thread_vs_room_linked_chunk(&self); } impl EventCacheStoreIntegrationTests for DynEventCacheStore { @@ -1153,6 +1158,139 @@ impl EventCacheStoreIntegrationTests for DynEventCacheStore { .is_none() ); } + + async fn test_thread_vs_room_linked_chunk(&self) { + let room_id = room_id!("!r0:matrix.org"); + + let event = |msg: &str| make_test_event(room_id, msg); + + let thread1_ev = event("comté"); + let thread2_ev = event("gruyère"); + let thread2_ev2 = event("beaufort"); + let room_ev = event("brillat savarin triple crème"); + + let thread_root1 = event("thread1"); + let thread_root2 = event("thread2"); + + // Add one event in a thread linked chunk. + self.handle_linked_chunk_updates( + LinkedChunkId::Thread(room_id, thread_root1.event_id().unwrap().as_ref()), + vec![ + Update::NewItemsChunk { previous: None, new: CId::new(0), next: None }, + Update::PushItems { + at: Position::new(CId::new(0), 0), + items: vec![thread1_ev.clone()], + }, + ], + ) + .await + .unwrap(); + + // Add one event in another thread linked chunk (same room). + self.handle_linked_chunk_updates( + LinkedChunkId::Thread(room_id, thread_root2.event_id().unwrap().as_ref()), + vec![ + Update::NewItemsChunk { previous: None, new: CId::new(0), next: None }, + Update::PushItems { + at: Position::new(CId::new(0), 0), + items: vec![thread2_ev.clone(), thread2_ev2.clone()], + }, + ], + ) + .await + .unwrap(); + + // Add another event to the room linked chunk. + self.handle_linked_chunk_updates( + LinkedChunkId::Room(room_id), + vec![ + Update::NewItemsChunk { previous: None, new: CId::new(0), next: None }, + Update::PushItems { + at: Position::new(CId::new(0), 0), + items: vec![room_ev.clone()], + }, + ], + ) + .await + .unwrap(); + + // All the events can be found with `find_event()` for the room. + self.find_event(room_id, thread2_ev.event_id().unwrap().as_ref()) + .await + .expect("failed to query for finding an event") + .expect("failed to find thread1_ev"); + + self.find_event(room_id, thread2_ev.event_id().unwrap().as_ref()) + .await + .expect("failed to query for finding an event") + .expect("failed to find thread2_ev"); + + self.find_event(room_id, thread2_ev2.event_id().unwrap().as_ref()) + .await + .expect("failed to query for finding an event") + .expect("failed to find thread2_ev2"); + + self.find_event(room_id, room_ev.event_id().unwrap().as_ref()) + .await + .expect("failed to query for finding an event") + .expect("failed to find room_ev"); + + // Finding duplicates operates based on the linked chunk id. + let dups = self + .filter_duplicated_events( + LinkedChunkId::Thread(room_id, thread_root1.event_id().unwrap().as_ref()), + vec![ + thread1_ev.event_id().unwrap().to_owned(), + room_ev.event_id().unwrap().to_owned(), + ], + ) + .await + .unwrap(); + assert_eq!(dups.len(), 1); + assert_eq!(dups[0].0, thread1_ev.event_id().unwrap()); + + // Loading all chunks operates based on the linked chunk id. + let all_chunks = self + .load_all_chunks(LinkedChunkId::Thread( + room_id, + thread_root2.event_id().unwrap().as_ref(), + )) + .await + .unwrap(); + assert_eq!(all_chunks.len(), 1); + assert_eq!(all_chunks[0].identifier, CId::new(0)); + assert_let!(ChunkContent::Items(observed_items) = all_chunks[0].content.clone()); + assert_eq!(observed_items.len(), 2); + assert_eq!(observed_items[0].event_id(), thread2_ev.event_id()); + assert_eq!(observed_items[1].event_id(), thread2_ev2.event_id()); + + // Loading the metadata of all chunks operates based on the linked chunk + // id. + let metas = self + .load_all_chunks_metadata(LinkedChunkId::Thread( + room_id, + thread_root2.event_id().unwrap().as_ref(), + )) + .await + .unwrap(); + assert_eq!(metas.len(), 1); + assert_eq!(metas[0].identifier, CId::new(0)); + assert_eq!(metas[0].num_items, 2); + + // Loading the last chunk operates based on the linked chunk id. + let (last_chunk, _chunk_identifier_generator) = self + .load_last_chunk(LinkedChunkId::Thread( + room_id, + thread_root1.event_id().unwrap().as_ref(), + )) + .await + .unwrap(); + let last_chunk = last_chunk.unwrap(); + assert_eq!(last_chunk.identifier, CId::new(0)); + assert_let!(ChunkContent::Items(observed_items) = last_chunk.content); + assert_eq!(observed_items.len(), 1); + assert_eq!(observed_items[0].event_id(), thread1_ev.event_id()); + } } /// Macro building to allow your `EventCacheStore` implementation to run the @@ -1277,6 +1415,13 @@ macro_rules! event_cache_store_integration_tests { get_event_cache_store().await.unwrap().into_event_cache_store(); event_cache_store.test_save_event().await; } + + #[async_test] + async fn test_thread_vs_room_linked_chunk() { + let event_cache_store = + get_event_cache_store().await.unwrap().into_event_cache_store(); + event_cache_store.test_thread_vs_room_linked_chunk().await; + } } }; } 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 8087164a147..cfbb44cd371 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 @@ -21,8 +21,8 @@ use std::{ use async_trait::async_trait; use matrix_sdk_common::{ linked_chunk::{ - ChunkIdentifier, ChunkIdentifierGenerator, ChunkMetadata, LinkedChunkId, - OwnedLinkedChunkId, Position, RawChunk, Update, relational::RelationalLinkedChunk, + ChunkIdentifier, ChunkIdentifierGenerator, ChunkMetadata, LinkedChunkId, Position, + RawChunk, Update, relational::RelationalLinkedChunk, }, ring_buffer::RingBuffer, store_locks::memory_store_helper::try_take_leased_lock, @@ -222,11 +222,9 @@ impl EventCacheStore for MemoryStore { ) -> Result, Self::Error> { let inner = self.inner.read().unwrap(); - let target_linked_chunk_id = OwnedLinkedChunkId::Room(room_id.to_owned()); - let event = inner .events - .items(&target_linked_chunk_id) + .items(room_id) .find_map(|(event, _pos)| (event.event_id()? == event_id).then_some(event.clone())); Ok(event) @@ -240,13 +238,11 @@ impl EventCacheStore for MemoryStore { ) -> Result)>, Self::Error> { let inner = self.inner.read().unwrap(); - let target_linked_chunk_id = OwnedLinkedChunkId::Room(room_id.to_owned()); - let filters = compute_filters_string(filters); let related_events = inner .events - .items(&target_linked_chunk_id) + .items(room_id) .filter_map(|(event, pos)| { // Must have a relation. let (related_to, rel_type) = extract_event_relation(event.raw())?; 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 a4e5376a2ac..84f46ac7134 100644 --- a/crates/matrix-sdk-base/src/event_cache/store/traits.rs +++ b/crates/matrix-sdk-base/src/event_cache/store/traits.rs @@ -128,6 +128,9 @@ pub trait EventCacheStore: AsyncTraitDeps { ) -> Result, Self::Error>; /// Find an event by its ID in a room. + /// + /// This method must return events saved either in any linked chunks, *or* + /// events saved "out-of-band" with the [`Self::save_event`] method. async fn find_event( &self, room_id: &RoomId, @@ -147,6 +150,9 @@ pub trait EventCacheStore: AsyncTraitDeps { /// /// An additional filter can be provided to only retrieve related events for /// a certain relationship. + /// + /// This method must return events saved either in any linked chunks, *or* + /// events saved "out-of-band" with the [`Self::save_event`] method. async fn find_event_relations( &self, room_id: &RoomId, diff --git a/crates/matrix-sdk-common/src/linked_chunk/mod.rs b/crates/matrix-sdk-common/src/linked_chunk/mod.rs index 138dcca5641..a4175b1dcdd 100644 --- a/crates/matrix-sdk-common/src/linked_chunk/mod.rs +++ b/crates/matrix-sdk-common/src/linked_chunk/mod.rs @@ -106,27 +106,30 @@ use std::{ pub use as_vector::*; pub use order_tracker::OrderTracker; -use ruma::{OwnedRoomId, RoomId}; +use ruma::{EventId, OwnedEventId, OwnedRoomId, RoomId}; pub use updates::*; /// An identifier for a linked chunk; borrowed variant. #[derive(Debug, Clone, Copy, PartialEq)] pub enum LinkedChunkId<'a> { Room(&'a RoomId), - // TODO(bnjbvr): Soon™. - // Thread(&'a RoomId, &'a EventId), + Thread(&'a RoomId, &'a EventId), } impl LinkedChunkId<'_> { pub fn storage_key(&self) -> impl '_ + AsRef<[u8]> { match self { - LinkedChunkId::Room(room_id) => room_id, + LinkedChunkId::Room(room_id) => room_id.to_string(), + LinkedChunkId::Thread(room_id, event_id) => format!("t:{room_id}:{event_id}"), } } pub fn to_owned(&self) -> OwnedLinkedChunkId { match self { LinkedChunkId::Room(room_id) => OwnedLinkedChunkId::Room((*room_id).to_owned()), + LinkedChunkId::Thread(room_id, event_id) => { + OwnedLinkedChunkId::Thread((*room_id).to_owned(), (*event_id).to_owned()) + } } } } @@ -135,6 +138,11 @@ impl PartialEq<&OwnedLinkedChunkId> for LinkedChunkId<'_> { fn eq(&self, other: &&OwnedLinkedChunkId) -> bool { match (self, other) { (LinkedChunkId::Room(a), OwnedLinkedChunkId::Room(b)) => *a == b, + (LinkedChunkId::Thread(r, ev), OwnedLinkedChunkId::Thread(r2, ev2)) => { + r == r2 && ev == ev2 + } + (LinkedChunkId::Room(..), OwnedLinkedChunkId::Thread(..)) + | (LinkedChunkId::Thread(..), OwnedLinkedChunkId::Room(..)) => false, } } } @@ -149,14 +157,16 @@ impl PartialEq> for OwnedLinkedChunkId { #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub enum OwnedLinkedChunkId { Room(OwnedRoomId), - // TODO(bnjbvr): Soon™. - // Thread(OwnedRoomId, OwnedEventId), + Thread(OwnedRoomId, OwnedEventId), } impl Display for OwnedLinkedChunkId { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { OwnedLinkedChunkId::Room(room_id) => write!(f, "{room_id}"), + OwnedLinkedChunkId::Thread(room_id, thread_root) => { + write!(f, "{room_id}:thread:{thread_root}") + } } } } @@ -166,12 +176,16 @@ impl OwnedLinkedChunkId { fn as_ref(&self) -> LinkedChunkId<'_> { match self { OwnedLinkedChunkId::Room(room_id) => LinkedChunkId::Room(room_id.as_ref()), + OwnedLinkedChunkId::Thread(room_id, event_id) => { + LinkedChunkId::Thread(room_id.as_ref(), event_id.as_ref()) + } } } pub fn room_id(&self) -> &RoomId { match self { OwnedLinkedChunkId::Room(room_id) => room_id, + OwnedLinkedChunkId::Thread(room_id, ..) => room_id, } } } diff --git a/crates/matrix-sdk-common/src/linked_chunk/relational.rs b/crates/matrix-sdk-common/src/linked_chunk/relational.rs index 20714c46cee..441724c0d78 100644 --- a/crates/matrix-sdk-common/src/linked_chunk/relational.rs +++ b/crates/matrix-sdk-common/src/linked_chunk/relational.rs @@ -20,7 +20,7 @@ use std::{ hash::Hash, }; -use ruma::{OwnedEventId, OwnedRoomId}; +use ruma::{OwnedEventId, OwnedRoomId, RoomId}; use super::{ChunkContent, ChunkIdentifierGenerator, RawChunk}; use crate::{ @@ -376,20 +376,22 @@ where }) } - /// Return an iterator over all items of a given linked chunk, along with - /// their positions, if available. + /// Return an iterator over all items of all linked chunks of a room, along + /// with their positions, if available. /// /// The only items which will NOT have a position are those saved with /// [`Self::save_item`]. /// /// This will include out-of-band items. - pub fn items( - &self, - target: &OwnedLinkedChunkId, - ) -> impl Iterator)> { + pub fn items<'a>( + &'a self, + room_id: &'a RoomId, + ) -> impl Iterator)> { self.items - .get(target) - .into_iter() + .iter() + .filter_map(move |(linked_chunk_id, items)| { + (linked_chunk_id.room_id() == room_id).then_some(items) + }) .flat_map(|items| items.values().map(|(item, pos)| (item, *pos))) }