Skip to content

Commit

Permalink
refactor(ui): Create ObservableItems(Transaction).
Browse files Browse the repository at this point in the history
  • Loading branch information
Hywan committed Dec 4, 2024
1 parent 7398694 commit 6f6e048
Show file tree
Hide file tree
Showing 6 changed files with 190 additions and 55 deletions.
12 changes: 9 additions & 3 deletions crates/matrix-sdk-ui/src/timeline/controller/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,14 @@ use tracing::{
debug, error, field, field::debug, info, info_span, instrument, trace, warn, Instrument as _,
};

pub(super) use self::state::{
AllRemoteEvents, FullEventMeta, PendingEdit, PendingEditKind, TimelineMetadata,
TimelineNewItemPosition, TimelineState, TimelineStateTransaction,
#[cfg(test)]
pub(super) use self::observable_items::ObservableItems;
pub(super) use self::{
observable_items::ObservableItemsTransaction,
state::{
AllRemoteEvents, FullEventMeta, PendingEdit, PendingEditKind, TimelineMetadata,
TimelineNewItemPosition, TimelineState, TimelineStateTransaction,
},
};
use super::{
event_handler::TimelineEventKind,
Expand All @@ -77,6 +82,7 @@ use crate::{
unable_to_decrypt_hook::UtdHookManager,
};

mod observable_items;
mod state;

/// Data associated to the current timeline focus.
Expand Down
142 changes: 142 additions & 0 deletions crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
// Copyright 2024 The Matrix.org Foundation C.I.C.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

use std::{ops::Deref, sync::Arc};

use eyeball_im::{
ObservableVector, ObservableVectorEntries, ObservableVectorEntry, ObservableVectorTransaction,
ObservableVectorTransactionEntry, VectorSubscriber,
};
use imbl::Vector;

use super::TimelineItem;

#[derive(Debug)]
pub struct ObservableItems {
items: ObservableVector<Arc<TimelineItem>>,
}

impl ObservableItems {
pub fn new() -> Self {
Self {
// Upstream default capacity is currently 16, which is making
// sliding-sync tests with 20 events lag. This should still be
// small enough.
items: ObservableVector::with_capacity(32),
}
}

pub fn is_empty(&self) -> bool {
self.items.is_empty()
}

pub fn subscribe(&self) -> VectorSubscriber<Arc<TimelineItem>> {
self.items.subscribe()
}

pub fn clone(&self) -> Vector<Arc<TimelineItem>> {
self.items.clone()
}

pub fn transaction(&mut self) -> ObservableItemsTransaction<'_> {
ObservableItemsTransaction { items: self.items.transaction() }
}

pub fn set(
&mut self,
timeline_item_index: usize,
timeline_item: Arc<TimelineItem>,
) -> Arc<TimelineItem> {
self.items.set(timeline_item_index, timeline_item)
}

pub fn entries(&mut self) -> ObservableVectorEntries<'_, Arc<TimelineItem>> {
self.items.entries()
}

pub fn for_each<F>(&mut self, f: F)
where
F: FnMut(ObservableVectorEntry<'_, Arc<TimelineItem>>),
{
self.items.for_each(f)
}
}

// It's fine to deref to an immutable reference to `Vector`.
impl Deref for ObservableItems {
type Target = Vector<Arc<TimelineItem>>;

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

#[derive(Debug)]
pub struct ObservableItemsTransaction<'observable_items> {
items: ObservableVectorTransaction<'observable_items, Arc<TimelineItem>>,
}

impl<'observable_items> ObservableItemsTransaction<'observable_items> {
pub fn get(&self, timeline_item_index: usize) -> Option<&Arc<TimelineItem>> {
self.items.get(timeline_item_index)
}

pub fn set(
&mut self,
timeline_item_index: usize,
timeline_item: Arc<TimelineItem>,
) -> Arc<TimelineItem> {
self.items.set(timeline_item_index, timeline_item)
}

pub fn remove(&mut self, timeline_item_index: usize) -> Arc<TimelineItem> {
self.items.remove(timeline_item_index)
}

pub fn insert(&mut self, timeline_item_index: usize, timeline_item: Arc<TimelineItem>) {
self.items.insert(timeline_item_index, timeline_item);
}

pub fn push_front(&mut self, timeline_item: Arc<TimelineItem>) {
self.items.push_front(timeline_item);
}

pub fn push_back(&mut self, timeline_item: Arc<TimelineItem>) {
self.items.push_back(timeline_item);
}

pub fn clear(&mut self) {
self.items.clear();
}

pub fn for_each<F>(&mut self, f: F)
where
F: FnMut(ObservableVectorTransactionEntry<'_, 'observable_items, Arc<TimelineItem>>),
{
self.items.for_each(f)
}

pub fn commit(self) {
self.items.commit()
}
}

// It's fine to deref to an immutable reference to `Vector`.
impl<'observable_items> Deref for ObservableItemsTransaction<'observable_items> {
type Target = Vector<Arc<TimelineItem>>;

fn deref(&self) -> &Self::Target {
&self.items
}
}
24 changes: 11 additions & 13 deletions crates/matrix-sdk-ui/src/timeline/controller/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use std::{
sync::{Arc, RwLock},
};

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

use super::{HandleManyEventsResult, TimelineFocusKind, TimelineSettings};
use super::{
observable_items::{ObservableItems, ObservableItemsTransaction},
HandleManyEventsResult, TimelineFocusKind, TimelineSettings,
};
use crate::{
events::SyncTimelineEventWithoutContent,
timeline::{
Expand Down Expand Up @@ -90,7 +93,7 @@ impl From<TimelineNewItemPosition> for TimelineItemPosition {

#[derive(Debug)]
pub(in crate::timeline) struct TimelineState {
pub items: ObservableVector<Arc<TimelineItem>>,
pub items: ObservableItems,
pub meta: TimelineMetadata,

/// The kind of focus of this timeline.
Expand All @@ -107,10 +110,7 @@ impl TimelineState {
is_room_encrypted: Option<bool>,
) -> Self {
Self {
// Upstream default capacity is currently 16, which is making
// sliding-sync tests with 20 events lag. This should still be
// small enough.
items: ObservableVector::with_capacity(32),
items: ObservableItems::new(),
meta: TimelineMetadata::new(
own_user_id,
room_version,
Expand Down Expand Up @@ -330,6 +330,7 @@ impl TimelineState {
pub(super) fn transaction(&mut self) -> TimelineStateTransaction<'_> {
let items = self.items.transaction();
let meta = self.meta.clone();

TimelineStateTransaction {
items,
previous_meta: &mut self.meta,
Expand All @@ -342,7 +343,7 @@ impl TimelineState {
pub(in crate::timeline) struct TimelineStateTransaction<'a> {
/// A vector transaction over the items themselves. Holds temporary state
/// until committed.
pub items: ObservableVectorTransaction<'a, Arc<TimelineItem>>,
pub items: ObservableItemsTransaction<'a>,

/// A clone of the previous meta, that we're operating on during the
/// transaction, and that will be committed to the previous meta location in
Expand Down Expand Up @@ -1041,10 +1042,7 @@ impl TimelineMetadata {
}

/// Try to update the read marker item in the timeline.
pub(crate) fn update_read_marker(
&mut self,
items: &mut ObservableVectorTransaction<'_, Arc<TimelineItem>>,
) {
pub(crate) fn update_read_marker(&mut self, items: &mut ObservableItemsTransaction<'_>) {
let Some(fully_read_event) = &self.fully_read_event else { return };
trace!(?fully_read_event, "Updating read marker");

Expand Down Expand Up @@ -1359,7 +1357,7 @@ pub(crate) struct EventMeta {
/// | 3 | content of `$ev3` | |
/// | 4 | content of `$ev5` | |
///
/// Note the day divider that is a virtual item. Also note ``$ev4`` which is
/// Note the day divider that is a virtual item. Also note `$ev4` which is
/// a reaction to `$ev2`. Finally note that `$ev1` is not rendered in
/// the timeline.
///
Expand Down
39 changes: 15 additions & 24 deletions crates/matrix-sdk-ui/src/timeline/day_dividers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@
use std::{fmt::Display, sync::Arc};

use eyeball_im::ObservableVectorTransaction;
use ruma::MilliSecondsSinceUnixEpoch;
use tracing::{error, event_enabled, instrument, trace, warn, Level};

use super::{
controller::TimelineMetadata, util::timestamp_to_date, TimelineItem, TimelineItemKind,
VirtualTimelineItem,
controller::{ObservableItemsTransaction, TimelineMetadata},
util::timestamp_to_date,
TimelineItem, TimelineItemKind, VirtualTimelineItem,
};

/// Algorithm ensuring that day dividers are adjusted correctly, according to
Expand Down Expand Up @@ -81,11 +81,7 @@ impl DayDividerAdjuster {
/// Ensures that date separators are properly inserted/removed when needs
/// be.
#[instrument(skip_all)]
pub fn run(
&mut self,
items: &mut ObservableVectorTransaction<'_, Arc<TimelineItem>>,
meta: &mut TimelineMetadata,
) {
pub fn run(&mut self, items: &mut ObservableItemsTransaction<'_>, meta: &mut TimelineMetadata) {
// We're going to record vector operations like inserting, replacing and
// removing day dividers. Since we may remove or insert new items,
// recorded offsets will change as we're iterating over the array. The
Expand Down Expand Up @@ -284,11 +280,7 @@ impl DayDividerAdjuster {
}
}

fn process_ops(
&self,
items: &mut ObservableVectorTransaction<'_, Arc<TimelineItem>>,
meta: &mut TimelineMetadata,
) {
fn process_ops(&self, items: &mut ObservableItemsTransaction<'_>, meta: &mut TimelineMetadata) {
// Record the deletion offset.
let mut offset = 0i64;
// Remember what the maximum index was, so we can assert that it's
Expand Down Expand Up @@ -366,7 +358,7 @@ impl DayDividerAdjuster {
/// Returns a report if and only if there was at least one error.
fn check_invariants<'a, 'o>(
&mut self,
items: &'a ObservableVectorTransaction<'o, Arc<TimelineItem>>,
items: &'a ObservableItemsTransaction<'o>,
initial_state: Option<Vec<Arc<TimelineItem>>>,
) -> Option<DayDividerInvariantsReport<'a, 'o>> {
let mut report = DayDividerInvariantsReport {
Expand Down Expand Up @@ -512,7 +504,7 @@ struct DayDividerInvariantsReport<'a, 'o> {
/// The operations that have been applied on the list.
operations: Vec<DayDividerOperation>,
/// Final state after inserting the day dividers.
final_state: &'a ObservableVectorTransaction<'o, Arc<TimelineItem>>,
final_state: &'a ObservableItemsTransaction<'o>,
/// Errors encountered in the algorithm.
errors: Vec<DayDividerInsertError>,
}
Expand Down Expand Up @@ -608,10 +600,9 @@ enum DayDividerInsertError {
#[cfg(test)]
mod tests {
use assert_matches2::assert_let;
use eyeball_im::ObservableVector;
use ruma::{owned_event_id, owned_user_id, uint, MilliSecondsSinceUnixEpoch};

use super::DayDividerAdjuster;
use super::{super::controller::ObservableItems, DayDividerAdjuster};
use crate::timeline::{
controller::TimelineMetadata,
event_item::{EventTimelineItemKind, RemoteEventTimelineItem},
Expand Down Expand Up @@ -654,7 +645,7 @@ mod tests {

#[test]
fn test_no_trailing_day_divider() {
let mut items = ObservableVector::new();
let mut items = ObservableItems::new();
let mut txn = items.transaction();

let mut meta = test_metadata();
Expand Down Expand Up @@ -688,7 +679,7 @@ mod tests {

#[test]
fn test_read_marker_in_between_event_and_day_divider() {
let mut items = ObservableVector::new();
let mut items = ObservableItems::new();
let mut txn = items.transaction();

let mut meta = test_metadata();
Expand Down Expand Up @@ -720,7 +711,7 @@ mod tests {

#[test]
fn test_read_marker_in_between_day_dividers() {
let mut items = ObservableVector::new();
let mut items = ObservableItems::new();
let mut txn = items.transaction();

let mut meta = test_metadata();
Expand Down Expand Up @@ -754,7 +745,7 @@ mod tests {

#[test]
fn test_remove_all_day_dividers() {
let mut items = ObservableVector::new();
let mut items = ObservableItems::new();
let mut txn = items.transaction();

let mut meta = test_metadata();
Expand Down Expand Up @@ -784,7 +775,7 @@ mod tests {

#[test]
fn test_event_read_marker_spurious_day_divider() {
let mut items = ObservableVector::new();
let mut items = ObservableItems::new();
let mut txn = items.transaction();

let mut meta = test_metadata();
Expand All @@ -810,7 +801,7 @@ mod tests {

#[test]
fn test_multiple_trailing_day_dividers() {
let mut items = ObservableVector::new();
let mut items = ObservableItems::new();
let mut txn = items.transaction();

let mut meta = test_metadata();
Expand All @@ -834,7 +825,7 @@ mod tests {

#[test]
fn test_start_with_read_marker() {
let mut items = ObservableVector::new();
let mut items = ObservableItems::new();
let mut txn = items.transaction();

let mut meta = test_metadata();
Expand Down
Loading

0 comments on commit 6f6e048

Please sign in to comment.