mirror/matrix-rust-sdk
1
0
Fork 0
mirror of https://github.com/matrix-org/matrix-rust-sdk.git synced 2026-05-13 02:25:51 -04:00
Code Issues Packages Projects Releases Wiki Activity

refactor(ui): Move AllRemoteEvents inside observable_items.

Browse Source 
This patch moves `AllRemoteEvents` inside `observable_items` so that
more methods can be made private, which reduces the risk of misuses
of this API. In particular,  the following methods are now strictly
private:

- `clear`
- `push_front`
- `push_back`
- `remove`
- `timeline_item_has_been_inserted_at`
- `timeline_item_has_been_removed_at`

In fact, now, all `&mut self` method (except `get_by_event_id_mut`) are
now strictly private!
This commit is contained in:
Ivan Enderlin
2024-12-04 16:46:14 +01:00
parent b069b20e18
commit 0647be1bc3
6 changed files with 341 additions and 265 deletions
Download Patch File Download Diff File Expand all files Collapse all files

43
crates/matrix-sdk-ui/src/timeline/controller/mod.rs
View File

@@ -54,10 +54,10 @@ use tracing::{
#[cfg(test)]
pub(super) use self::observable_items::ObservableItems;
pub(super) use self::{
observable_items::ObservableItemsTransaction,
observable_items::{AllRemoteEvents, ObservableItemsTransaction},
state::{
AllRemoteEvents, FullEventMeta, PendingEdit, PendingEditKind, TimelineMetadata,
TimelineNewItemPosition, TimelineState, TimelineStateTransaction,
FullEventMeta, PendingEdit, PendingEditKind, TimelineMetadata, TimelineNewItemPosition,
TimelineState, TimelineStateTransaction,
},
};
use super::{
@@ -1487,13 +1487,22 @@ impl TimelineController {
match receipt_type {
SendReceiptType::Read => {
if let Some((old_pub_read, _)) =
state.meta.user_receipt(own_user_id, ReceiptType::Read, room).await
if let Some((old_pub_read, _)) = state
.meta
.user_receipt(
own_user_id,
ReceiptType::Read,
room,
state.items.all_remote_events(),
)
.await
{
trace!(%old_pub_read, "found a previous public receipt");
if let Some(relative_pos) =
state.meta.compare_events_positions(&old_pub_read, event_id)
{
if let Some(relative_pos) = state.meta.compare_events_positions(
&old_pub_read,
event_id,
state.items.all_remote_events(),
) {
trace!("event referred to new receipt is {relative_pos:?} the previous receipt");
return relative_pos == RelativePosition::After;
}
@@ -1506,9 +1515,11 @@ impl TimelineController {
state.latest_user_read_receipt(own_user_id, room).await
{
trace!(%old_priv_read, "found a previous private receipt");
if let Some(relative_pos) =
state.meta.compare_events_positions(&old_priv_read, event_id)
{
if let Some(relative_pos) = state.meta.compare_events_positions(
&old_priv_read,
event_id,
state.items.all_remote_events(),
) {
trace!("event referred to new receipt is {relative_pos:?} the previous receipt");
return relative_pos == RelativePosition::After;
}
@@ -1517,9 +1528,11 @@ impl TimelineController {
SendReceiptType::FullyRead => {
if let Some(prev_event_id) = self.room_data_provider.load_fully_read_marker().await
{
if let Some(relative_pos) =
state.meta.compare_events_positions(&prev_event_id, event_id)
{
if let Some(relative_pos) = state.meta.compare_events_positions(
&prev_event_id,
event_id,
state.items.all_remote_events(),
) {
return relative_pos == RelativePosition::After;
}
}
@@ -1535,7 +1548,7 @@ impl TimelineController {
/// it's folded into another timeline item.
pub(crate) async fn latest_event_id(&self) -> Option<OwnedEventId> {
let state = self.state.read().await;
state.meta.all_remote_events.last().map(|event_meta| &event_meta.event_id).cloned()
state.items.all_remote_events().last().map(|event_meta| &event_meta.event_id).cloned()
}
}

215
crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs
View File

@@ -12,19 +12,36 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use std::{ops::Deref, sync::Arc};
use std::{
cmp::Ordering,
collections::{vec_deque::Iter, VecDeque},
ops::Deref,
sync::Arc,
};
use eyeball_im::{
ObservableVector, ObservableVectorEntries, ObservableVectorEntry, ObservableVectorTransaction,
ObservableVectorTransactionEntry, VectorSubscriber,
};
use imbl::Vector;
use ruma::EventId;
use super::TimelineItem;
use super::{state::EventMeta, TimelineItem};
#[derive(Debug)]
pub struct ObservableItems {
/// All timeline items.
///
/// Yeah, there are here!
items: ObservableVector<Arc<TimelineItem>>,
/// List of all the remote events as received in the timeline, even the ones
/// that are discarded in the timeline items.
///
/// This is useful to get this for the moment as it helps the `Timeline` to
/// compute read receipts and read markers. It also helps to map event to
/// timeline item, see [`EventMeta::timeline_item_index`] to learn more.
all_remote_events: AllRemoteEvents,
}
impl ObservableItems {
@@ -34,9 +51,14 @@ impl ObservableItems {
// sliding-sync tests with 20 events lag. This should still be
// small enough.
items: ObservableVector::with_capacity(32),
all_remote_events: AllRemoteEvents::default(),
}
}
pub fn all_remote_events(&self) -> &AllRemoteEvents {
&self.all_remote_events
}
pub fn is_empty(&self) -> bool {
self.items.is_empty()
}
@@ -50,7 +72,10 @@ impl ObservableItems {
}
pub fn transaction(&mut self) -> ObservableItemsTransaction<'_> {
ObservableItemsTransaction { items: self.items.transaction() }
ObservableItemsTransaction {
items: self.items.transaction(),
all_remote_events: &mut self.all_remote_events,
}
}
pub fn set(
@@ -85,6 +110,7 @@ impl Deref for ObservableItems {
#[derive(Debug)]
pub struct ObservableItemsTransaction<'observable_items> {
items: ObservableVectorTransaction<'observable_items, Arc<TimelineItem>>,
all_remote_events: &'observable_items mut AllRemoteEvents,
}
impl<'observable_items> ObservableItemsTransaction<'observable_items> {
@@ -92,6 +118,29 @@ impl<'observable_items> ObservableItemsTransaction<'observable_items> {
self.items.get(timeline_item_index)
}
pub fn all_remote_events(&self) -> &AllRemoteEvents {
&self.all_remote_events
}
pub fn remove_remote_event(&mut self, event_index: usize) -> Option<EventMeta> {
self.all_remote_events.remove(event_index)
}
pub fn push_front_remote_event(&mut self, event_meta: EventMeta) {
self.all_remote_events.push_front(event_meta);
}
pub fn push_back_remote_event(&mut self, event_meta: EventMeta) {
self.all_remote_events.push_back(event_meta);
}
pub fn get_remote_event_by_event_id_mut(
&mut self,
event_id: &EventId,
) -> Option<&mut EventMeta> {
self.all_remote_events.get_by_event_id_mut(event_id)
}
pub fn set(
&mut self,
timeline_item_index: usize,
@@ -101,23 +150,36 @@ impl<'observable_items> ObservableItemsTransaction<'observable_items> {
}
pub fn remove(&mut self, timeline_item_index: usize) -> Arc<TimelineItem> {
self.items.remove(timeline_item_index)
let removed_timeline_item = self.items.remove(timeline_item_index);
self.all_remote_events.timeline_item_has_been_removed_at(timeline_item_index);
removed_timeline_item
}
pub fn insert(&mut self, timeline_item_index: usize, timeline_item: Arc<TimelineItem>) {
pub fn insert(
&mut self,
timeline_item_index: usize,
timeline_item: Arc<TimelineItem>,
event_index: Option<usize>,
) {
self.items.insert(timeline_item_index, timeline_item);
self.all_remote_events.timeline_item_has_been_inserted_at(timeline_item_index, event_index);
}
pub fn push_front(&mut self, timeline_item: Arc<TimelineItem>) {
pub fn push_front(&mut self, timeline_item: Arc<TimelineItem>, event_index: Option<usize>) {
self.items.push_front(timeline_item);
self.all_remote_events.timeline_item_has_been_inserted_at(0, event_index);
}
pub fn push_back(&mut self, timeline_item: Arc<TimelineItem>) {
pub fn push_back(&mut self, timeline_item: Arc<TimelineItem>, event_index: Option<usize>) {
self.items.push_back(timeline_item);
self.all_remote_events
.timeline_item_has_been_inserted_at(self.items.len().saturating_sub(1), event_index);
}
pub fn clear(&mut self) {
self.items.clear();
self.all_remote_events.clear();
}
pub fn for_each<F>(&mut self, f: F)
@@ -140,3 +202,142 @@ impl Deref for ObservableItemsTransaction<'_> {
&self.items
}
}
/// A type for all remote events.
///
/// Having this type helps to know exactly which parts of the code and how they
/// use all remote events. It also helps to give a bit of semantics on top of
/// them.
#[derive(Clone, Debug, Default)]
pub struct AllRemoteEvents(VecDeque<EventMeta>);
impl AllRemoteEvents {
/// Return a front-to-back iterator over all remote events.
pub fn iter(&self) -> Iter<'_, EventMeta> {
self.0.iter()
}
/// Remove all remote events.
fn clear(&mut self) {
self.0.clear();
}
/// Insert a new remote event at the front of all the others.
fn push_front(&mut self, event_meta: EventMeta) {
// If there is an associated `timeline_item_index`, shift all the
// `timeline_item_index` that come after this one.
if let Some(new_timeline_item_index) = event_meta.timeline_item_index {
self.increment_all_timeline_item_index_after(new_timeline_item_index);
}
// Push the event.
self.0.push_front(event_meta)
}
/// Insert a new remote event at the back of all the others.
fn push_back(&mut self, event_meta: EventMeta) {
// If there is an associated `timeline_item_index`, shift all the
// `timeline_item_index` that come after this one.
if let Some(new_timeline_item_index) = event_meta.timeline_item_index {
self.increment_all_timeline_item_index_after(new_timeline_item_index);
}
// Push the event.
self.0.push_back(event_meta)
}
/// Remove one remote event at a specific index, and return it if it exists.
fn remove(&mut self, event_index: usize) -> Option<EventMeta> {
// Remove the event.
let event_meta = self.0.remove(event_index)?;
// If there is an associated `timeline_item_index`, shift all the
// `timeline_item_index` that come after this one.
if let Some(removed_timeline_item_index) = event_meta.timeline_item_index {
self.decrement_all_timeline_item_index_after(removed_timeline_item_index);
};
Some(event_meta)
}
/// Return a reference to the last remote event if it exists.
pub fn last(&self) -> Option<&EventMeta> {
self.0.back()
}
/// Return the index of the last remote event if it exists.
pub fn last_index(&self) -> Option<usize> {
self.0.len().checked_sub(1)
}
/// Get a mutable reference to a specific remote event by its ID.
pub fn get_by_event_id_mut(&mut self, event_id: &EventId) -> Option<&mut EventMeta> {
self.0.iter_mut().rev().find(|event_meta| event_meta.event_id == event_id)
}
/// Shift to the right all timeline item indexes that are equal to or
/// greater than `new_timeline_item_index`.
fn increment_all_timeline_item_index_after(&mut self, new_timeline_item_index: usize) {
for event_meta in self.0.iter_mut() {
if let Some(timeline_item_index) = event_meta.timeline_item_index.as_mut() {
if *timeline_item_index >= new_timeline_item_index {
*timeline_item_index += 1;
}
}
}
}
/// Shift to the left all timeline item indexes that are greater than
/// `removed_wtimeline_item_index`.
fn decrement_all_timeline_item_index_after(&mut self, removed_timeline_item_index: usize) {
for event_meta in self.0.iter_mut() {
if let Some(timeline_item_index) = event_meta.timeline_item_index.as_mut() {
if *timeline_item_index > removed_timeline_item_index {
*timeline_item_index -= 1;
}
}
}
}
fn timeline_item_has_been_inserted_at(
&mut self,
new_timeline_item_index: usize,
event_index: Option<usize>,
) {
self.increment_all_timeline_item_index_after(new_timeline_item_index);
if let Some(event_index) = event_index {
if let Some(event_meta) = self.0.get_mut(event_index) {
event_meta.timeline_item_index = Some(new_timeline_item_index);
}
}
}
fn timeline_item_has_been_removed_at(&mut self, timeline_item_index_to_remove: usize) {
for event_meta in self.0.iter_mut() {
let mut remove_timeline_item_index = false;
// A `timeline_item_index` is removed. Let's shift all indexes that come
// after the removed one.
if let Some(timeline_item_index) = event_meta.timeline_item_index.as_mut() {
match (*timeline_item_index).cmp(&timeline_item_index_to_remove) {
Ordering::Equal => {
remove_timeline_item_index = true;
}
Ordering::Greater => {
*timeline_item_index -= 1;
}
Ordering::Less => {}
}
}
// This is the `event_meta` that holds the `timeline_item_index` that is being
// removed. So let's clean it.
if remove_timeline_item_index {
event_meta.timeline_item_index = None;
}
}
}
}

198
crates/matrix-sdk-ui/src/timeline/controller/state.rs
View File

@@ -13,8 +13,7 @@
// limitations under the License.
use std::{
cmp::Ordering,
collections::{vec_deque::Iter, HashMap, VecDeque},
collections::HashMap,
future::Future,
num::NonZeroUsize,
sync::{Arc, RwLock},
@@ -46,7 +45,7 @@ use ruma::{
use tracing::{debug, instrument, trace, warn};
use super::{
observable_items::{ObservableItems, ObservableItemsTransaction},
observable_items::{AllRemoteEvents, ObservableItems, ObservableItemsTransaction},
HandleManyEventsResult, TimelineFocusKind, TimelineSettings,
};
use crate::{
@@ -546,7 +545,7 @@ impl TimelineStateTransaction<'_> {
};
// Remember the event before returning prematurely.
// See [`TimelineMetadata::all_remote_events`].
// See [`ObservableItems::all_remote_events`].
self.add_or_update_remote_event(
event_meta,
position,
@@ -585,7 +584,7 @@ impl TimelineStateTransaction<'_> {
};
// Remember the event before returning prematurely.
// See [`TimelineMetadata::all_remote_events`].
// See [`ObservableItems::all_remote_events`].
self.add_or_update_remote_event(
event_meta,
position,
@@ -611,7 +610,7 @@ impl TimelineStateTransaction<'_> {
};
// Remember the event.
// See [`TimelineMetadata::all_remote_events`].
// See [`ObservableItems::all_remote_events`].
self.add_or_update_remote_event(event_meta, position, room_data_provider, settings).await;
let sender_profile = room_data_provider.profile_from_user_id(&sender).await;
@@ -623,7 +622,7 @@ impl TimelineStateTransaction<'_> {
read_receipts: if settings.track_read_receipts && should_add {
self.meta.read_receipts.compute_event_receipts(
&event_id,
&self.meta.all_remote_events,
self.items.all_remote_events(),
matches!(position, TimelineItemPosition::End { .. }),
)
} else {
@@ -702,7 +701,7 @@ impl TimelineStateTransaction<'_> {
}
/// Add or update a remote event in the
/// [`TimelineMetadata::all_remote_events`] collection.
/// [`ObservableItems::all_remote_events`] collection.
///
/// This method also adjusts read receipt if needed.
async fn add_or_update_remote_event<P: RoomDataProvider>(
@@ -712,7 +711,7 @@ impl TimelineStateTransaction<'_> {
room_data_provider: &P,
settings: &TimelineSettings,
) {
// Detect if an event already exists in [`TimelineMetadata::all_remote_events`].
// Detect if an event already exists in [`ObservableItems::all_remote_events`].
//
// Returns its position, in this case.
fn event_already_exists(
@@ -725,27 +724,27 @@ impl TimelineStateTransaction<'_> {
match position {
TimelineItemPosition::Start { .. } => {
if let Some(pos) =
event_already_exists(event_meta.event_id, &self.meta.all_remote_events)
event_already_exists(event_meta.event_id, &self.items.all_remote_events())
{
self.meta.all_remote_events.remove(pos);
self.items.remove_remote_event(pos);
}
self.meta.all_remote_events.push_front(event_meta.base_meta())
self.items.push_front_remote_event(event_meta.base_meta())
}
TimelineItemPosition::End { .. } => {
if let Some(pos) =
event_already_exists(event_meta.event_id, &self.meta.all_remote_events)
event_already_exists(event_meta.event_id, &self.items.all_remote_events())
{
self.meta.all_remote_events.remove(pos);
self.items.remove_remote_event(pos);
}
self.meta.all_remote_events.push_back(event_meta.base_meta());
self.items.push_back_remote_event(event_meta.base_meta());
}
TimelineItemPosition::UpdateDecrypted { .. } => {
if let Some(event) =
self.meta.all_remote_events.get_by_event_id_mut(event_meta.event_id)
self.items.get_remote_event_by_event_id_mut(event_meta.event_id)
{
if event.visible != event_meta.visible {
event.visible = event_meta.visible;
@@ -918,13 +917,6 @@ pub(in crate::timeline) struct TimelineMetadata {
/// the device has terabytes of RAM.
next_internal_id: u64,
/// List of all the remote events as received in the timeline, even the ones
/// that are discarded in the timeline items.
///
/// This is useful to get this for the moment as it helps the `Timeline` to
/// compute read receipts and read markers.
pub all_remote_events: AllRemoteEvents,
/// State helping matching reactions to their associated events, and
/// stashing pending reactions.
pub reactions: Reactions,
@@ -967,7 +959,6 @@ impl TimelineMetadata {
) -> Self {
Self {
own_user_id,
all_remote_events: Default::default(),
next_internal_id: Default::default(),
reactions: Default::default(),
pending_poll_events: Default::default(),
@@ -987,7 +978,6 @@ impl TimelineMetadata {
pub(crate) fn clear(&mut self) {
// Note: we don't clear the next internal id to avoid bad cases of stale unique
// ids across timeline clears.
self.all_remote_events.clear();
self.reactions.clear();
self.pending_poll_events.clear();
self.pending_edits.clear();
@@ -1008,6 +998,7 @@ impl TimelineMetadata {
&self,
event_a: &EventId,
event_b: &EventId,
all_remote_events: &AllRemoteEvents,
) -> Option<RelativePosition> {
if event_a == event_b {
return Some(RelativePosition::Same);
@@ -1015,11 +1006,11 @@ impl TimelineMetadata {
// We can make early returns here because we know all events since the end of
// the timeline, so the first event encountered is the oldest one.
for meta in self.all_remote_events.iter().rev() {
if meta.event_id == event_a {
for event_meta in all_remote_events.iter().rev() {
if event_meta.event_id == event_a {
return Some(RelativePosition::Before);
}
if meta.event_id == event_b {
if event_meta.event_id == event_b {
return Some(RelativePosition::After);
}
}
@@ -1092,8 +1083,7 @@ impl TimelineMetadata {
// Only insert the read marker if it is not at the end of the timeline.
if idx + 1 < items.len() {
let idx = idx + 1;
items.insert(idx, TimelineItem::read_marker());
self.all_remote_events.timeline_item_has_been_inserted_at(idx, None);
items.insert(idx, TimelineItem::read_marker(), None);
self.has_up_to_date_read_marker_item = true;
} else {
// The next event might require a read marker to be inserted at the current
@@ -1114,7 +1104,6 @@ impl TimelineMetadata {
if from + 1 == items.len() {
// The read marker has nothing after it. An item disappeared; remove it.
items.remove(from);
self.all_remote_events.timeline_item_has_been_removed_at(from);
}
self.has_up_to_date_read_marker_item = true;
return;
@@ -1122,15 +1111,13 @@ impl TimelineMetadata {
let prev_len = items.len();
let read_marker = items.remove(from);
self.all_remote_events.timeline_item_has_been_removed_at(from);
// Only insert the read marker if it is not at the end of the timeline.
if to + 1 < prev_len {
// Since the fully-read event's index was shifted to the left
// by one position by the remove call above, insert the fully-
// read marker at its previous position, rather than that + 1
items.insert(to, read_marker);
self.all_remote_events.timeline_item_has_been_inserted_at(to, None);
items.insert(to, read_marker, None);
self.has_up_to_date_read_marker_item = true;
} else {
self.has_up_to_date_read_marker_item = false;
@@ -1140,145 +1127,6 @@ impl TimelineMetadata {
}
}
/// A type for all remote events.
///
/// Having this type helps to know exactly which parts of the code and how they
/// use all remote events. It also helps to give a bit of semantics on top of
/// them.
#[derive(Clone, Debug, Default)]
pub(crate) struct AllRemoteEvents(VecDeque<EventMeta>);
impl AllRemoteEvents {
/// Return a front-to-back iterator over all remote events.
pub fn iter(&self) -> Iter<'_, EventMeta> {
self.0.iter()
}
/// Remove all remote events.
pub fn clear(&mut self) {
self.0.clear();
}
/// Insert a new remote event at the front of all the others.
pub fn push_front(&mut self, event_meta: EventMeta) {
// If there is an associated `timeline_item_index`, shift all the
// `timeline_item_index` that come after this one.
if let Some(new_timeline_item_index) = event_meta.timeline_item_index {
self.increment_all_timeline_item_index_after(new_timeline_item_index);
}
// Push the event.
self.0.push_front(event_meta)
}
/// Insert a new remote event at the back of all the others.
pub fn push_back(&mut self, event_meta: EventMeta) {
// If there is an associated `timeline_item_index`, shift all the
// `timeline_item_index` that come after this one.
if let Some(new_timeline_item_index) = event_meta.timeline_item_index {
self.increment_all_timeline_item_index_after(new_timeline_item_index);
}
// Push the event.
self.0.push_back(event_meta)
}
/// Remove one remote event at a specific index, and return it if it exists.
pub fn remove(&mut self, event_index: usize) -> Option<EventMeta> {
// Remove the event.
let event_meta = self.0.remove(event_index)?;
// If there is an associated `timeline_item_index`, shift all the
// `timeline_item_index` that come after this one.
if let Some(removed_timeline_item_index) = event_meta.timeline_item_index {
self.decrement_all_timeline_item_index_after(removed_timeline_item_index);
};
Some(event_meta)
}
/// Return a reference to the last remote event if it exists.
pub fn last(&self) -> Option<&EventMeta> {
self.0.back()
}
/// Return the index of the last remote event if it exists.
pub fn last_index(&self) -> Option<usize> {
self.0.len().checked_sub(1)
}
/// Get a mutable reference to a specific remote event by its ID.
pub fn get_by_event_id_mut(&mut self, event_id: &EventId) -> Option<&mut EventMeta> {
self.0.iter_mut().rev().find(|event_meta| event_meta.event_id == event_id)
}
/// Shift to the right all timeline item indexes that are equal to or
/// greater than `new_timeline_item_index`.
fn increment_all_timeline_item_index_after(&mut self, new_timeline_item_index: usize) {
for event_meta in self.0.iter_mut() {
if let Some(timeline_item_index) = event_meta.timeline_item_index.as_mut() {
if *timeline_item_index >= new_timeline_item_index {
*timeline_item_index += 1;
}
}
}
}
/// Shift to the left all timeline item indexes that are greater than
/// `removed_wtimeline_item_index`.
fn decrement_all_timeline_item_index_after(&mut self, removed_timeline_item_index: usize) {
for event_meta in self.0.iter_mut() {
if let Some(timeline_item_index) = event_meta.timeline_item_index.as_mut() {
if *timeline_item_index > removed_timeline_item_index {
*timeline_item_index -= 1;
}
}
}
}
pub fn timeline_item_has_been_inserted_at(
&mut self,
new_timeline_item_index: usize,
event_index: Option<usize>,
) {
self.increment_all_timeline_item_index_after(new_timeline_item_index);
if let Some(event_index) = event_index {
if let Some(event_meta) = self.0.get_mut(event_index) {
event_meta.timeline_item_index = Some(new_timeline_item_index);
}
}
}
pub fn timeline_item_has_been_removed_at(&mut self, timeline_item_index_to_remove: usize) {
for event_meta in self.0.iter_mut() {
let mut remove_timeline_item_index = false;
// A `timeline_item_index` is removed. Let's shift all indexes that come
// after the removed one.
if let Some(timeline_item_index) = event_meta.timeline_item_index.as_mut() {
match (*timeline_item_index).cmp(&timeline_item_index_to_remove) {
Ordering::Equal => {
remove_timeline_item_index = true;
}
Ordering::Greater => {
*timeline_item_index -= 1;
}
Ordering::Less => {}
}
}
// This is the `event_meta` that holds the `timeline_item_index` that is being
// removed. So let's clean it.
if remove_timeline_item_index {
event_meta.timeline_item_index = None;
}
}
}
}
/// Full metadata about an event.
///
/// Only used to group function parameters.
@@ -1317,9 +1165,9 @@ pub(crate) struct EventMeta {
/// Foundation for the mapping between remote events to timeline items.
///
/// Let's explain it. The events represent the first set and are stored in
/// [`TimelineMetadata::all_remote_events`], and the timeline
/// [`ObservableItems::all_remote_events`], and the timeline
/// items represent the second set and are stored in
/// [`TimelineState::items`].
/// [`ObservableItems::items`].
///
/// Each event is mapped to at most one timeline item:
///

62
crates/matrix-sdk-ui/src/timeline/day_dividers.rs
View File

@@ -301,11 +301,11 @@ impl DayDividerAdjuster {
// Keep push semantics, if we're inserting at the front or the back.
if at == items.len() {
items.push_back(item);
items.push_back(item, None);
} else if at == 0 {
items.push_front(item);
items.push_front(item, None);
} else {
items.insert(at, item);
items.insert(at, item, None);
}
offset += 1;
@@ -654,9 +654,12 @@ mod tests {
let timestamp_next_day =
MilliSecondsSinceUnixEpoch((42 + 3600 * 24 * 1000).try_into().unwrap());
txn.push_back(meta.new_timeline_item(event_with_ts(timestamp)));
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp_next_day)));
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::ReadMarker));
txn.push_back(meta.new_timeline_item(event_with_ts(timestamp)), None);
txn.push_back(
meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp_next_day)),
None,
);
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::ReadMarker), None);
let mut adjuster = DayDividerAdjuster::default();
adjuster.run(&mut txn, &mut meta);
@@ -690,10 +693,13 @@ mod tests {
assert_ne!(timestamp_to_date(timestamp), timestamp_to_date(timestamp_next_day));
let event = event_with_ts(timestamp);
txn.push_back(meta.new_timeline_item(event.clone()));
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp_next_day)));
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::ReadMarker));
txn.push_back(meta.new_timeline_item(event));
txn.push_back(meta.new_timeline_item(event.clone()), None);
txn.push_back(
meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp_next_day)),
None,
);
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::ReadMarker), None);
txn.push_back(meta.new_timeline_item(event), None);
let mut adjuster = DayDividerAdjuster::default();
adjuster.run(&mut txn, &mut meta);
@@ -721,12 +727,12 @@ mod tests {
MilliSecondsSinceUnixEpoch((42 + 3600 * 24 * 1000).try_into().unwrap());
assert_ne!(timestamp_to_date(timestamp), timestamp_to_date(timestamp_next_day));
txn.push_back(meta.new_timeline_item(event_with_ts(timestamp)));
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp)));
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp)));
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::ReadMarker));
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp)));
txn.push_back(meta.new_timeline_item(event_with_ts(timestamp_next_day)));
txn.push_back(meta.new_timeline_item(event_with_ts(timestamp)), None);
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp)), None);
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp)), None);
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::ReadMarker), None);
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp)), None);
txn.push_back(meta.new_timeline_item(event_with_ts(timestamp_next_day)), None);
let mut adjuster = DayDividerAdjuster::default();
adjuster.run(&mut txn, &mut meta);
@@ -755,10 +761,10 @@ mod tests {
MilliSecondsSinceUnixEpoch((42 + 3600 * 24 * 1000).try_into().unwrap());
assert_ne!(timestamp_to_date(timestamp), timestamp_to_date(timestamp_next_day));
txn.push_back(meta.new_timeline_item(event_with_ts(timestamp_next_day)));
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp)));
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp)));
txn.push_back(meta.new_timeline_item(event_with_ts(timestamp_next_day)));
txn.push_back(meta.new_timeline_item(event_with_ts(timestamp_next_day)), None);
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp)), None);
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp)), None);
txn.push_back(meta.new_timeline_item(event_with_ts(timestamp_next_day)), None);
let mut adjuster = DayDividerAdjuster::default();
adjuster.run(&mut txn, &mut meta);
@@ -782,9 +788,9 @@ mod tests {
let timestamp = MilliSecondsSinceUnixEpoch(uint!(42));
txn.push_back(meta.new_timeline_item(event_with_ts(timestamp)));
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::ReadMarker));
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp)));
txn.push_back(meta.new_timeline_item(event_with_ts(timestamp)), None);
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::ReadMarker), None);
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp)), None);
let mut adjuster = DayDividerAdjuster::default();
adjuster.run(&mut txn, &mut meta);
@@ -808,9 +814,9 @@ mod tests {
let timestamp = MilliSecondsSinceUnixEpoch(uint!(42));
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::ReadMarker));
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp)));
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp)));
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::ReadMarker), None);
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp)), None);
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::DayDivider(timestamp)), None);
let mut adjuster = DayDividerAdjuster::default();
adjuster.run(&mut txn, &mut meta);
@@ -831,8 +837,8 @@ mod tests {
let mut meta = test_metadata();
let timestamp = MilliSecondsSinceUnixEpoch(uint!(42));
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::ReadMarker));
txn.push_back(meta.new_timeline_item(event_with_ts(timestamp)));
txn.push_back(meta.new_timeline_item(VirtualTimelineItem::ReadMarker), None);
txn.push_back(meta.new_timeline_item(event_with_ts(timestamp)), None);
let mut adjuster = DayDividerAdjuster::default();
adjuster.run(&mut txn, &mut meta);

