day divider: make it impossible to handle an event without adjusting day dividers (#3267)

The previous API relied on the callers not forgetting about adjusting day dividers after handling an event.

This makes it statically impossible, by requiring that `TimelineEventHandler` takes a `&mut DayDividerAdjuster` when operating, that it marks as not "consumed". Later, the caller must call `DayDividerAdjuster::maybe_adjust_day_dividers()`, to mark it as consumed. When dropping, we check that it's been consumed, otherwise we panic — as it's a developer error to not call `maybe_adjust_day_dividers()`.

crates/matrix-sdk-ui/src/timeline/day_dividers.rs

@@ -21,6 +21,8 @@ use eyeball_im::ObservableVectorTransaction;
 use ruma::MilliSecondsSinceUnixEpoch;
 use tracing::{event_enabled, instrument, trace, warn, Level};
 
+#[cfg(doc)]
+use super::event_handler::TimelineEventHandler;
 use super::{
     inner::TimelineInnerMetadata, util::timestamp_to_date, TimelineItem, TimelineItemKind,
     VirtualTimelineItem,
@@ -28,17 +30,48 @@ use super::{
 
 /// Algorithm ensuring that day dividers are adjusted correctly, according to
 /// new items that have been inserted.
-#[derive(Default)]
 pub(super) struct DayDividerAdjuster {
+    /// The list of recorded operations to apply, after analyzing the latest
+    /// items.
     ops: Vec<DayDividerOperation>,
+
+    /// A boolean indicating whether the struct has been used and thus must be
+    /// mark unused manually by calling [`Self::maybe_adjust_day_dividers`].
+    consumed: bool,
+}
+
+impl Drop for DayDividerAdjuster {
+    fn drop(&mut self) {
+        assert!(
+            self.consumed,
+            "the DayDividerAdjuster must be consumed with maybe_adjust_day_dividers"
+        );
+    }
+}
+
+impl Default for DayDividerAdjuster {
+    fn default() -> Self {
+        Self {
+            ops: Default::default(),
+            // The adjuster starts as consumed, and it will be marked no consumed iff it's used
+            // with `mark_used`.
+            consumed: true,
+        }
+    }
 }
 
 impl DayDividerAdjuster {
+    /// Returns a [`DayDividerToken`] ready for consumption.
+    pub fn mark_used(&mut self) {
+        // Mark the adjuster as needing to be consumed.
+        self.consumed = false;
+    }
+
     /// Ensures that date separators are properly inserted/removed when needs
     /// be.
     #[instrument(skip_all)]
     pub fn maybe_adjust_day_dividers(
-        mut self,
+        &mut self,
         items: &mut ObservableVectorTransaction<'_, Arc<TimelineItem>>,
         meta: &mut TimelineInnerMetadata,
     ) {
@@ -94,6 +127,8 @@ impl DayDividerAdjuster {
             #[cfg(not(debug))]
             warn!("{report}");
         }
+
+        self.consumed = true;
     }
 
     #[inline]
@@ -268,14 +303,14 @@ impl DayDividerAdjuster {
     ///
     /// Returns a report if and only if there was at least one error.
     fn check_invariants<'a, 'o>(
-        self,
+        &mut self,
         items: &'a ObservableVectorTransaction<'o, Arc<TimelineItem>>,
         initial_state: Option<Vec<Arc<TimelineItem>>>,
     ) -> Option<DayDividerInvariantsReport<'a, 'o>> {
         let mut report = DayDividerInvariantsReport {
             initial_state,
             errors: Vec::new(),
-            operations: self.ops,
+            operations: std::mem::take(&mut self.ops),
             final_state: items,
         };
 

crates/matrix-sdk-ui/src/timeline/event_handler.rs

@@ -48,6 +48,7 @@ use ruma::{
 use tracing::{debug, error, field::debug, info, instrument, trace, warn};
 
 use super::{
+    day_dividers::DayDividerAdjuster,
     event_item::{
         AnyOtherFullStateEventContent, BundledReactions, EventItemIdentifier, EventSendState,
         EventTimelineItemKind, LocalEventTimelineItem, Profile, RemoteEventOrigin,
@@ -273,9 +274,15 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
     ///
     /// Returns the number of timeline updates that were made.
     #[instrument(skip_all, fields(txn_id, event_id, position))]
-    pub(super) fn handle_event(mut self, event_kind: TimelineEventKind) -> HandleEventResult {
+    pub(super) fn handle_event(
+        mut self,
+        day_divider_adjuster: &mut DayDividerAdjuster,
+        event_kind: TimelineEventKind,
+    ) -> HandleEventResult {
         let span = tracing::Span::current();
 
+        day_divider_adjuster.mark_used();
+
         let should_add = match &self.ctx.flow {
             Flow::Local { txn_id, .. } => {
                 span.record("txn_id", debug(txn_id));

crates/matrix-sdk-ui/src/timeline/inner/state.rs

@@ -184,10 +184,15 @@ impl TimelineInnerState {
         };
 
         let mut txn = self.transaction();
-        TimelineEventHandler::new(&mut txn, ctx)
-            .handle_event(TimelineEventKind::Message { content, relations: Default::default() });
 
-        txn.maybe_adjust_day_dividers();
+        let mut day_divider_adjuster = DayDividerAdjuster::default();
+
+        TimelineEventHandler::new(&mut txn, ctx).handle_event(
+            &mut day_divider_adjuster,
+            TimelineEventKind::Message { content, relations: Default::default() },
+        );
+
+        txn.adjust_day_dividers(day_divider_adjuster);
 
         txn.commit();
     }
@@ -218,20 +223,24 @@ impl TimelineInnerState {
         let mut txn = self.transaction();
         let timeline_event_handler = TimelineEventHandler::new(&mut txn, ctx);
 
+        let mut day_divider_adjuster = DayDividerAdjuster::default();
+
         match to_redact {
             EventItemIdentifier::TransactionId(txn_id) => {
-                timeline_event_handler.handle_event(TimelineEventKind::LocalRedaction {
-                    redacts: txn_id,
-                    content: content.clone(),
-                });
+                timeline_event_handler.handle_event(
+                    &mut day_divider_adjuster,
+                    TimelineEventKind::LocalRedaction { redacts: txn_id, content: content.clone() },
+                );
             }
             EventItemIdentifier::EventId(event_id) => {
-                timeline_event_handler
-                    .handle_event(TimelineEventKind::Redaction { redacts: event_id, content });
+                timeline_event_handler.handle_event(
+                    &mut day_divider_adjuster,
+                    TimelineEventKind::Redaction { redacts: event_id, content },
+                );
             }
         }
 
-        txn.maybe_adjust_day_dividers();
+        txn.adjust_day_dividers(day_divider_adjuster);
 
         txn.commit();
     }
@@ -249,6 +258,8 @@ impl TimelineInnerState {
     {
         let mut txn = self.transaction();
 
+        let mut day_divider_adjuster = DayDividerAdjuster::default();
+
         // Loop through all the indices, in order so we don't decrypt edits
         // before the event being edited, if both were UTD. Keep track of
         // index change as UTDs are removed instead of updated.
@@ -269,6 +280,7 @@ impl TimelineInnerState {
                     TimelineItemPosition::Update(idx),
                     room_data_provider,
                     settings,
+                    &mut day_divider_adjuster,
                 )
                 .await;
 
@@ -279,7 +291,7 @@ impl TimelineInnerState {
             }
         }
 
-        txn.maybe_adjust_day_dividers();
+        txn.adjust_day_dividers(day_divider_adjuster);
 
         txn.commit();
     }
@@ -443,16 +455,24 @@ impl TimelineInnerStateTransaction<'_> {
             TimelineEnd::Back { from_cache } => TimelineItemPosition::End { from_cache },
         };
 
+        let mut day_divider_adjuster = DayDividerAdjuster::default();
+
         for event in events {
             let handle_one_res = self
-                .handle_remote_event(event.into(), position, room_data_provider, settings)
+                .handle_remote_event(
+                    event.into(),
+                    position,
+                    room_data_provider,
+                    settings,
+                    &mut day_divider_adjuster,
+                )
                 .await;
 
             total.items_added += handle_one_res.item_added as u64;
             total.items_updated += handle_one_res.items_updated as u64;
         }
 
-        self.maybe_adjust_day_dividers();
+        self.adjust_day_dividers(day_divider_adjuster);
 
         total
     }
@@ -466,6 +486,7 @@ impl TimelineInnerStateTransaction<'_> {
         position: TimelineItemPosition,
         room_data_provider: &P,
         settings: &TimelineInnerSettings,
+        day_divider_adjuster: &mut DayDividerAdjuster,
     ) -> HandleEventResult {
         let raw = event.event;
         let (event_id, sender, timestamp, txn_id, event_kind, should_add) = match raw.deserialize()
@@ -576,7 +597,7 @@ impl TimelineInnerStateTransaction<'_> {
             },
         };
 
-        TimelineEventHandler::new(self, ctx).handle_event(event_kind)
+        TimelineEventHandler::new(self, ctx).handle_event(day_divider_adjuster, event_kind)
     }
 
     fn clear(&mut self) {
@@ -686,10 +707,8 @@ impl TimelineInnerStateTransaction<'_> {
         }
     }
 
-    /// Inserts/removes any day dividers that might be missing or superfluous,
-    /// according to the events we just handled.
-    fn maybe_adjust_day_dividers(&mut self) {
-        DayDividerAdjuster::default().maybe_adjust_day_dividers(&mut self.items, &mut self.meta);
+    fn adjust_day_dividers(&mut self, mut adjuster: DayDividerAdjuster) {
+        adjuster.maybe_adjust_day_dividers(&mut self.items, &mut self.meta);
     }
 }