From 3f0712010f686de2f2987d746f89647039e7dfe0 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Tue, 17 Dec 2024 16:15:11 +0100 Subject: [PATCH] refactor(event cache): add a way to know if we deduplicated all events (at least one) --- .../matrix-sdk/src/event_cache/pagination.rs | 11 ++-- .../matrix-sdk/src/event_cache/room/events.rs | 66 +++++++++++++++---- 2 files changed, 60 insertions(+), 17 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/pagination.rs b/crates/matrix-sdk/src/event_cache/pagination.rs index 20f1f14c4..7ceb38fc4 100644 --- a/crates/matrix-sdk/src/event_cache/pagination.rs +++ b/crates/matrix-sdk/src/event_cache/pagination.rs @@ -185,7 +185,7 @@ impl RoomPagination { let first_event_pos = room_events.events().next().map(|(item_pos, _)| item_pos); // First, insert events. - let insert_new_gap_pos = if let Some(gap_id) = prev_gap_id { + let (add_event_report, insert_new_gap_pos) = if let Some(gap_id) = prev_gap_id { // There is a prior gap, let's replace it by new events! trace!("replaced gap with new events from backpagination"); room_events @@ -195,16 +195,17 @@ impl RoomPagination { // No prior gap, but we had some events: assume we need to prepend events // before those. trace!("inserted events before the first known event"); - room_events + let report = room_events .insert_events_at(sync_events, pos) .expect("pos is a valid position we just read above"); - Some(pos) + (report, Some(pos)) } else { // No prior gap, and no prior events: push the events. trace!("pushing events received from back-pagination"); - room_events.push_events(sync_events); + let report = room_events.push_events(sync_events); // A new gap may be inserted before the new events, if there are any. - room_events.events().next().map(|(item_pos, _)| item_pos) + let next_pos = room_events.events().next().map(|(item_pos, _)| item_pos); + (report, next_pos) }; // And insert the new gap if needs be. diff --git a/crates/matrix-sdk/src/event_cache/room/events.rs b/crates/matrix-sdk/src/event_cache/room/events.rs index 67a656351..81e75bf79 100644 --- a/crates/matrix-sdk/src/event_cache/room/events.rs +++ b/crates/matrix-sdk/src/event_cache/room/events.rs @@ -95,13 +95,18 @@ impl RoomEvents { /// Push events after all events or gaps. /// /// The last event in `events` is the most recent one. - pub fn push_events(&mut self, events: I) + pub fn push_events(&mut self, events: I) -> AddEventReport where I: IntoIterator, { let (unique_events, duplicated_event_ids) = self.filter_duplicated_events(events.into_iter()); + let report = AddEventReport { + num_new_unique: unique_events.len(), + num_duplicated: duplicated_event_ids.len(), + }; + // Remove the _old_ duplicated events! // // We don't have to worry the removals can change the position of the existing @@ -110,6 +115,8 @@ impl RoomEvents { // Push new `events`. self.chunks.push_items_back(unique_events); + + report } /// Push a gap after all events or gaps. @@ -118,13 +125,22 @@ impl RoomEvents { } /// Insert events at a specified position. - pub fn insert_events_at(&mut self, events: I, mut position: Position) -> Result<(), Error> + pub fn insert_events_at( + &mut self, + events: I, + mut position: Position, + ) -> Result where I: IntoIterator, { let (unique_events, duplicated_event_ids) = self.filter_duplicated_events(events.into_iter()); + let report = AddEventReport { + num_new_unique: unique_events.len(), + num_duplicated: duplicated_event_ids.len(), + }; + // Remove the _old_ duplicated events! // // We **have to worry* the removals can change the position of the @@ -132,7 +148,9 @@ impl RoomEvents { // argument value for each removal. self.remove_events_and_update_insert_position(duplicated_event_ids, &mut position); - self.chunks.insert_items_at(unique_events, position) + self.chunks.insert_items_at(unique_events, position)?; + + Ok(report) } /// Insert a gap at a specified position. @@ -145,19 +163,24 @@ impl RoomEvents { /// Because the `gap_identifier` can represent non-gap chunk, this method /// returns a `Result`. /// - /// This method returns either the position of the first chunk that's been - /// created, or the next insert position if the chunk has been removed. + /// This method returns a reference to the (first if many) newly created + /// `Chunk` that contains the `items`. pub fn replace_gap_at( &mut self, events: I, gap_identifier: ChunkIdentifier, - ) -> Result, Error> + ) -> Result<(AddEventReport, Option), Error> where I: IntoIterator, { let (unique_events, duplicated_event_ids) = self.filter_duplicated_events(events.into_iter()); + let report = AddEventReport { + num_new_unique: unique_events.len(), + num_duplicated: duplicated_event_ids.len(), + }; + // Remove the _old_ duplicated events! // // We don't have to worry the removals can change the position of the existing @@ -165,14 +188,15 @@ impl RoomEvents { // because of the removals. self.remove_events(duplicated_event_ids); - if unique_events.is_empty() { + let next_pos = if unique_events.is_empty() { // There are no new events, so there's no need to create a new empty items // chunk; instead, remove the gap. - self.chunks.remove_gap_at(gap_identifier) + self.chunks.remove_gap_at(gap_identifier)? } else { // Replace the gap by new events. - Ok(Some(self.chunks.replace_gap_at(unique_events, gap_identifier)?.first_position())) - } + Some(self.chunks.replace_gap_at(unique_events, gap_identifier)?.first_position()) + }; + Ok((report, next_pos)) } /// Search for a chunk, and return its identifier. @@ -398,6 +422,20 @@ impl RoomEvents { } } +pub(in crate::event_cache) struct AddEventReport { + /// Number of new unique events that have been added. + num_new_unique: usize, + /// Number of events which have been deduplicated. + num_duplicated: usize, +} + +impl AddEventReport { + /// Were all the events (at least one) we added already known? + pub fn deduplicated_all_new_events(&self) -> bool { + self.num_new_unique > 0 && self.num_new_unique == self.num_duplicated + } +} + #[cfg(test)] mod tests { use assert_matches::assert_matches; @@ -814,8 +852,10 @@ mod tests { .unwrap(); // The next insert position is the next chunk's start. - let pos = room_events.replace_gap_at([], first_gap_id).unwrap(); + let (report, pos) = room_events.replace_gap_at([], first_gap_id).unwrap(); assert_eq!(pos, Some(Position::new(ChunkIdentifier::new(2), 0))); + assert_eq!(report.num_new_unique, 0); + assert_eq!(report.num_duplicated, 0); // Remove the second gap. let second_gap_id = room_events @@ -824,8 +864,10 @@ mod tests { .unwrap(); // No next insert position. - let pos = room_events.replace_gap_at([], second_gap_id).unwrap(); + let (report, pos) = room_events.replace_gap_at([], second_gap_id).unwrap(); assert!(pos.is_none()); + assert_eq!(report.num_new_unique, 0); + assert_eq!(report.num_duplicated, 0); } #[test]