28
crates/matrix-sdk-ui/src/timeline/event_handler.rs
View File

@@ -514,7 +514,6 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
// wouldn't normally be visible. Remove it.
trace!("Removing UTD that was successfully retried");
self.items.remove(timeline_item_index);
self.meta.all_remote_events.timeline_item_has_been_removed_at(timeline_item_index);
self.result.item_removed = true;
}
@@ -1095,7 +1094,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
trace!("Adding new local timeline item");
let item = self.meta.new_timeline_item(item);
self.items.push_back(item);
self.items.push_back(item, None);
}
Flow::Remote { position: TimelineItemPosition::Start { .. }, event_id, .. } => {
@@ -1112,8 +1111,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
trace!("Adding new remote timeline item at the start");
let item = self.meta.new_timeline_item(item);
self.items.push_front(item);
self.meta.all_remote_events.timeline_item_has_been_inserted_at(0, Some(0));
self.items.push_front(item, Some(0));
}
Flow::Remote {
@@ -1189,6 +1187,13 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
})
.unwrap_or(0);
let event_index =
Some(self.items.all_remote_events().last_index()
// The last remote event is necessarily associated to this
// timeline item, see the contract of this method.
.expect("A timeline item is being added but its associated remote event is missing")
);
// Try to keep precise insertion semantics here, in this exact order:
//
// * _push back_ when the new item is inserted after all items (the assumption
@@ -1199,26 +1204,17 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
if timeline_item_index == self.items.len() {
trace!("Adding new remote timeline item at the back");
self.items.push_back(new_item);
self.items.push_back(new_item, event_index);
} else if timeline_item_index == 0 {
trace!("Adding new remote timeline item at the front");
self.items.push_front(new_item);
self.items.push_front(new_item, event_index);
} else {
trace!(
timeline_item_index,
"Adding new remote timeline item at specific index"
);
self.items.insert(timeline_item_index, new_item);
self.items.insert(timeline_item_index, new_item, event_index);
}
self.meta.all_remote_events.timeline_item_has_been_inserted_at(
timeline_item_index,
Some(self.meta.all_remote_events.last_index()
// The last remote event is necessarily associated to this
// timeline item, see the contract of this method.
.expect("A timeline item is being added but its associated remote event is missing")
),
);
}
Flow::Remote {

60
crates/matrix-sdk-ui/src/timeline/read_receipts.rs
View File

@@ -99,9 +99,10 @@ impl ReadReceipts {
&mut self,
new_receipt: FullReceipt<'_>,
is_own_user_id: bool,
all_events: &AllRemoteEvents,
timeline_items: &mut ObservableItemsTransaction<'_>,
) {
let all_events = timeline_items.all_remote_events();
// Get old receipt.
let old_receipt = self.get_latest(new_receipt.user_id, &new_receipt.receipt_type);
if old_receipt
@@ -382,7 +383,6 @@ impl TimelineStateTransaction<'_> {
self.meta.read_receipts.maybe_update_read_receipt(
full_receipt,
is_own_user_id,
&self.meta.all_remote_events,
&mut self.items,
);
}
@@ -417,7 +417,6 @@ impl TimelineStateTransaction<'_> {
self.meta.read_receipts.maybe_update_read_receipt(
full_receipt,
user_id == own_user_id,
&self.meta.all_remote_events,
&mut self.items,
);
}
@@ -446,7 +445,6 @@ impl TimelineStateTransaction<'_> {
self.meta.read_receipts.maybe_update_read_receipt(
full_receipt,
is_own_event,
&self.meta.all_remote_events,
&mut self.items,
);
}
@@ -456,8 +454,8 @@ impl TimelineStateTransaction<'_> {
pub(super) fn maybe_update_read_receipts_of_prev_event(&mut self, event_id: &EventId) {
// Find the previous visible event, if there is one.
let Some(prev_event_meta) = self
.meta
.all_remote_events
.items
.all_remote_events()
.iter()
.rev()
// Find the event item.
@@ -487,7 +485,7 @@ impl TimelineStateTransaction<'_> {
let read_receipts = self.meta.read_receipts.compute_event_receipts(
&remote_prev_event_item.event_id,
&self.meta.all_remote_events,
&self.items.all_remote_events(),
false,
);
@@ -535,18 +533,24 @@ impl TimelineState {
user_id: &UserId,
room_data_provider: &P,
) -> Option<(OwnedEventId, Receipt)> {
let public_read_receipt =
self.meta.user_receipt(user_id, ReceiptType::Read, room_data_provider).await;
let private_read_receipt =
self.meta.user_receipt(user_id, ReceiptType::ReadPrivate, room_data_provider).await;
let all_remote_events = self.items.all_remote_events();
let public_read_receipt = self
.meta
.user_receipt(user_id, ReceiptType::Read, room_data_provider, all_remote_events)
.await;
let private_read_receipt = self
.meta
.user_receipt(user_id, ReceiptType::ReadPrivate, room_data_provider, all_remote_events)
.await;
// Let's assume that a private read receipt should be more recent than a public
// read receipt, otherwise there's no point in the private read receipt,
// and use it as default.
match self
.meta
.compare_optional_receipts(public_read_receipt.as_ref(), private_read_receipt.as_ref())
{
match self.meta.compare_optional_receipts(
public_read_receipt.as_ref(),
private_read_receipt.as_ref(),
self.items.all_remote_events(),
) {
Ordering::Greater => public_read_receipt,
Ordering::Less => private_read_receipt,
_ => unreachable!(),
@@ -568,16 +572,19 @@ impl TimelineState {
// Let's assume that a private read receipt should be more recent than a public
// read receipt, otherwise there's no point in the private read receipt,
// and use it as default.
let (latest_receipt_id, _) =
match self.meta.compare_optional_receipts(public_read_receipt, private_read_receipt) {
Ordering::Greater => public_read_receipt?,
Ordering::Less => private_read_receipt?,
_ => unreachable!(),
};
let (latest_receipt_id, _) = match self.meta.compare_optional_receipts(
public_read_receipt,
private_read_receipt,
self.items.all_remote_events(),
) {
Ordering::Greater => public_read_receipt?,
Ordering::Less => private_read_receipt?,
_ => unreachable!(),
};
// Find the corresponding visible event.
self.meta
.all_remote_events
self.items
.all_remote_events()
.iter()
.rev()
.skip_while(|ev| ev.event_id != *latest_receipt_id)
@@ -597,6 +604,7 @@ impl TimelineMetadata {
user_id: &UserId,
receipt_type: ReceiptType,
room_data_provider: &P,
all_remote_events: &AllRemoteEvents,
) -> Option<(OwnedEventId, Receipt)> {
if let Some(receipt) = self.read_receipts.get_latest(user_id, &receipt_type) {
// Since it is in the timeline, it should be the most recent.
@@ -616,6 +624,7 @@ impl TimelineMetadata {
match self.compare_optional_receipts(
main_thread_read_receipt.as_ref(),
unthreaded_read_receipt.as_ref(),
all_remote_events,
) {
Ordering::Greater => main_thread_read_receipt,
Ordering::Less => unthreaded_read_receipt,
@@ -633,6 +642,7 @@ impl TimelineMetadata {
&self,
lhs: Option<&(OwnedEventId, Receipt)>,
rhs_or_default: Option<&(OwnedEventId, Receipt)>,
all_remote_events: &AllRemoteEvents,
) -> Ordering {
// If we only have one, use it.
let Some((lhs_event_id, lhs_receipt)) = lhs else {
@@ -643,7 +653,9 @@ impl TimelineMetadata {
};
// Compare by position in the timeline.
if let Some(relative_pos) = self.compare_events_positions(lhs_event_id, rhs_event_id) {
if let Some(relative_pos) =
self.compare_events_positions(lhs_event_id, rhs_event_id, all_remote_events)
{
if relative_pos == RelativePosition::Before {
return Ordering::Greater;
}