From c1ff5ff49f79526aa03f3354c4353de3ad2f363d Mon Sep 17 00:00:00 2001
From: Ivan Enderlin <ivan@mnt.io>
Date: Fri, 13 Dec 2024 09:10:08 +0100
Subject: [PATCH] refactor(ui): Deduplicate remote events conditionnally.

The `Timeline` has its own remote event deduplication mechanism. But
we are transitioning to receive updates from the `EventCache` via
`VectorDiff`, which are emitted via `RoomEvents`, which already runs its
own deduplication mechanism (`matrix_sdk::event_cache::Deduplicator`).
Deduplication from the `EventCache` will generate `VectorDiff::Remove`
for example. It can create a conflict with the `Timeline` deduplication
mechanism.

This patch updates the deduplication mechanism from the `Timeline`
when adding or updating remote events to be conditionnal: when
`TimelineSettings::vectordiffs_as_inputs` is enabled, the deduplication
mechanism of the `Timeline` is silent, it does nothing, otherwise it
runs.
---
 .../src/timeline/controller/state.rs          | 39 +++++++++++--------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state.rs b/crates/matrix-sdk-ui/src/timeline/controller/state.rs
index 44b9ced240..5a95e670d1 100644
--- a/crates/matrix-sdk-ui/src/timeline/controller/state.rs
+++ b/crates/matrix-sdk-ui/src/timeline/controller/state.rs
@@ -842,33 +842,38 @@ impl TimelineStateTransaction<'_> {
         room_data_provider: &P,
         settings: &TimelineSettings,
     ) {
-        // Detect if an event already exists in [`ObservableItems::all_remote_events`].
-        //
-        // Returns its position, in this case.
-        fn event_already_exists(
+        /// Remove duplicated events.
+        ///
+        /// If `VectorDiff`s are the inputs of the `Timeline`, this is not
+        /// necessary, as they are generated by the `EventCache`, which supports
+        /// its own deduplication algorithm.
+        fn deduplicate(
             new_event_id: &EventId,
-            all_remote_events: &AllRemoteEvents,
-        ) -> Option<usize> {
-            all_remote_events.iter().position(|EventMeta { event_id, .. }| event_id == new_event_id)
+            items: &mut ObservableItemsTransaction<'_>,
+            settings: &TimelineSettings,
+        ) {
+            if settings.vectordiffs_as_inputs {
+                return;
+            }
+
+            if let Some(pos) = items
+                .all_remote_events()
+                .iter()
+                .position(|EventMeta { event_id, .. }| event_id == new_event_id)
+            {
+                items.remove_remote_event(pos);
+            }
         }
 
         match position {
             TimelineItemPosition::Start { .. } => {
-                if let Some(pos) =
-                    event_already_exists(event_meta.event_id, self.items.all_remote_events())
-                {
-                    self.items.remove_remote_event(pos);
-                }
+                deduplicate(event_meta.event_id, &mut self.items, settings);
 
                 self.items.push_front_remote_event(event_meta.base_meta())
             }
 
             TimelineItemPosition::End { .. } => {
-                if let Some(pos) =
-                    event_already_exists(event_meta.event_id, self.items.all_remote_events())
-                {
-                    self.items.remove_remote_event(pos);
-                }
+                deduplicate(event_meta.event_id, &mut self.items, settings);
 
                 self.items.push_back_remote_event(event_meta.base_meta());
             }