timeline: stash edits around in case they arrive before the related event

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

@@ -12,7 +12,11 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use std::{collections::VecDeque, future::Future, sync::Arc};
+use std::{
+    collections::{HashMap, VecDeque},
+    future::Future,
+    sync::Arc,
+};
 
 use eyeball_im::{ObservableVector, ObservableVectorTransaction, ObservableVectorTransactionEntry};
 use itertools::Itertools as _;
@@ -21,9 +25,14 @@ use matrix_sdk_base::deserialized_responses::TimelineEvent;
 #[cfg(test)]
 use ruma::events::receipt::ReceiptEventContent;
 use ruma::{
-    events::AnySyncEphemeralRoomEvent, push::Action, serde::Raw, EventId,
-    MilliSecondsSinceUnixEpoch, OwnedEventId, OwnedTransactionId, OwnedUserId, RoomVersionId,
-    UserId,
+    events::{
+        relation::Replacement, room::message::RoomMessageEventContentWithoutRelation,
+        AnySyncEphemeralRoomEvent,
+    },
+    push::Action,
+    serde::Raw,
+    EventId, MilliSecondsSinceUnixEpoch, OwnedEventId, OwnedTransactionId, OwnedUserId,
+    RoomVersionId, UserId,
 };
 use tracing::{debug, instrument, trace, warn};
 
