Skip to content

Commit

Permalink
fix(linked chunk): fix order handling of initial chunks in `UpdateToV…
Browse files Browse the repository at this point in the history
…ectorDiff::new()`

The code would use a chunk iterator that moves forward, but call
`push_front()` repetitively on each chunk, semantically storing the
lengths in *reverse* order.

This could result in subsequent panics, when a new chunk was added,
because the links would not match what's expected (e.g. the last chunk
must have no successor, etc.).
  • Loading branch information
bnjbvr committed Dec 18, 2024
1 parent ff7077b commit 0ea0a5d
Showing 1 changed file with 37 additions and 3 deletions.
40 changes: 37 additions & 3 deletions crates/matrix-sdk-common/src/linked_chunk/as_vector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ impl UpdateToVectorDiff {
let mut initial_chunk_lengths = VecDeque::new();

for chunk in chunk_iterator {
initial_chunk_lengths.push_front((
initial_chunk_lengths.push_back((
chunk.identifier(),
match chunk.content() {
ChunkContent::Gap(_) => 0,
Expand Down Expand Up @@ -771,7 +771,7 @@ mod tests {
}

#[test]
fn updates_are_drained_when_constructing_as_vector() {
fn test_updates_are_drained_when_constructing_as_vector() {
let mut linked_chunk = LinkedChunk::<10, char, ()>::new_with_update_history();

linked_chunk.push_items_back(['a']);
Expand All @@ -791,6 +791,40 @@ mod tests {
assert_eq!(diffs.len(), 1);
}

#[test]
fn test_as_vector_with_initial_content() {
// Fill the linked chunk with some initial items.
let mut linked_chunk = LinkedChunk::<3, char, ()>::new_with_update_history();
linked_chunk.push_items_back(['a', 'b', 'c', 'd']);

#[rustfmt::skip]
assert_items_eq!(linked_chunk, ['a', 'b', 'c'] ['d']);

// Empty updates first.
let _ = linked_chunk.updates().take();

// Start observing future updates.
let mut as_vector = linked_chunk.as_vector().unwrap();

assert!(as_vector.take().is_empty());

// It's important to cause a change that will create new chunks, like pushing
// enough items.
linked_chunk.push_items_back(['e', 'f', 'g']);
#[rustfmt::skip]
assert_items_eq!(linked_chunk, ['a', 'b', 'c'] ['d', 'e', 'f'] ['g']);

// And the vector diffs can be computed without crashing.
let diffs = as_vector.take();
assert_eq!(diffs.len(), 2);
assert_matches!(&diffs[0], VectorDiff::Append { values } => {
assert_eq!(*values, ['e', 'f'].into());
});
assert_matches!(&diffs[1], VectorDiff::Append { values } => {
assert_eq!(*values, ['g'].into());
});
}

#[cfg(not(target_arch = "wasm32"))]
mod proptests {
use proptest::prelude::*;
Expand Down Expand Up @@ -822,7 +856,7 @@ mod tests {

proptest! {
#[test]
fn as_vector_is_correct(
fn test_as_vector_is_correct(
operations in prop::collection::vec(as_vector_operation_strategy(), 50..=200)
) {
let mut linked_chunk = LinkedChunk::<10, char, ()>::new_with_update_history();
Expand Down

0 comments on commit 0ea0a5d

Please sign in to comment.