Skip to content

Commit

Permalink
WIP fix(event cache): don't touch the linked chunk if an operation wo…
Browse files Browse the repository at this point in the history
…uldn't cause meaningful changes
  • Loading branch information
bnjbvr committed Dec 18, 2024
1 parent 47193f1 commit c11b4ce
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 14 deletions.
36 changes: 25 additions & 11 deletions crates/matrix-sdk/src/event_cache/room/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ impl RoomEvents {
num_duplicated: duplicated_event_ids.len(),
};

if report.deduplicated_all_new_events() {
return report;
}

// Remove the _old_ duplicated events!
//
// We don't have to worry the removals can change the position of the existing
Expand Down Expand Up @@ -141,6 +145,10 @@ impl RoomEvents {
num_duplicated: duplicated_event_ids.len(),
};

if report.deduplicated_all_new_events() {
return Ok(report);
}

// Remove the _old_ duplicated events!
//
// We **have to worry* the removals can change the position of the
Expand Down Expand Up @@ -181,6 +189,11 @@ impl RoomEvents {
num_duplicated: duplicated_event_ids.len(),
};

if report.deduplicated_all_new_events() {
let pos = self.chunks.remove_gap_at(gap_identifier)?;
return Ok((report, pos));
}

// Remove the _old_ duplicated events!
//
// We don't have to worry the removals can change the position of the existing
Expand Down Expand Up @@ -505,28 +518,30 @@ mod tests {
fn test_push_events_with_duplicates() {
let (event_id_0, event_0) = new_event("$ev0");
let (event_id_1, event_1) = new_event("$ev1");
let (event_id_2, event_2) = new_event("$ev1");

let mut room_events = RoomEvents::new();

room_events.push_events([event_0.clone(), event_1]);
room_events.push_events([event_2.clone()]);

assert_events_eq!(
room_events.events(),
[
(event_id_0 at (0, 0)),
(event_id_1 at (0, 1)),
(event_id_2 at (0, 0)),
]
);

// Everything is alright. Now let's push a duplicated event.
room_events.push_events([event_0]);
// Everything is alright. Now let's push a duplicated event by simulating a
// wider sync.
room_events.push_events([event_0, event_1, event_2]);

assert_events_eq!(
room_events.events(),
[
// The first `event_id_0` has been removed.
(event_id_1 at (0, 0)),
(event_id_0 at (0, 1)),
// The first `event_id_2` has been removed.
(event_id_0 at (0, 0)),
(event_id_1 at (0, 1)),
(event_id_2 at (0, 2)),
]
);
}
Expand All @@ -552,12 +567,11 @@ mod tests {
// Everything is alright. Now let's push a duplicated event.
room_events.push_events([event_0]);

// The event has been removed, then the chunk was empty, so removed, and a new
// chunk has been created with identifier 3.
// Nothing has changed in the linked chunk.
assert_events_eq!(
room_events.events(),
[
(event_id_0 at (3, 0)),
(event_id_0 at (2, 0)),
]
);
}
Expand Down
6 changes: 3 additions & 3 deletions crates/matrix-sdk/src/event_cache/room/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1524,7 +1524,7 @@ mod tests {
assert_eq!(items[1].event_id().unwrap(), event_id2);

// A new update with one of these events leads to deduplication.
let timeline = Timeline { limited: false, prev_batch: None, events: vec![ev1] };
let timeline = Timeline { limited: false, prev_batch: None, events: vec![ev2] };
room_event_cache
.inner
.handle_joined_room_update(true, JoinedRoomUpdate { timeline, ..Default::default() })
Expand All @@ -1537,8 +1537,8 @@ mod tests {
// element anymore), and it's added to the back of the list.
let (items, _stream) = room_event_cache.subscribe().await.unwrap();
assert_eq!(items.len(), 2);
assert_eq!(items[0].event_id().unwrap(), event_id2);
assert_eq!(items[1].event_id().unwrap(), event_id1);
assert_eq!(items[0].event_id().unwrap(), event_id1);
assert_eq!(items[1].event_id().unwrap(), event_id2);
}

#[cfg(not(target_arch = "wasm32"))] // This uses the cross-process lock, so needs time support.
Expand Down

0 comments on commit c11b4ce

Please sign in to comment.