@@ -727,7 +736,10 @@ pub(in crate::timeline) struct TimelineMetadata {
     pub all_events: VecDeque<EventMeta>,
 
     pub reactions: Reactions,
+
     pub pending_poll_events: PendingPollEvents,
+    pub pending_edits: HashMap<OwnedEventId, Replacement<RoomMessageEventContentWithoutRelation>>,
+
     pub fully_read_event: Option<OwnedEventId>,
 
     /// Whether we have a fully read-marker item in the timeline, that's up to
@@ -755,6 +767,7 @@ impl TimelineMetadata {
             next_internal_id: Default::default(),
             reactions: Default::default(),
             pending_poll_events: Default::default(),
+            pending_edits: Default::default(),
             fully_read_event: Default::default(),
             // It doesn't make sense to set this to false until we fill the `fully_read_event`
             // field, otherwise we'll keep on exiting early in `Self::update_read_marker`.
@@ -774,6 +787,7 @@ impl TimelineMetadata {
         self.all_events.clear();
         self.reactions.clear();
         self.pending_poll_events.clear();
+        self.pending_edits.clear();
         self.fully_read_event = None;
         // We forgot about the fully read marker right above, so wait for a new one
         // before attempting to update it for each new timeline item.

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

@@ -453,7 +453,39 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
         replacement: Replacement<RoomMessageEventContentWithoutRelation>,
     ) {
         let Some((item_pos, item)) = rfind_event_by_id(self.items, &replacement.event_id) else {
-            debug!("Timeline item not found, discarding edit");
+            if let Flow::Remote { position, .. } = &self.ctx.flow {
+                match position {
+                    TimelineItemPosition::Start { .. } => {
+                        // Only insert the edit if there wasn't any other edit
+                        // before.
+                        if self.meta.pending_edits.get(&replacement.event_id).is_none() {
+                            self.meta
+                                .pending_edits
+                                .insert(replacement.event_id.clone(), replacement);
+                            debug!("Timeline item not found, stashing edit");
+                        } else {
+                            debug!("Timeline item not found, but there was a previous edit for the event: discarding");
+                        }
+                    }
+
+                    TimelineItemPosition::End { .. } => {
+                        // This is a more recent edit: it's fine to overwrite the previous one, if
+                        // available.
+                        self.meta.pending_edits.insert(replacement.event_id.clone(), replacement);
+                        debug!("Timeline item not found, stashing edit");
+                    }
+
+                    TimelineItemPosition::Update(_) => {
+                        // This is not trivial: we don't really have any recency information about
+                        // the edit. Maybe there was another edit that's more recent and could be
+                        // decrypted, or maybe it's the opposite. Discard.
+                        debug!("Timeline item not found, but discarding as we don't know the relative position of this edit event");
+                    }
+                }
+            } else {
+                debug!("Local edit for a timeline item not found, discarding");
+            }
+
             return;
         };
 
@@ -969,6 +1001,12 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
             }
         }
 
+        if let Flow::Remote { event_id, .. } = &self.ctx.flow {
+            if let Some(edit) = self.meta.pending_edits.remove(event_id) {
+                self.handle_room_message_edit(edit);
+            }
+        }
+
         // If we don't have a read marker item, look if we need to add one now.
         if !self.meta.has_up_to_date_read_marker_item {
             self.meta.update_read_marker(self.items);

crates/matrix-sdk-ui/tests/integration/timeline/edit.rs

@@ -601,3 +601,77 @@ async fn test_send_edit_when_timeline_is_clear() {
 
     server.verify().await;
 }
+
+#[async_test]
+async fn test_pending_edit() {
+    let room_id = room_id!("!a98sd12bjh:example.org");
+    let (client, server) = logged_in_client_with_server().await;
+    let sync_settings = SyncSettings::new().timeout(Duration::from_millis(3000));
+
+    let f = EventFactory::new();
+
+    let mut sync_builder = SyncResponseBuilder::new();
+    sync_builder.add_joined_room(JoinedRoomBuilder::new(room_id));
+
+    mock_sync(&server, sync_builder.build_json_sync_response(), None).await;
+    let _response = client.sync_once(sync_settings.clone()).await.unwrap();
+    server.reset().await;
+
+    mock_encryption_state(&server, false).await;
+
+    let room = client.get_room(room_id).unwrap();
+    let timeline = room.timeline().await.unwrap();
+    let (_, mut timeline_stream) = timeline.subscribe().await;
+
+    // When I receive an edit event for an event I don't know about…
+    let original_event_id = event_id!("$edited");
+    let edit_event_id = event_id!("$original");
+    sync_builder.add_joined_room(
+        JoinedRoomBuilder::new(room_id).add_timeline_event(
+            f.text_msg("* hello")
+                .sender(&ALICE)
+                .event_id(edit_event_id)
+                .edit(original_event_id, RoomMessageEventContent::text_plain("[edit]").into()),
+        ),
+    );
+
+    mock_sync(&server, sync_builder.build_json_sync_response(), None).await;
+    let _response = client.sync_once(sync_settings.clone()).await.unwrap();
+    server.reset().await;
+
+    // Nothing happens.
+    assert!(timeline_stream.next().now_or_never().is_none());
+
+    // But when I receive the original event after a bit…
+    sync_builder.add_joined_room(
+        JoinedRoomBuilder::new(room_id)
+            .add_timeline_event(f.text_msg("hi").sender(&ALICE).event_id(&original_event_id)),
+    );
+
+    mock_sync(&server, sync_builder.build_json_sync_response(), None).await;
+    let _response = client.sync_once(sync_settings.clone()).await.unwrap();
+    server.reset().await;
+
+    // Then I get the content as original first…
+    assert_let!(Some(VectorDiff::PushBack { value }) = timeline_stream.next().await);
+    let item = value.as_event().unwrap();
+    assert!(item.event_id().is_some());
+    assert!(!item.is_own());
+    assert_eq!(item.content().as_message().unwrap().body(), "hi");
+
+    // And then the edit.
+    assert_next_matches!(timeline_stream, VectorDiff::Set { index: 0, value } => {
+        let item = value.as_event().unwrap();
+        assert!(item.event_id().is_some());
+        assert!(!item.is_own());
+        assert_eq!(item.content().as_message().unwrap().body(), "[edit]");
+    });
+
+    // The day divider.
+    assert_next_matches!(timeline_stream, VectorDiff::PushFront { value } => {
+        assert!(value.is_day_divider());
+    });
+
+    // And nothing else.
+    assert!(timeline_stream.next().now_or_never().is_none());
+}