From 05cbb9e290e4e0b219324aec31fbe6b91c6fb2d5 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Wed, 2 Oct 2024 16:58:27 +0200 Subject: [PATCH] timeline(refactor): get rid of the stored event id in the pending_edits array Since it's implied from the `Replacement` data structure. Also reuse `find_and_remove_pending` in more places. --- .../src/timeline/controller/state.rs | 11 ++- .../src/timeline/event_handler.rs | 69 +++++++++---------- 2 files changed, 41 insertions(+), 39 deletions(-) diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state.rs b/crates/matrix-sdk-ui/src/timeline/controller/state.rs index 371c0b356..4cc3af19a 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state.rs @@ -816,6 +816,15 @@ pub(in crate::timeline) enum PendingEdit { Poll(Replacement), } +impl PendingEdit { + pub fn edited_event(&self) -> &EventId { + match self { + PendingEdit::RoomMessage(Replacement { event_id, .. }) + | PendingEdit::Poll(Replacement { event_id, .. }) => event_id, + } + } +} + #[cfg(not(tarpaulin_include))] impl std::fmt::Debug for PendingEdit { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { @@ -869,7 +878,7 @@ pub(in crate::timeline) struct TimelineMetadata { pub pending_poll_events: PendingPollEvents, /// Edit events received before the related event they're editing. - pub pending_edits: RingBuffer<(OwnedEventId, PendingEdit)>, + pub pending_edits: RingBuffer, /// Identifier of the fully-read event, helping knowing where to introduce /// the read marker. diff --git a/crates/matrix-sdk-ui/src/timeline/event_handler.rs b/crates/matrix-sdk-ui/src/timeline/event_handler.rs index 06a9cfc58..4cc55834d 100644 --- a/crates/matrix-sdk-ui/src/timeline/event_handler.rs +++ b/crates/matrix-sdk-ui/src/timeline/event_handler.rs @@ -18,7 +18,8 @@ use as_variant::as_variant; use eyeball_im::{ObservableVectorTransaction, ObservableVectorTransactionEntry}; use indexmap::IndexMap; use matrix_sdk::{ - crypto::types::events::UtdCause, deserialized_responses::EncryptionInfo, send_queue::SendHandle, + crypto::types::events::UtdCause, deserialized_responses::EncryptionInfo, + ring_buffer::RingBuffer, send_queue::SendHandle, }; use ruma::{ events::{ @@ -42,7 +43,8 @@ use ruma::{ MessageLikeEventType, StateEventType, SyncStateEvent, }, serde::Raw, - MilliSecondsSinceUnixEpoch, OwnedEventId, OwnedTransactionId, OwnedUserId, RoomVersionId, + EventId, MilliSecondsSinceUnixEpoch, OwnedEventId, OwnedTransactionId, OwnedUserId, + RoomVersionId, }; use tracing::{debug, error, field::debug, info, instrument, trace, warn}; @@ -329,25 +331,17 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { // there's an edit in the relations mapping, we want to prefer it over any // other pending edit, since it's more likely to be up to date, and we // don't want to apply another pending edit on top of it. - let pending_edit = if let Flow::Remote { event_id, .. } = &self.ctx.flow { - let edits = &mut self.meta.pending_edits; - edits - .iter() - .position(|(prev_event_id, _)| prev_event_id == event_id) - .into_iter() - .filter_map(|pos| { - Some( - as_variant!( - edits.remove(pos).unwrap().1, - PendingEdit::RoomMessage - )? - .new_content, + let pending_edit = + as_variant!(&self.ctx.flow, Flow::Remote { event_id, .. } => event_id) + .and_then(|event_id| { + Self::find_and_remove_pending( + &mut self.meta.pending_edits, + event_id, ) }) - .next() - } else { - None - }; + .and_then(|edit| { + Some(as_variant!(edit, PendingEdit::RoomMessage)?.new_content) + }); let edit = extract_edit_content(relations).or(pending_edit); @@ -517,9 +511,9 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { .meta .pending_edits .iter() - .any(|(event_id, _)| *event_id == replaced_event_id) + .any(|edit| edit.edited_event() == replaced_event_id) { - self.meta.pending_edits.push((replaced_event_id, replacement)); + self.meta.pending_edits.push(replacement); debug!("Timeline item not found, stashing edit"); } else { debug!("Timeline item not found, but there was a previous edit for the event: discarding"); @@ -531,41 +525,40 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { // forward-pagination: it's fine to overwrite the previous one, if // available. let edits = &mut self.meta.pending_edits; - if let Some(pos) = - edits.iter().position(|(event_id, _)| *event_id == replaced_event_id) - { - edits.remove(pos); - } - edits.push((replaced_event_id, replacement)); + let _ = Self::find_and_remove_pending(edits, &replaced_event_id); + edits.push(replacement); debug!("Timeline item not found, stashing edit"); } } } + /// TODO rename to maybe_unstash_pending_edit? + fn find_and_remove_pending( + edits: &mut RingBuffer, + event_id: &EventId, + ) -> Option { + let pos = edits.iter().position(|edit| edit.edited_event() == event_id)?; + Some(edits.remove(pos).unwrap()) + } + /// If there's a pending edit for an item, apply it immediately, returning /// an updated [`EventTimelineItem`]. Otherwise, return `None`. fn maybe_unstash_pending_edit( &mut self, item: &EventTimelineItem, ) -> Option { - let Flow::Remote { event_id, .. } = &self.ctx.flow else { - return None; - }; - - let mut find_and_remove_pending = |event_id| { - let edits = &mut self.meta.pending_edits; - let pos = edits.iter().position(|(prev_event_id, _)| prev_event_id == event_id)?; - Some(edits.remove(pos).unwrap().1) - }; + let event_id = as_variant!(&self.ctx.flow, Flow::Remote { event_id, .. } => event_id)?; match item.content() { TimelineItemContent::Message(..) => { - let pending = find_and_remove_pending(event_id)?; + let pending = + Self::find_and_remove_pending(&mut self.meta.pending_edits, event_id)?; let edit = as_variant!(pending, PendingEdit::RoomMessage)?; self.apply_msg_edit(item, edit) } TimelineItemContent::Poll(..) => { - let pending = find_and_remove_pending(event_id)?; + let pending = + Self::find_and_remove_pending(&mut self.meta.pending_edits, event_id)?; let edit = as_variant!(pending, PendingEdit::Poll)?; self.apply_poll_edit(item, edit) }