Skip to content

Commit

Permalink
refactor(ui): Create ObservableItemsTransactionEntry.
Browse files Browse the repository at this point in the history
This patch creates `ObservableItemsTransactionEntry` that mimics
`ObservableVectorTransactionEntry`. The differences are `set` is
renamed `replace`, and `remove` is unsafe (because I failed to update
`AllRemoteEvents` in this method due to the borrow checker).
  • Loading branch information
Hywan committed Dec 4, 2024
1 parent 97373c8 commit 7a6b2ce
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 25 deletions.
5 changes: 4 additions & 1 deletion crates/matrix-sdk-ui/src/timeline/controller/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ use tracing::{
#[cfg(test)]
pub(super) use self::observable_items::ObservableItems;
pub(super) use self::{
observable_items::{AllRemoteEvents, ObservableItemsEntry, ObservableItemsTransaction},
observable_items::{
AllRemoteEvents, ObservableItemsEntry, ObservableItemsTransaction,
ObservableItemsTransactionEntry,
},
state::{
FullEventMeta, PendingEdit, PendingEditKind, TimelineMetadata, TimelineNewItemPosition,
TimelineState, TimelineStateTransaction,
Expand Down
37 changes: 34 additions & 3 deletions crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,11 +279,11 @@ impl<'observable_items> ObservableItemsTransaction<'observable_items> {

/// Call the given closure for every element in this `ObservableItems`,
/// with an entry struct that allows updating that element.
pub fn for_each<F>(&mut self, f: F)
pub fn for_each<F>(&mut self, mut f: F)
where
F: FnMut(ObservableVectorTransactionEntry<'_, 'observable_items, Arc<TimelineItem>>),
F: FnMut(ObservableItemsTransactionEntry<'_, 'observable_items>),
{
self.items.for_each(f)
self.items.for_each(|entry| f(ObservableItemsTransactionEntry(entry)))
}

/// Commit this transaction, persisting the changes and notifying
Expand All @@ -302,6 +302,37 @@ impl Deref for ObservableItemsTransaction<'_> {
}
}

/// A handle to a single timeline item in an `ObservableItemsTransaction`.
pub struct ObservableItemsTransactionEntry<'a, 'observable_items>(
ObservableVectorTransactionEntry<'a, 'observable_items, Arc<TimelineItem>>,
);

impl<'a, 'o> ObservableItemsTransactionEntry<'a, 'o> {
/// Replace the timeline item by `timeline_item`.
pub fn replace(this: &mut Self, timeline_item: Arc<TimelineItem>) -> Arc<TimelineItem> {
ObservableVectorTransactionEntry::set(&mut this.0, timeline_item)
}

/// Remove this timeline item.
///
/// # Safety
///
/// This method doesn't update `AllRemoteEvents`. Be sure that the caller
/// doesn't break the mapping between remote events and timeline items. See
/// [`EventMeta::timeline_item_index`] to learn more.
pub unsafe fn remove(this: Self) {
ObservableVectorTransactionEntry::remove(this.0);
}
}

impl Deref for ObservableItemsTransactionEntry<'_, '_> {
type Target = Arc<TimelineItem>;

fn deref(&self) -> &Self::Target {
&self.0
}
}

/// A type for all remote events.
///
/// Having this type helps to know exactly which parts of the code and how they
Expand Down
13 changes: 10 additions & 3 deletions crates/matrix-sdk-ui/src/timeline/controller/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ use std::{
sync::{Arc, RwLock},
};

use eyeball_im::ObservableVectorTransactionEntry;
use itertools::Itertools as _;
use matrix_sdk::{
deserialized_responses::SyncTimelineEvent, ring_buffer::RingBuffer, send_queue::SendHandle,
Expand All @@ -45,7 +44,10 @@ use ruma::{
use tracing::{debug, instrument, trace, warn};

use super::{
observable_items::{AllRemoteEvents, ObservableItems, ObservableItemsTransaction},
observable_items::{
AllRemoteEvents, ObservableItems, ObservableItemsTransaction,
ObservableItemsTransactionEntry,
},
HandleManyEventsResult, TimelineFocusKind, TimelineSettings,
};
use crate::{
Expand Down Expand Up @@ -655,7 +657,12 @@ impl TimelineStateTransaction<'_> {
// Remove all remote events and the read marker
self.items.for_each(|entry| {
if entry.is_remote_event() || entry.is_read_marker() {
ObservableVectorTransactionEntry::remove(entry);
// SAFETY: this method removes all events except local events. Local events
// don't have a mapping from remote events to timeline items because… well… they
// are local events, not remove events.
unsafe {
ObservableItemsTransactionEntry::remove(entry);
}
}
});

Expand Down
28 changes: 10 additions & 18 deletions crates/matrix-sdk-ui/src/timeline/event_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
use std::sync::Arc;

use as_variant::as_variant;
use eyeball_im::ObservableVectorTransactionEntry;
use indexmap::IndexMap;
use matrix_sdk::{
crypto::types::events::UtdCause,
Expand Down Expand Up @@ -52,30 +51,23 @@ use tracing::{debug, error, field::debug, info, instrument, trace, warn};

use super::{
controller::{
ObservableItemsTransaction, PendingEditKind, TimelineMetadata, TimelineStateTransaction,
ObservableItemsTransaction, ObservableItemsTransactionEntry, PendingEdit, PendingEditKind,
TimelineMetadata, TimelineStateTransaction,
},
day_dividers::DayDividerAdjuster,
event_item::{
extract_bundled_edit_event_json, extract_poll_edit_content, extract_room_msg_edit_content,
AnyOtherFullStateEventContent, EventSendState, EventTimelineItemKind,
LocalEventTimelineItem, PollState, Profile, ReactionsByKeyBySender, RemoteEventOrigin,
RemoteEventTimelineItem, TimelineEventItemId,
LocalEventTimelineItem, PollState, Profile, ReactionInfo, ReactionStatus,
ReactionsByKeyBySender, RemoteEventOrigin, RemoteEventTimelineItem, TimelineEventItemId,
},
reactions::FullReactionKey,
reactions::{FullReactionKey, PendingReaction},
traits::RoomDataProvider,
util::{rfind_event_by_id, rfind_event_item},
EventTimelineItem, InReplyToDetails, OtherState, Sticker, TimelineDetails, TimelineItem,
TimelineItemContent,
};
use crate::{
events::SyncTimelineEventWithoutContent,
timeline::{
controller::PendingEdit,
event_item::{ReactionInfo, ReactionStatus},
reactions::PendingReaction,
traits::RoomDataProvider,
RepliedToEvent,
},
EventTimelineItem, InReplyToDetails, OtherState, RepliedToEvent, Sticker, TimelineDetails,
TimelineItem, TimelineItemContent,
};
use crate::events::SyncTimelineEventWithoutContent;

/// When adding an event, useful information related to the source of the event.
pub(super) enum Flow {
Expand Down Expand Up @@ -1262,7 +1254,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
TimelineItemContent::Message(message.with_in_reply_to(in_reply_to));
let new_reply_item =
entry.with_kind(event_item.with_content(new_reply_content, None));
ObservableVectorTransactionEntry::set(&mut entry, new_reply_item);
ObservableItemsTransactionEntry::replace(&mut entry, new_reply_item);
}
});
}
Expand Down

0 comments on commit 7a6b2ce

Please sign in to comment.