From 0e5cc44ffa30a9f8f5c2ccc5f14f3e7c49e283f4 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Sat, 13 Jan 2024 02:14:54 +0100 Subject: [PATCH] read receipts: use a lax comparison when selecting better receipts Before, restoring a timeline from the cache, for which we had a receipt for the last element, would not mark that receipt as "interesting", thus considering that all the events are new. This is incorrect, and another way to fix that would be to set `best_receipt` to the initial read receipt; but having the information that we had a change is more useful in general, so the comparison is changed from strict to lax here. --- crates/matrix-sdk-base/src/read_receipts.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/crates/matrix-sdk-base/src/read_receipts.rs b/crates/matrix-sdk-base/src/read_receipts.rs index 20803094b..20d42f69f 100644 --- a/crates/matrix-sdk-base/src/read_receipts.rs +++ b/crates/matrix-sdk-base/src/read_receipts.rs @@ -229,9 +229,10 @@ impl ReceiptSelector { // We now have a position for an event that had a read receipt, but wasn't found // before. Consider if it is the most recent now. if let Some(best_pos) = self.best_pos.as_mut() { - // Note: by using a strict comparison here, we protect against the - // server sending a receipt on the same event multiple times. - if event_pos > *best_pos { + // Note: by using a lax comparison here, we properly handle the case where we + // received events that we have already seen with a persisted read + // receipt. + if event_pos >= *best_pos { *best_pos = event_pos; self.best_receipt = Some(event_id.to_owned()); } @@ -1109,6 +1110,15 @@ mod tests { assert!(best_receipt.is_none()); } + { + // The initial active receipt is returned, when it's part of the scanned + // elements. + let mut selector = ReceiptSelector::new(&events, Some(event_id!("$1"))); + selector.try_select_better(event_id!("$1"), 0); + let best_receipt = selector.finish(); + assert_eq!(best_receipt.unwrap().event_id, event_id!("$1")); + } + { // $3 is at pos 2, $4 at position 3, so $4 wins. let mut selector = ReceiptSelector::new(&events, Some(event_id!("$3")));