fix(sdk): Respect server ordering for remote echoes

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

@@ -532,21 +532,29 @@ impl<'a, 'i> TimelineEventHandler<'a, 'i> {
                     {
                         // TODO: Check whether anything is different about the
                         //       old and new item?
-                        self.timeline_items.set_cloned(idx, item);
-                        return;
+                        // Remove local echo, remote echo will be added below
+                        self.timeline_items.remove(idx);
                     } else {
                         warn!(
                             "Received event with transaction ID, but didn't \
                              find matching timeline item"
                         );
                     }
-                }
-
-                if let Some((idx, old_item)) = find_event_by_id(self.timeline_items, event_id) {
+                } else if let Some((idx, _old_item)) =
+                    find_local_echo_by_event_id(self.timeline_items, event_id)
+                {
                     // This occurs very often right now due to a sliding-sync
                     // bug: https://github.com/matrix-org/sliding-sync/issues/3
-                    // TODO: Use warn log level once that bug is fixed.
-                    trace!(
+                    // TODO: Remove this branch once the bug is fixed?
+                    trace!("Received remote echo without transaction ID");
+                    self.timeline_items.remove(idx);
+                } else if let Some((idx, old_item)) =
+                    find_event_by_id(self.timeline_items, event_id)
+                {
+                    // TODO: Remove for better performance? Doing another scan
+                    // over all the items if the event is not a remote echo is
+                    // slow.
+                    warn!(
                         ?item,
                         ?old_item,
                         "Received event with an ID we already have a timeline item for"
@@ -679,6 +687,19 @@ fn _update_timeline_item(
     }
 }
 
+fn find_local_echo_by_event_id<'a>(
+    lock: &'a [Arc<TimelineItem>],
+    event_id: &EventId,
+) -> Option<(usize, &'a EventTimelineItem)> {
+    lock.iter()
+        .enumerate()
+        .filter_map(|(idx, item)| Some((idx, item.as_event()?)))
+        // Note: not using event_id() method as this is only supposed to find
+        // local echoes, where the event ID is stored in the separate field
+        // rather than the key.
+        .rfind(|(_, it)| it.event_id.as_deref() == Some(event_id))
+}
+
 /// Converts a timestamp to a `(year, month, day)` tuple.
 fn timestamp_to_ymd(ts: MilliSecondsSinceUnixEpoch) -> (i32, u32, u32) {
     let datetime = Local

crates/matrix-sdk/src/room/timeline/tests.rs

@@ -416,6 +416,57 @@ async fn remote_echo_without_txn_id() {
     assert_matches!(item.as_event().unwrap().key(), TimelineKey::EventId(_));
 }
 
+#[async_test]
+async fn remote_echo_new_position() {
+    let timeline = TestTimeline::new();
+    let mut stream = timeline.stream();
+
+    // Given a local event…
+    let txn_id = timeline
+        .handle_local_event(AnyMessageLikeEventContent::RoomMessage(
+            RoomMessageEventContent::text_plain("echo"),
+        ))
+        .await;
+
+    let _day_divider = assert_matches!(stream.next().await, Some(VecDiff::Push { value }) => value);
+
+    let item = assert_matches!(stream.next().await, Some(VecDiff::Push { value }) => value);
+    let txn_id_from_event = assert_matches!(
+        item.as_event().unwrap().key(),
+        TimelineKey::TransactionId(txn_id) => txn_id
+    );
+    assert_eq!(txn_id, *txn_id_from_event);
+
+    // … and another event that comes back before the remote echo
+    timeline.handle_live_message_event(&BOB, RoomMessageEventContent::text_plain("test")).await;
+    let _day_divider = assert_matches!(stream.next().await, Some(VecDiff::Push { value }) => value);
+    let _bob_message = assert_matches!(stream.next().await, Some(VecDiff::Push { value }) => value);
+
+    // When the remote echo comes in…
+    timeline
+        .handle_live_custom_event(json!({
+            "content": {
+                "body": "echo",
+                "msgtype": "m.text",
+            },
+            "sender": &*ALICE,
+            "event_id": "$eeG0HA0FAZ37wP8kXlNkxx3I",
+            "origin_server_ts": 6,
+            "type": "m.room.message",
+            "unsigned": {
+                "transaction_id": txn_id,
+            },
+        }))
+        .await;
+
+    // … the local echo should be removed
+    assert_matches!(stream.next().await, Some(VecDiff::RemoveAt { index: 1 }));
+
+    // … and the remote echo added
+    let item = assert_matches!(stream.next().await, Some(VecDiff::Push { value }) => value);
+    assert_matches!(item.as_event().unwrap().key(), TimelineKey::EventId(_));
+}
+
 #[async_test]
 async fn day_divider() {
     let timeline = TestTimeline::new();