diff --git a/crates/matrix-sdk-base/src/lib.rs b/crates/matrix-sdk-base/src/lib.rs index ba165ba8026..cc4e5128403 100644 --- a/crates/matrix-sdk-base/src/lib.rs +++ b/crates/matrix-sdk-base/src/lib.rs @@ -37,7 +37,6 @@ mod rooms; pub mod read_receipts; pub use read_receipts::PreviousEventsProvider; -pub use rooms::RoomMembersUpdate; pub mod sliding_sync; pub mod store; @@ -56,9 +55,9 @@ pub use http; pub use matrix_sdk_crypto as crypto; pub use once_cell; pub use rooms::{ - Room, RoomCreateWithCreatorEventContent, RoomDisplayName, RoomHero, RoomInfo, - RoomInfoNotableUpdate, RoomInfoNotableUpdateReasons, RoomMember, RoomMemberships, RoomState, - RoomStateFilter, + apply_redaction, Room, RoomCreateWithCreatorEventContent, RoomDisplayName, RoomHero, RoomInfo, + RoomInfoNotableUpdate, RoomInfoNotableUpdateReasons, RoomMember, RoomMembersUpdate, + RoomMemberships, RoomState, RoomStateFilter, }; pub use store::{ ComposerDraft, ComposerDraftType, QueueWedgeError, StateChanges, StateStore, StateStoreDataKey, diff --git a/crates/matrix-sdk-base/src/rooms/mod.rs b/crates/matrix-sdk-base/src/rooms/mod.rs index d738199ed95..a128a10f17e 100644 --- a/crates/matrix-sdk-base/src/rooms/mod.rs +++ b/crates/matrix-sdk-base/src/rooms/mod.rs @@ -12,7 +12,7 @@ use std::{ use bitflags::bitflags; pub use members::RoomMember; pub use normal::{ - Room, RoomHero, RoomInfo, RoomInfoNotableUpdate, RoomInfoNotableUpdateReasons, + apply_redaction, Room, RoomHero, RoomInfo, RoomInfoNotableUpdate, RoomInfoNotableUpdateReasons, RoomMembersUpdate, RoomState, RoomStateFilter, }; use regex::Regex; diff --git a/crates/matrix-sdk-base/src/rooms/normal.rs b/crates/matrix-sdk-base/src/rooms/normal.rs index 3a1b8a61d75..7ab3f5ad703 100644 --- a/crates/matrix-sdk-base/src/rooms/normal.rs +++ b/crates/matrix-sdk-base/src/rooms/normal.rs @@ -2007,7 +2007,9 @@ impl RoomInfo { } } -fn apply_redaction( +/// Apply a redaction to the given target `event`, given the raw redaction event +/// and the room version. +pub fn apply_redaction( event: &Raw, raw_redaction: &Raw, room_version: &RoomVersionId, @@ -2033,7 +2035,7 @@ fn apply_redaction( let redact_result = redact_in_place(&mut event_json, room_version, Some(redacted_because)); if let Err(e) = redact_result { - warn!("Failed to redact latest event: {e}"); + warn!("Failed to redact event: {e}"); return None; } diff --git a/crates/matrix-sdk-common/src/deserialized_responses.rs b/crates/matrix-sdk-common/src/deserialized_responses.rs index 69458887539..cf27dae0d9c 100644 --- a/crates/matrix-sdk-common/src/deserialized_responses.rs +++ b/crates/matrix-sdk-common/src/deserialized_responses.rs @@ -390,6 +390,19 @@ impl SyncTimelineEvent { self.kind.raw() } + /// Replace the raw event included in this item by another one. + pub fn replace_raw(&mut self, replacement: Raw) { + match &mut self.kind { + TimelineEventKind::Decrypted(decrypted) => decrypted.event = replacement, + TimelineEventKind::UnableToDecrypt { event, .. } + | TimelineEventKind::PlainText { event } => { + // It's safe to cast `AnyMessageLikeEvent` into `AnySyncMessageLikeEvent`, + // because the former contains a superset of the fields included in the latter. + *event = replacement.cast(); + } + } + } + /// If the event was a decrypted event that was successfully decrypted, get /// its encryption info. Otherwise, `None`. pub fn encryption_info(&self) -> Option<&EncryptionInfo> { diff --git a/crates/matrix-sdk-common/src/linked_chunk/as_vector.rs b/crates/matrix-sdk-common/src/linked_chunk/as_vector.rs index c0d65af1c15..ec23243084f 100644 --- a/crates/matrix-sdk-common/src/linked_chunk/as_vector.rs +++ b/crates/matrix-sdk-common/src/linked_chunk/as_vector.rs @@ -364,6 +364,14 @@ impl UpdateToVectorDiff { } } + Update::ReplaceItem { at: position, item } => { + let (offset, (_chunk_index, _chunk_length)) = self.map_to_offset(position); + + // The chunk length doesn't change. + + diffs.push(VectorDiff::Set { index: offset, value: item.clone() }); + } + Update::RemoveItem { at: position } => { let (offset, (_chunk_index, chunk_length)) = self.map_to_offset(position); @@ -484,15 +492,7 @@ mod tests { assert_eq!(diffs, expected_diffs); for diff in diffs { - match diff { - VectorDiff::Insert { index, value } => accumulator.insert(index, value), - VectorDiff::Append { values } => accumulator.append(values), - VectorDiff::Remove { index } => { - accumulator.remove(index); - } - VectorDiff::Clear => accumulator.clear(), - diff => unimplemented!("{diff:?}"), - } + diff.apply(accumulator); } } @@ -710,6 +710,31 @@ mod tests { vector!['m', 'a', 'w', 'x', 'y', 'b', 'd', 'i', 'j', 'k', 'l', 'e', 'f', 'g', 'z', 'h'] ); + // Replace element 8 by an uppercase J. + linked_chunk + .replace_item_at(linked_chunk.item_position(|item| *item == 'j').unwrap(), 'J') + .unwrap(); + + assert_items_eq!( + linked_chunk, + ['m', 'a', 'w'] ['x'] ['y', 'b'] ['d'] ['i', 'J', 'k'] ['l'] ['e', 'f', 'g'] ['z', 'h'] + ); + + // From an `ObservableVector` point of view, it would look like: + // + // 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 + // +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ + // | m | a | w | x | y | b | d | i | J | k | l | e | f | g | z | h | + // +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ + // ^^^^ + // | + // new! + apply_and_assert_eq( + &mut accumulator, + as_vector.take(), + &[VectorDiff::Set { index: 8, value: 'J' }], + ); + // Let's try to clear the linked chunk now. linked_chunk.clear(); diff --git a/crates/matrix-sdk-common/src/linked_chunk/mod.rs b/crates/matrix-sdk-common/src/linked_chunk/mod.rs index 113edbdc8c4..06bb4c63cee 100644 --- a/crates/matrix-sdk-common/src/linked_chunk/mod.rs +++ b/crates/matrix-sdk-common/src/linked_chunk/mod.rs @@ -528,6 +528,47 @@ impl LinkedChunk { Ok(removed_item) } + /// Replace item at a specified position in the [`LinkedChunk`]. + /// + /// `position` must point to a valid item, otherwise the method returns + /// `Err`. + pub fn replace_item_at(&mut self, position: Position, item: Item) -> Result<(), Error> + where + Item: Clone, + { + let chunk_identifier = position.chunk_identifier(); + let item_index = position.index(); + + let chunk = self + .links + .chunk_mut(chunk_identifier) + .ok_or(Error::InvalidChunkIdentifier { identifier: chunk_identifier })?; + + match &mut chunk.content { + ChunkContent::Gap(..) => { + return Err(Error::ChunkIsAGap { identifier: chunk_identifier }) + } + + ChunkContent::Items(current_items) => { + if item_index >= current_items.len() { + return Err(Error::InvalidItemIndex { index: item_index }); + } + + // Avoid one spurious clone by notifying about the update *before* applying it. + if let Some(updates) = self.updates.as_mut() { + updates.push(Update::ReplaceItem { + at: Position(chunk_identifier, item_index), + item: item.clone(), + }); + } + + current_items[item_index] = item; + } + } + + Ok(()) + } + /// Insert a gap at a specified position in the [`LinkedChunk`]. /// /// Because the `position` can be invalid, this method returns a @@ -2919,4 +2960,30 @@ mod tests { ] ); } + + #[test] + fn test_replace_item() { + let mut linked_chunk = LinkedChunk::<3, char, ()>::new(); + + linked_chunk.push_items_back(['a', 'b', 'c']); + linked_chunk.push_gap_back(()); + // Sanity check. + assert_items_eq!(linked_chunk, ['a', 'b', 'c'] [-]); + + // Replace item in bounds. + linked_chunk.replace_item_at(Position(ChunkIdentifier::new(0), 1), 'B').unwrap(); + assert_items_eq!(linked_chunk, ['a', 'B', 'c'] [-]); + + // Attempt to replace out-of-bounds. + assert_matches!( + linked_chunk.replace_item_at(Position(ChunkIdentifier::new(0), 3), 'Z'), + Err(Error::InvalidItemIndex { index: 3 }) + ); + + // Attempt to replace gap. + assert_matches!( + linked_chunk.replace_item_at(Position(ChunkIdentifier::new(1), 0), 'Z'), + Err(Error::ChunkIsAGap { .. }) + ); + } } diff --git a/crates/matrix-sdk-common/src/linked_chunk/relational.rs b/crates/matrix-sdk-common/src/linked_chunk/relational.rs index fabc55a2e14..833c0f72415 100644 --- a/crates/matrix-sdk-common/src/linked_chunk/relational.rs +++ b/crates/matrix-sdk-common/src/linked_chunk/relational.rs @@ -136,6 +136,19 @@ impl RelationalLinkedChunk { } } + Update::ReplaceItem { at, item } => { + let existing = self + .items + .iter_mut() + .find(|item| item.position == at) + .expect("trying to replace at an unknown position"); + assert!( + matches!(existing.item, Either::Item(..)), + "trying to replace a gap with an item" + ); + existing.item = Either::Item(item); + } + Update::RemoveItem { at } => { let mut entry_to_remove = None; @@ -938,4 +951,55 @@ mod tests { // The linked chunk is correctly reloaded. assert_items_eq!(lc, ['a', 'b', 'c'] [-] ['d', 'e', 'f']); } + + #[test] + fn test_replace_item() { + let room_id = room_id!("!r0:matrix.org"); + let mut relational_linked_chunk = RelationalLinkedChunk::::new(); + + 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 + Update::PushItems { at: Position::new(CId::new(0), 0), items: vec!['a', 'b', 'c'] }, + // update item at (0; 1). + Update::ReplaceItem { at: Position::new(CId::new(0), 1), item: 'B' }, + ], + ); + + // Chunks are correctly linked. + assert_eq!( + relational_linked_chunk.chunks, + &[ChunkRow { + room_id: room_id.to_owned(), + previous_chunk: None, + chunk: CId::new(0), + next_chunk: None, + },], + ); + + // Items contains the pushed *and* replaced items. + assert_eq!( + relational_linked_chunk.items, + &[ + ItemRow { + room_id: room_id.to_owned(), + position: Position::new(CId::new(0), 0), + item: Either::Item('a') + }, + ItemRow { + room_id: room_id.to_owned(), + position: Position::new(CId::new(0), 1), + item: Either::Item('B') + }, + ItemRow { + room_id: room_id.to_owned(), + position: Position::new(CId::new(0), 2), + item: Either::Item('c') + }, + ], + ); + } } diff --git a/crates/matrix-sdk-common/src/linked_chunk/updates.rs b/crates/matrix-sdk-common/src/linked_chunk/updates.rs index c14ee4ab09e..ac91749ca4f 100644 --- a/crates/matrix-sdk-common/src/linked_chunk/updates.rs +++ b/crates/matrix-sdk-common/src/linked_chunk/updates.rs @@ -79,6 +79,18 @@ pub enum Update { items: Vec, }, + /// An item has been replaced in the linked chunk. + /// + /// The `at` position MUST resolve to the actual position an existing *item* + /// (not a gap). + ReplaceItem { + /// The position of the item that's being replaced. + at: Position, + + /// The new value for the item. + item: Item, + }, + /// An item has been removed inside a chunk of kind Items. RemoveItem { /// The [`Position`] of the item. diff --git a/crates/matrix-sdk-sqlite/src/event_cache_store.rs b/crates/matrix-sdk-sqlite/src/event_cache_store.rs index 88c9d1801bb..2afef54ef91 100644 --- a/crates/matrix-sdk-sqlite/src/event_cache_store.rs +++ b/crates/matrix-sdk-sqlite/src/event_cache_store.rs @@ -459,6 +459,28 @@ impl EventCacheStore for SqliteEventCacheStore { } } + Update::ReplaceItem { at, item: event } => { + let chunk_id = at.chunk_identifier().index(); + let index = at.index(); + + trace!(%room_id, "replacing item @ {chunk_id}:{index}"); + + let serialized = serde_json::to_vec(&event)?; + let content = this.encode_value(serialized)?; + + // The event id should be the same, but just in case it changed… + let event_id = event.event_id().map(|event_id| event_id.to_string()); + + txn.execute( + r#" + UPDATE events + SET content = ?, event_id = ? + WHERE room_id = ? AND chunk_id = ? AND position = ? + "#, + (content, event_id, &hashed_room_id, chunk_id, index,) + )?; + } + Update::RemoveItem { at } => { let chunk_id = at.chunk_identifier().index(); let index = at.index(); @@ -978,6 +1000,52 @@ mod tests { }); } + #[async_test] + async fn test_linked_chunk_replace_item() { + let store = get_event_cache_store().await.expect("creating cache store failed"); + + let room_id = &DEFAULT_TEST_ROOM_ID; + + store + .handle_linked_chunk_updates( + room_id, + vec![ + Update::NewItemsChunk { + previous: None, + new: ChunkIdentifier::new(42), + next: None, + }, + Update::PushItems { + at: Position::new(ChunkIdentifier::new(42), 0), + items: vec![ + make_test_event(room_id, "hello"), + make_test_event(room_id, "world"), + ], + }, + Update::ReplaceItem { + at: Position::new(ChunkIdentifier::new(42), 1), + item: make_test_event(room_id, "yolo"), + }, + ], + ) + .await + .unwrap(); + + let mut chunks = store.reload_linked_chunk(room_id).await.unwrap(); + + assert_eq!(chunks.len(), 1); + + let c = chunks.remove(0); + assert_eq!(c.identifier, ChunkIdentifier::new(42)); + assert_eq!(c.previous, None); + assert_eq!(c.next, None); + assert_matches!(c.content, ChunkContent::Items(events) => { + assert_eq!(events.len(), 2); + check_test_event(&events[0], "hello"); + check_test_event(&events[1], "yolo"); + }); + } + #[async_test] async fn test_linked_chunk_remove_chunk() { let store = get_event_cache_store().await.expect("creating cache store failed"); diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state.rs b/crates/matrix-sdk-ui/src/timeline/controller/state.rs index 2f91777777c..5763f990d22 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state.rs @@ -281,7 +281,7 @@ impl TimelineState { let handle_one_res = txn .handle_remote_event( event.into(), - TimelineItemPosition::UpdateDecrypted { timeline_item_index: idx }, + TimelineItemPosition::UpdateAt { timeline_item_index: idx }, room_data_provider, settings, &mut date_divider_adjuster, @@ -497,6 +497,26 @@ impl TimelineStateTransaction<'_> { .await; } + VectorDiff::Set { index: event_index, value: event } => { + if let Some(timeline_item_index) = self + .items + .all_remote_events() + .get(event_index) + .and_then(|meta| meta.timeline_item_index) + { + self.handle_remote_event( + event, + TimelineItemPosition::UpdateAt { timeline_item_index }, + room_data_provider, + settings, + &mut date_divider_adjuster, + ) + .await; + } else { + warn!(event_index, "Set update dropped because there wasn't any attached timeline item index."); + } + } + VectorDiff::Remove { index: event_index } => { self.remove_timeline_item(event_index, &mut date_divider_adjuster); } @@ -572,7 +592,7 @@ impl TimelineStateTransaction<'_> { | TimelineItemPosition::Start { origin } | TimelineItemPosition::At { origin, .. } => origin, - TimelineItemPosition::UpdateDecrypted { timeline_item_index: idx } => self + TimelineItemPosition::UpdateAt { timeline_item_index: idx } => self .items .get(idx) .and_then(|item| item.as_event()) @@ -856,7 +876,7 @@ impl TimelineStateTransaction<'_> { self.items.insert_remote_event(event_index, event_meta.base_meta()); } - TimelineItemPosition::UpdateDecrypted { .. } => { + TimelineItemPosition::UpdateAt { .. } => { if let Some(event) = self.items.get_remote_event_by_event_id_mut(event_meta.event_id) { diff --git a/crates/matrix-sdk-ui/src/timeline/event_handler.rs b/crates/matrix-sdk-ui/src/timeline/event_handler.rs index b3288ca299f..f9d286b45dd 100644 --- a/crates/matrix-sdk-ui/src/timeline/event_handler.rs +++ b/crates/matrix-sdk-ui/src/timeline/event_handler.rs @@ -300,11 +300,11 @@ pub(super) enum TimelineItemPosition { origin: RemoteEventOrigin, }, - /// A single item is updated, after it's been successfully decrypted. + /// A single item is updated. /// - /// This happens when an item that was a UTD must be replaced with the - /// decrypted event. - UpdateDecrypted { + /// This can happen for instance after a UTD has been successfully + /// decrypted, or when it's been redacted at the source. + UpdateAt { /// The index of the **timeline item**. timeline_item_index: usize, }, @@ -511,7 +511,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { trace!("No new item added"); if let Flow::Remote { - position: TimelineItemPosition::UpdateDecrypted { timeline_item_index }, + position: TimelineItemPosition::UpdateAt { timeline_item_index }, .. } = self.ctx.flow { @@ -609,7 +609,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { match position { TimelineItemPosition::Start { .. } | TimelineItemPosition::At { .. } - | TimelineItemPosition::UpdateDecrypted { .. } => { + | TimelineItemPosition::UpdateAt { .. } => { // Only insert the edit if there wasn't any other edit // before. // @@ -1057,8 +1057,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { | TimelineItemPosition::At { origin, .. } => origin, // For updates, reuse the origin of the encrypted event. - TimelineItemPosition::UpdateDecrypted { timeline_item_index: idx } => self - .items[idx] + TimelineItemPosition::UpdateAt { timeline_item_index: idx } => self.items[idx] .as_event() .and_then(|ev| Some(ev.as_remote()?.origin)) .unwrap_or_else(|| { @@ -1241,7 +1240,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { Flow::Remote { event_id: decrypted_event_id, - position: TimelineItemPosition::UpdateDecrypted { timeline_item_index: idx }, + position: TimelineItemPosition::UpdateAt { timeline_item_index: idx }, .. } => { trace!("Updating timeline item at position {idx}"); diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/mod.rs b/crates/matrix-sdk-ui/tests/integration/timeline/mod.rs index 570323f4e8c..303a12b276c 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/mod.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/mod.rs @@ -115,15 +115,15 @@ async fn test_reaction() { server.reset().await; // The new message starts with their author's read receipt. - assert_let!(Some(VectorDiff::PushBack { value: message }) = timeline_stream.next().await); + assert_let_timeout!(Some(VectorDiff::PushBack { value: message }) = timeline_stream.next()); let event_item = message.as_event().unwrap(); assert_matches!(event_item.content(), TimelineItemContent::Message(_)); assert_eq!(event_item.read_receipts().len(), 1); // The new message is getting the reaction, which implies an implicit read // receipt that's obtained first. - assert_let!( - Some(VectorDiff::Set { index: 0, value: updated_message }) = timeline_stream.next().await + assert_let_timeout!( + Some(VectorDiff::Set { index: 0, value: updated_message }) = timeline_stream.next() ); let event_item = updated_message.as_event().unwrap(); assert_let!(TimelineItemContent::Message(msg) = event_item.content()); @@ -132,8 +132,8 @@ async fn test_reaction() { assert_eq!(event_item.reactions().len(), 0); // Then the reaction is taken into account. - assert_let!( - Some(VectorDiff::Set { index: 0, value: updated_message }) = timeline_stream.next().await + assert_let_timeout!( + Some(VectorDiff::Set { index: 0, value: updated_message }) = timeline_stream.next() ); let event_item = updated_message.as_event().unwrap(); assert_let!(TimelineItemContent::Message(msg) = event_item.content()); @@ -146,7 +146,9 @@ async fn test_reaction() { assert_eq!(senders.as_slice(), [user_id!("@bob:example.org")]); // The date divider. - assert_let!(Some(VectorDiff::PushFront { value: date_divider }) = timeline_stream.next().await); + assert_let_timeout!( + Some(VectorDiff::PushFront { value: date_divider }) = timeline_stream.next() + ); assert!(date_divider.is_date_divider()); sync_builder.add_joined_room(JoinedRoomBuilder::new(room_id).add_timeline_event( @@ -164,8 +166,8 @@ async fn test_reaction() { let _response = client.sync_once(sync_settings.clone()).await.unwrap(); server.reset().await; - assert_let!( - Some(VectorDiff::Set { index: 1, value: updated_message }) = timeline_stream.next().await + assert_let_timeout!( + Some(VectorDiff::Set { index: 1, value: updated_message }) = timeline_stream.next() ); let event_item = updated_message.as_event().unwrap(); assert_let!(TimelineItemContent::Message(msg) = event_item.content()); diff --git a/crates/matrix-sdk/src/event_cache/mod.rs b/crates/matrix-sdk/src/event_cache/mod.rs index d41c54d88bd..9df66ddf4f6 100644 --- a/crates/matrix-sdk/src/event_cache/mod.rs +++ b/crates/matrix-sdk/src/event_cache/mod.rs @@ -52,7 +52,7 @@ use ruma::{ AnySyncTimelineEvent, }, serde::Raw, - EventId, OwnedEventId, OwnedRoomId, RoomId, + EventId, OwnedEventId, OwnedRoomId, RoomId, RoomVersionId, }; use tokio::sync::{ broadcast::{error::RecvError, Receiver}, @@ -612,10 +612,21 @@ impl EventCacheInner { let room_state = RoomEventCacheState::new(room_id.to_owned(), self.store.clone()).await?; + let room_version = self + .client + .get() + .and_then(|client| client.get_room(room_id)) + .map(|room| room.clone_info().room_version_or_default()) + .unwrap_or_else(|| { + warn!("unknown room version for {room_id}, using default V1"); + RoomVersionId::V1 + }); + let room_event_cache = RoomEventCache::new( self.client.clone(), room_state, room_id.to_owned(), + room_version, self.all_events.clone(), ); diff --git a/crates/matrix-sdk/src/event_cache/pagination.rs b/crates/matrix-sdk/src/event_cache/pagination.rs index 4d135666649..31cdf963301 100644 --- a/crates/matrix-sdk/src/event_cache/pagination.rs +++ b/crates/matrix-sdk/src/event_cache/pagination.rs @@ -181,7 +181,8 @@ impl RoomPagination { // (backward). The `RoomEvents` API expects the first event to be the oldest. .rev() .cloned() - .map(SyncTimelineEvent::from); + .map(SyncTimelineEvent::from) + .collect::>(); let first_event_pos = room_events.events().next().map(|(item_pos, _)| item_pos); @@ -190,20 +191,20 @@ impl RoomPagination { // There is a prior gap, let's replace it by new events! trace!("replaced gap with new events from backpagination"); room_events - .replace_gap_at(sync_events, gap_id) + .replace_gap_at(sync_events.clone(), gap_id) .expect("gap_identifier is a valid chunk id we read previously") } else if let Some(pos) = first_event_pos { // No prior gap, but we had some events: assume we need to prepend events // before those. trace!("inserted events before the first known event"); let report = room_events - .insert_events_at(sync_events, pos) + .insert_events_at(sync_events.clone(), pos) .expect("pos is a valid position we just read above"); (report, Some(pos)) } else { // No prior gap, and no prior events: push the events. trace!("pushing events received from back-pagination"); - let report = room_events.push_events(sync_events); + let report = room_events.push_events(sync_events.clone()); // A new gap may be inserted before the new events, if there are any. let next_pos = room_events.events().next().map(|(item_pos, _)| item_pos); (report, next_pos) @@ -228,6 +229,8 @@ impl RoomPagination { debug!("not storing previous batch token, because we deduplicated all new back-paginated events"); } + room_events.on_new_events(&self.inner.room_version, sync_events.iter()); + BackPaginationOutcome { events, reached_start } }) .await?; diff --git a/crates/matrix-sdk/src/event_cache/room/events.rs b/crates/matrix-sdk/src/event_cache/room/events.rs index 516432304a6..142594ecb7a 100644 --- a/crates/matrix-sdk/src/event_cache/room/events.rs +++ b/crates/matrix-sdk/src/event_cache/room/events.rs @@ -15,14 +15,17 @@ use std::cmp::Ordering; use eyeball_im::VectorDiff; -use matrix_sdk_base::event_cache::store::DEFAULT_CHUNK_CAPACITY; pub use matrix_sdk_base::event_cache::{Event, Gap}; +use matrix_sdk_base::{apply_redaction, event_cache::store::DEFAULT_CHUNK_CAPACITY}; use matrix_sdk_common::linked_chunk::{ AsVector, Chunk, ChunkIdentifier, EmptyChunk, Error, Iter, IterBackward, LinkedChunk, ObservableUpdates, Position, }; -use ruma::OwnedEventId; -use tracing::{debug, error, warn}; +use ruma::{ + events::{room::redaction::SyncRoomRedactionEvent, AnySyncTimelineEvent}, + OwnedEventId, RoomVersionId, +}; +use tracing::{debug, error, instrument, trace, warn}; use super::{ super::deduplicator::{Decoration, Deduplicator}, @@ -90,6 +93,84 @@ impl RoomEvents { self.chunks.clear(); } + /// If the given event is a redaction, try to retrieve the to-be-redacted + /// event in the chunk, and replace it by the redacted form. + #[instrument(skip_all)] + fn maybe_apply_new_redaction(&mut self, room_version: &RoomVersionId, event: &Event) { + let Ok(AnySyncTimelineEvent::MessageLike( + ruma::events::AnySyncMessageLikeEvent::RoomRedaction(redaction), + )) = event.raw().deserialize() + else { + return; + }; + + let Some(event_id) = redaction.redacts(room_version) else { + warn!("missing target event id from the redaction event"); + return; + }; + + // Replace the redacted event by a redacted form, if we knew about it. + let mut items = self.chunks.items(); + + if let Some((pos, target_event)) = + items.find(|(_, item)| item.event_id().as_deref() == Some(event_id)) + { + // Don't redact already redacted events. + if let Ok(deserialized) = target_event.raw().deserialize() { + match deserialized { + AnySyncTimelineEvent::MessageLike(ev) => { + if ev.original_content().is_none() { + // Already redacted. + return; + } + } + AnySyncTimelineEvent::State(ev) => { + if ev.original_content().is_none() { + // Already redacted. + return; + } + } + } + } + + if let Some(redacted_event) = apply_redaction( + target_event.raw(), + event.raw().cast_ref::(), + room_version, + ) { + let mut copy = target_event.clone(); + + // It's safe to cast `redacted_event` here: + // - either the event was an `AnyTimelineEvent` cast to `AnySyncTimelineEvent` + // when calling .raw(), so it's still one under the hood. + // - or it wasn't, and it's a plain `AnySyncTimelineEvent` in this case. + copy.replace_raw(redacted_event.cast()); + + // Get rid of the immutable borrow on self.chunks. + drop(items); + + self.chunks + .replace_item_at(pos, copy) + .expect("should have been a valid position of an item"); + } + } else { + trace!("redacted event is missing from the linked chunk"); + } + + // TODO: remove all related events too! + } + + /// Callback to call whenever we touch events in the database. + pub fn on_new_events<'a>( + &mut self, + room_version: &RoomVersionId, + events: impl Iterator, + ) { + for ev in events { + self.maybe_apply_new_redaction(room_version, ev); + } + } + /// Push events after all events or gaps. /// /// The last event in `events` is the most recent one. diff --git a/crates/matrix-sdk/src/event_cache/room/mod.rs b/crates/matrix-sdk/src/event_cache/room/mod.rs index 08b18b69aa4..5d011d4a906 100644 --- a/crates/matrix-sdk/src/event_cache/room/mod.rs +++ b/crates/matrix-sdk/src/event_cache/room/mod.rs @@ -25,7 +25,7 @@ use matrix_sdk_base::{ use ruma::{ events::{relation::RelationType, AnyRoomAccountDataEvent, AnySyncEphemeralRoomEvent}, serde::Raw, - EventId, OwnedEventId, OwnedRoomId, + EventId, OwnedEventId, OwnedRoomId, RoomVersionId, }; use tokio::sync::{ broadcast::{Receiver, Sender}, @@ -61,9 +61,18 @@ impl RoomEventCache { client: WeakClient, state: RoomEventCacheState, room_id: OwnedRoomId, + room_version: RoomVersionId, all_events_cache: Arc>, ) -> Self { - Self { inner: Arc::new(RoomEventCacheInner::new(client, state, room_id, all_events_cache)) } + Self { + inner: Arc::new(RoomEventCacheInner::new( + client, + state, + room_id, + room_version, + all_events_cache, + )), + } } /// Subscribe to room updates for this room, after getting the initial list @@ -189,6 +198,9 @@ pub(super) struct RoomEventCacheInner { /// The room id for this room. room_id: OwnedRoomId, + /// The room version for this room. + pub(crate) room_version: RoomVersionId, + /// Sender part for subscribers to this room. pub sender: Sender, @@ -222,12 +234,14 @@ impl RoomEventCacheInner { client: WeakClient, state: RoomEventCacheState, room_id: OwnedRoomId, + room_version: RoomVersionId, all_events_cache: Arc>, ) -> Self { let sender = Sender::new(32); let weak_room = WeakRoom::new(client, room_id); Self { room_id: weak_room.room_id().to_owned(), + room_version, state: RwLock::new(state), all_events: all_events_cache, sender, @@ -444,6 +458,8 @@ impl RoomEventCacheInner { .expect("we obtained the valid position beforehand"); } } + + room_events.on_new_events(&self.room_version, sync_timeline_events.iter()); }) .await?; @@ -636,24 +652,28 @@ mod private { let _ = closure(); } + fn strip_relations_from_event(ev: &mut SyncTimelineEvent) { + match &mut ev.kind { + TimelineEventKind::Decrypted(decrypted) => { + // Remove all information about encryption info for + // the bundled events. + decrypted.unsigned_encryption_info = None; + + // Remove the `unsigned`/`m.relations` field, if needs be. + Self::strip_relations_if_present(&mut decrypted.event); + } + + TimelineEventKind::UnableToDecrypt { event, .. } + | TimelineEventKind::PlainText { event } => { + Self::strip_relations_if_present(event); + } + } + } + /// Strips the bundled relations from a collection of events. fn strip_relations_from_events(items: &mut [SyncTimelineEvent]) { for ev in items.iter_mut() { - match &mut ev.kind { - TimelineEventKind::Decrypted(decrypted) => { - // Remove all information about encryption info for - // the bundled events. - decrypted.unsigned_encryption_info = None; - - // Remove the `unsigned`/`m.relations` field, if needs be. - Self::strip_relations_if_present(&mut decrypted.event); - } - - TimelineEventKind::UnableToDecrypt { event, .. } - | TimelineEventKind::PlainText { event } => { - Self::strip_relations_if_present(event); - } - } + Self::strip_relations_from_event(ev); } } @@ -672,10 +692,11 @@ mod private { trace!("propagating {} updates", updates.len()); - // Strip relations from the `PushItems` updates. + // Strip relations from updates which insert or replace items. for up in updates.iter_mut() { match up { Update::PushItems { items, .. } => Self::strip_relations_from_events(items), + Update::ReplaceItem { item, .. } => Self::strip_relations_from_event(item), // Other update kinds don't involve adding new events. Update::NewItemsChunk { .. } | Update::NewGapChunk { .. } diff --git a/crates/matrix-sdk/tests/integration/event_cache.rs b/crates/matrix-sdk/tests/integration/event_cache.rs index e43fcbb1f2a..a2e2aaf7d44 100644 --- a/crates/matrix-sdk/tests/integration/event_cache.rs +++ b/crates/matrix-sdk/tests/integration/event_cache.rs @@ -22,7 +22,11 @@ use matrix_sdk::{ use matrix_sdk_test::{ async_test, event_factory::EventFactory, GlobalAccountDataTestEvent, JoinedRoomBuilder, }; -use ruma::{event_id, room_id, user_id}; +use ruma::{ + event_id, + events::{AnySyncMessageLikeEvent, AnySyncTimelineEvent}, + room_id, user_id, RoomVersionId, +}; use serde_json::json; use tokio::{spawn, sync::broadcast, time::sleep}; @@ -1453,3 +1457,148 @@ async fn test_dont_delete_gap_that_wasnt_inserted() { // This doesn't cause an update, because nothing changed. assert!(stream.is_empty()); } + +#[async_test] +async fn test_apply_redaction_when_redaction_comes_later() { + let server = MatrixMockServer::new().await; + let client = server.client_builder().build().await; + + let event_cache = client.event_cache(); + + // Immediately subscribe the event cache to sync updates. + event_cache.subscribe().unwrap(); + event_cache.enable_storage().unwrap(); + + let room_id = room_id!("!omelette:fromage.fr"); + + let f = EventFactory::new().room(room_id).sender(user_id!("@a:b.c")); + + // Start with a room with two events. + let room = server + .sync_room( + &client, + JoinedRoomBuilder::new(room_id).add_timeline_event( + f.text_msg("inapprops").event_id(event_id!("$1")).into_raw_sync(), + ), + ) + .await; + + let (room_event_cache, _drop_handles) = room.event_cache().await.unwrap(); + + // Wait for the first event. + let (events, mut subscriber) = room_event_cache.subscribe().await.unwrap(); + if events.is_empty() { + assert_let_timeout!( + Ok(RoomEventCacheUpdate::UpdateTimelineEvents { .. }) = subscriber.recv() + ); + } + + // Sync a redaction for the event $1. + server + .sync_room( + &client, + JoinedRoomBuilder::new(room_id) + .add_timeline_event(f.redaction(event_id!("$1")).into_raw_sync()), + ) + .await; + + assert_let_timeout!( + Ok(RoomEventCacheUpdate::UpdateTimelineEvents { diffs, .. }) = subscriber.recv() + ); + + assert_eq!(diffs.len(), 2); + + // First, the redaction event itself. + { + assert_let!(VectorDiff::Append { values: new_events } = &diffs[0]); + assert_eq!(new_events.len(), 1); + let ev = new_events[0].raw().deserialize().unwrap(); + assert_let!( + AnySyncTimelineEvent::MessageLike(AnySyncMessageLikeEvent::RoomRedaction(ev)) = ev + ); + assert_eq!(ev.redacts(&RoomVersionId::V1).unwrap(), event_id!("$1")); + } + + // Then, we have an update for the redacted event. + { + assert_let!(VectorDiff::Set { index: 0, value: redacted_event } = &diffs[1]); + let ev = redacted_event.raw().deserialize().unwrap(); + assert_let!( + AnySyncTimelineEvent::MessageLike(AnySyncMessageLikeEvent::RoomMessage(ev)) = ev + ); + // The event has been redacted! + assert_matches!(ev.as_original(), None); + } + + // And done for now. + assert!(subscriber.is_empty()); +} + +#[async_test] +async fn test_apply_redaction_when_redacted_and_redaction_are_in_same_sync() { + let server = MatrixMockServer::new().await; + let client = server.client_builder().build().await; + + let event_cache = client.event_cache(); + + // Immediately subscribe the event cache to sync updates. + event_cache.subscribe().unwrap(); + event_cache.enable_storage().unwrap(); + + let room_id = room_id!("!omelette:fromage.fr"); + let room = server.sync_joined_room(&client, room_id).await; + let (room_event_cache, _drop_handles) = room.event_cache().await.unwrap(); + let (_events, mut subscriber) = room_event_cache.subscribe().await.unwrap(); + + let f = EventFactory::new().room(room_id).sender(user_id!("@a:b.c")); + + // Now include a sync with both the original event *and* the redacted one. + server + .sync_room( + &client, + JoinedRoomBuilder::new(room_id) + .add_timeline_event(f.text_msg("bleh").event_id(event_id!("$2")).into_raw_sync()) + .add_timeline_event(f.redaction(event_id!("$2")).into_raw_sync()), + ) + .await; + + assert_let_timeout!( + Ok(RoomEventCacheUpdate::UpdateTimelineEvents { diffs, .. }) = subscriber.recv() + ); + + assert_eq!(diffs.len(), 2); + + // First, we get an update with all the new events. + { + assert_let!(VectorDiff::Append { values: new_events } = &diffs[0]); + assert_eq!(new_events.len(), 2); + + // The original event. + let ev = new_events[0].raw().deserialize().unwrap(); + assert_let!( + AnySyncTimelineEvent::MessageLike(AnySyncMessageLikeEvent::RoomMessage(ev)) = ev + ); + assert_eq!(ev.as_original().unwrap().content.body(), "bleh"); + + // The redaction. + let ev = new_events[1].raw().deserialize().unwrap(); + assert_let!( + AnySyncTimelineEvent::MessageLike(AnySyncMessageLikeEvent::RoomRedaction(ev)) = ev + ); + assert_eq!(ev.redacts(&RoomVersionId::V1).unwrap(), event_id!("$2")); + } + + // Then the redaction of the event happens separately. + { + assert_let!(VectorDiff::Set { index: 0, value: redacted_event } = &diffs[1]); + let ev = redacted_event.raw().deserialize().unwrap(); + assert_let!( + AnySyncTimelineEvent::MessageLike(AnySyncMessageLikeEvent::RoomMessage(ev)) = ev + ); + // The event has been redacted! + assert_matches!(ev.as_original(), None); + } + + // That's all, folks! + assert!(subscriber.is_empty()); +}