Skip to content

Commit

Permalink
timeline: when an edit fails on a stale local echo, retry on the matc…
Browse files Browse the repository at this point in the history
…hing remote echo
  • Loading branch information
bnjbvr committed Jul 3, 2024
1 parent acd21d9 commit 14c0c5e
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 11 deletions.
1 change: 1 addition & 0 deletions crates/matrix-sdk-ui/src/timeline/day_dividers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,7 @@ mod tests {
fn event_with_ts(timestamp: MilliSecondsSinceUnixEpoch) -> EventTimelineItem {
let event_kind = EventTimelineItemKind::Remote(RemoteEventTimelineItem {
event_id: owned_event_id!("$1"),
transaction_id: None,
reactions: Default::default(),
read_receipts: Default::default(),
is_own: false,
Expand Down
3 changes: 2 additions & 1 deletion crates/matrix-sdk-ui/src/timeline/event_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -891,7 +891,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
}
.into(),

Flow::Remote { event_id, raw_event, position, .. } => {
Flow::Remote { event_id, raw_event, position, txn_id, .. } => {
// Drop pending reactions if the message is redacted.
if let TimelineItemContent::RedactedMessage = content {
if !reactions.is_empty() {
Expand All @@ -916,6 +916,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {

RemoteEventTimelineItem {
event_id: event_id.clone(),
transaction_id: txn_id.clone(),
reactions,
read_receipts: self.ctx.read_receipts.clone(),
is_own: self.ctx.is_own_event,
Expand Down
1 change: 1 addition & 0 deletions crates/matrix-sdk-ui/src/timeline/event_item/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ impl EventTimelineItem {

let event_kind = RemoteEventTimelineItem {
event_id,
transaction_id: None,
reactions,
read_receipts,
is_own,
Expand Down
7 changes: 6 additions & 1 deletion crates/matrix-sdk-ui/src/timeline/event_item/remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use matrix_sdk::deserialized_responses::EncryptionInfo;
use ruma::{
events::{receipt::Receipt, AnySyncTimelineEvent},
serde::Raw,
OwnedEventId, OwnedUserId,
OwnedEventId, OwnedTransactionId, OwnedUserId,
};

use super::BundledReactions;
Expand All @@ -30,6 +30,9 @@ pub(in crate::timeline) struct RemoteEventTimelineItem {
/// The event ID.
pub event_id: OwnedEventId,

/// If available, the transaction id we've used to send this event.
pub transaction_id: Option<OwnedTransactionId>,

/// All bundled reactions about the event.
pub reactions: BundledReactions,

Expand Down Expand Up @@ -107,6 +110,7 @@ impl fmt::Debug for RemoteEventTimelineItem {
// skip raw JSON, too noisy
let Self {
event_id,
transaction_id,
reactions,
read_receipts,
is_own,
Expand All @@ -119,6 +123,7 @@ impl fmt::Debug for RemoteEventTimelineItem {

f.debug_struct("RemoteEventTimelineItem")
.field("event_id", event_id)
.field("transaction_id", transaction_id)
.field("reactions", reactions)
.field("read_receipts", read_receipts)
.field("is_own", is_own)
Expand Down
53 changes: 44 additions & 9 deletions crates/matrix-sdk-ui/src/timeline/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,22 @@ impl Timeline {
})
}

/// Given a transaction id, try to find a remote echo that used this
/// transaction id upon sending.
async fn find_remote_by_transaction_id(&self, txn_id: &TransactionId) -> Option<OwnedEventId> {
let items = self.inner.items().await;

let (_, found) = rfind_event_item(&items, |item| {
if let Some(remote) = item.as_remote() {
remote.transaction_id.as_deref() == Some(txn_id)
} else {
false
}
})?;

Some(found.event_id().expect("remote echoes have event id").to_owned())
}

/// Edit an event.
///
/// Only supports events for which [`EventTimelineItem::is_editable()`]
Expand All @@ -503,19 +519,38 @@ impl Timeline {
) -> Result<bool, RoomSendQueueError> {
let event_id = match edit_info.id {
TimelineEventItemId::TransactionId(txn_id) => {
let Some(item) = self.item_by_transaction_id(&txn_id).await else {
warn!("Couldn't find the local echo anymore");
return Ok(false);
};
if let Some(item) = self.item_by_transaction_id(&txn_id).await {
let Some(handle) = item.as_local().and_then(|item| item.send_handle.clone())
else {
warn!("No handle for a local echo; is this a test?");
return Ok(false);
};

if let Some(handle) = item.as_local().and_then(|item| item.send_handle.clone()) {
// Assume no relations, since it's not been sent yet.
let new_content: RoomMessageEventContent = new_content.into();
return Ok(handle.edit(new_content.into()).await?);
let new_content: RoomMessageEventContent = new_content.clone().into();

if handle.edit(new_content.into()).await? {
return Ok(true);
}
}

warn!("No handle for a local echo; should only happen in testing situations");
return Ok(false);
// We end up here in two cases: either there wasn't a local echo with this
// transaction id, or the send queue refused to edit the local echo (likely
// because it's sent).
//
// Try to find a matching local echo that now has an event id (it's been sent),
// or a remote echo with a matching transaction id, so as to
// send an actual edit.
if let Some(TimelineEventItemId::EventId(event_id)) =
self.item_by_transaction_id(&txn_id).await.map(|item| item.identifier())
{
event_id
} else if let Some(event_id) = self.find_remote_by_transaction_id(&txn_id).await {
event_id
} else {
warn!("Couldn't find the local echo anymore, nor a matching remote echo");
return Ok(false);
}
}

TimelineEventItemId::EventId(event_id) => event_id,
Expand Down

0 comments on commit 14c0c5e

Please sign in to comment.