From f7df0ebf97012e4ea440e934ccff14ab38e1a834 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Tue, 1 Jul 2025 16:15:24 +0200 Subject: [PATCH] refactor(event cache): change order of parameters in `replace_gap` "Replace *this* gap with *these* events" read more natural to me than "replace with *these* events *this* gap". --- crates/matrix-sdk/src/event_cache/room/events.rs | 13 +++++++------ crates/matrix-sdk/src/event_cache/room/mod.rs | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/room/events.rs b/crates/matrix-sdk/src/event_cache/room/events.rs index a0989e816..50f6a4ed1 100644 --- a/crates/matrix-sdk/src/event_cache/room/events.rs +++ b/crates/matrix-sdk/src/event_cache/room/events.rs @@ -136,10 +136,11 @@ impl EventLinkedChunk { /// `Chunk` that contains the `items`. pub fn replace_gap_at( &mut self, - events: Vec, gap_identifier: ChunkIdentifier, + events: Vec, ) -> Result, Error> { - // As an optimization, we'll remove the empty chunk if it's a gap. + // As an optimization, we'll remove the chunk if it's a gap that would be + // replaced with no events. // // However, our linked chunk requires that it includes at least one chunk in the // in-memory representation. We could tweak this invariant, but in the @@ -602,12 +603,12 @@ mod tests { linked_chunk.push_events([event_0]); linked_chunk.push_gap(Gap { prev_token: "hello".to_owned() }); - let chunk_identifier_of_gap = linked_chunk + let gap_chunk_id = linked_chunk .chunks() .find_map(|chunk| chunk.is_gap().then_some(chunk.identifier())) .unwrap(); - linked_chunk.replace_gap_at(vec![event_1, event_2], chunk_identifier_of_gap).unwrap(); + linked_chunk.replace_gap_at(gap_chunk_id, vec![event_1, event_2]).unwrap(); assert_events_eq!( linked_chunk.events(), @@ -651,7 +652,7 @@ mod tests { .unwrap(); // The next insert position is the next chunk's start. - let pos = linked_chunk.replace_gap_at(vec![], first_gap_id).unwrap(); + let pos = linked_chunk.replace_gap_at(first_gap_id, vec![]).unwrap(); assert_eq!(pos, Some(Position::new(ChunkIdentifier::new(2), 0))); // Remove the second gap. @@ -661,7 +662,7 @@ mod tests { .unwrap(); // No next insert position. - let pos = linked_chunk.replace_gap_at(vec![], second_gap_id).unwrap(); + let pos = linked_chunk.replace_gap_at(second_gap_id, vec![]).unwrap(); assert!(pos.is_none()); } diff --git a/crates/matrix-sdk/src/event_cache/room/mod.rs b/crates/matrix-sdk/src/event_cache/room/mod.rs index 1e025ff9b..1f2b0f9c6 100644 --- a/crates/matrix-sdk/src/event_cache/room/mod.rs +++ b/crates/matrix-sdk/src/event_cache/room/mod.rs @@ -1654,7 +1654,7 @@ mod private { // the underlying gap, if the conditions are favorable to // us. self.room_linked_chunk - .replace_gap_at(reversed_events.clone(), gap_id) + .replace_gap_at(gap_id, reversed_events.clone()) .expect("gap_identifier is a valid chunk id we read previously") } else if let Some(pos) = first_event_pos { // No prior gap, but we had some events: assume we need to prepend events