mirror/matrix-rust-sdk
1
0
Fork 0
mirror of https://github.com/matrix-org/matrix-rust-sdk.git synced 2026-06-22 07:08:32 -04:00
Code Issues Packages Projects Releases Wiki Activity

doc+test(sdk): Add documentation for AsVectorSubscriber + improve tests.

Browse Source 
This patch add more documentation for `AsVectorSubscriber` and improve
the tests.
This commit is contained in:
Ivan Enderlin
2024-05-15 16:16:50 +02:00
parent 8d2db9d35a
commit 397a970636
2 changed files with 255 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

280
crates/matrix-sdk/src/event_cache/linked_chunk/as_vector.rs
View File

@@ -28,20 +28,38 @@ use super::{
ChunkIdentifier,
};
type Offset = usize;
/// A type alias to represent a chunk's length. This is purely for commodity.
type ChunkLength = usize;
pin_project! {
/// A type used to transform a `Stream<Item = Vec<Update<Item, Gap>>>` into
/// a `Stream<Item = Vec<VectorDiff<Item>>>`. Basically, it helps to consume
// a [`LinkedChunk<CAP, Item, Gap>`] as if it was an [`ObservableVector<Item>`].
pub struct AsVectorSubscriber<Item, Gap> {
// The inner `UpdatesSubscriber`.
#[pin]
updates_subscriber: UpdatesSubscriber<Item, Gap>,
// Pairs of all known chunks and their respective length. This is the only
// required data for this algorithm.
chunks: VecDeque<(ChunkIdentifier, ChunkLength)>,
}
}
impl<Item, Gap> AsVectorSubscriber<Item, Gap> {
pub(super) fn new(
/// Create a new [`Self`].
///
/// `updates_subscriber` is simply `UpdatesSubscriber`.
/// `initial_chunk_lengths` must be pairs of all chunk identifiers with the
/// associated chunk length. The pairs must be in the exact same order than
/// in [`LinkedChunk`].
///
/// # Safety
///
/// This method is marked as unsafe only to attract the caller's attention
/// on the order of pairs. If the order of pairs are incorrect, the entire
/// algorithm will not work properly and is very likely to panic.
pub(super) unsafe fn new(
updates_subscriber: UpdatesSubscriber<Item, Gap>,
initial_chunk_lengths: VecDeque<(ChunkIdentifier, ChunkLength)>,
) -> Self {
@@ -65,7 +83,86 @@ where
let mut diffs = Vec::with_capacity(updates.len());
let mut reattaching = false;
// A flag specifying when updates are reattaching detached items.
//
// Why is it useful?
//
// Imagine a `LinkedChunk::<3, char, ()>` containing `['a', 'b', 'c'] ['d']`. If
// one wants to insert [`w`, x`, 'y', 'z'] at position
// `Position(ChunkIdentifier(0), 1)`, i.e. at the position of `b`, here is what
// happens:
//
// 1. `LinkedChunk` will split off `['a', 'b', 'c']` at index 1, the chunk
// becomes `['a']` and `b` and `c` are _detached_, thus we have:
//
// ['a'] ['d']
//
// 2. `LinkedChunk` will then insert `w`, `x`, `y` and `z` to get:
//
// ['a', 'w', 'x'] ['y', 'z'] ['d']
//
// 3. `LinkedChunk` will now reattach `b` and `c` after `z`, like so:
//
// ['a', 'w', 'x'] ['y', 'z', 'b'] ['c'] ['d']
//
// This detaching/reattaching approach makes it reliable and safe. Good. Now,
// what updates are we going to receive for each step?
//
// Step 1, detaching last items:
//
// ```
// Update::DetachLastItems { at: Position(ChunkIdentifier(0), 1) }
// ```
//
// Step 2, inserting new items:
//
// ```
// Update::PushItems {
// position_hint: Position(ChunkIdentifier(0), 1),
// items: vec!['w', 'x'],
// }
// Update::NewItemsChunk {
// previous: Some(ChunkIdentifier(0)),
// new: ChunkIdentifier(2),
// next: Some(ChunkIdentifier(1)),
// }
// Update::PushItems {
// position_hint: Position(ChunkIdentifier(2), 0),
// items: vec!['y', 'z'],
// }
// ```
//
// Step 3, reattaching detached items:
//
// ```
// Update::ReattachItems
// Update::PushItems {
// position_hint: Position(ChunkIdentifier(2), 2),
// items: vec!['b']
// }
// Update::NewItemsChunk {
// previous: Some(ChunkIdentifier(2)),
// new: ChunkIdentifier(3),
// next: Some(ChunkIdentifier(1)),
// }
// Update::PushItems {
// position_hint: Position(ChunkIdentifier(3), 0),
// items: vec!['c'],
// }
// Update::ReattachItemsDone
// ```
//
// To ensure an optimised behaviour of this algorithm:
//
// * `Update::DetachLastItems` must not emit `VectorDiff::Remove`,
//
// * `Update::PushItems` must not emit `VectorDiff::Insert`s or
// `VectorDiff::Append`s if it happens after `ReattachItems` and before
// `ReattachItemsDone`. However, `Self::chunks` must always be updated.
//
// From the `VectorDiff` “point of view”, this optimisation aims at avoiding
// removing items to push them again later.
let mut mute_push_items = false;
for update in updates {
match update {
@@ -73,31 +170,51 @@ where
| Update::NewGapChunk { previous, new, next, .. } => {
match (previous, next) {
// New chunk at the end.
(Some(_previous), None) => {
// TODO: chec `previous` is correct
(Some(previous), None) => {
debug_assert!(
matches!(this.chunks.back(), Some((p, _)) if *p == previous),
"Inserting new chunk at the end: The previous chunk is invalid"
);
this.chunks.push_back((new, 0));
}
// New chunk at the beginning.
(None, Some(_next)) => {
// TODO: check `next` is correct
(None, Some(next)) => {
debug_assert!(
matches!(this.chunks.front(), Some((n, _)) if *n == next),
"Inserting new chunk at the end: The previous chunk is invalid"
);
this.chunks.push_front((new, 0));
}
// New chunk is inserted between 2 chunks.
(Some(_previous), Some(next)) => {
// TODO: check `previous` is correct
(Some(previous), Some(next)) => {
let next_chunk_index = this
.chunks
.iter()
.position(|(chunk_identifier, _)| *chunk_identifier == next)
.expect("next chunk not found");
// SAFETY: Assuming `LinkedChunk` and `Updates` are not buggy, and
// assuming `Self::chunks` is correctly initialized, it is not
// possible to insert a chunk between two chunks where one does not
// exist. If this predicate fails, it means `LinkedChunk` or
// `Updates` contain a bug.
.expect("Inserting new chunk: The chunk is not found");
debug_assert!(
matches!(this.chunks.get(next_chunk_index - 1), Some((p, _)) if *p == previous),
"Inserting new chunk: The previous chunk is invalid"
);
this.chunks.insert(next_chunk_index, (new, 0));
}
(None, None) => {
unreachable!("?");
unreachable!(
"Inserting new chunk with no previous nor next chunk identifiers \
is impossible"
);
}
}
}
@@ -109,9 +226,15 @@ where
.position(|(chunk_identifier, _)| {
*chunk_identifier == expected_chunk_identifier
})
.expect("oops 4");
// SAFETY: Assuming `LinkedChunk` and `Updates` are not buggy, and assuming
// `Self::chunks` is correctly initialized, it is not possible to remove a
// chunk that does not exist. If this predicate fails, it means
// `LinkedChunk` or `Updates` contain a bug.
.expect("Removing a chunk: The chunk is not found");
this.chunks.remove(chunk_index).unwrap();
// It's OK to ignore the result. The `chunk_index` exists because it's been
// found, and we don't care about its associated value.
let _ = this.chunks.remove(chunk_index);
}
Update::PushItems { position_hint: position, items } => {
@@ -134,11 +257,16 @@ where
ControlFlow::Continue(..) => None,
}
}
.expect("`ChunkIdentifier` must exist");
// SAFETY: Assuming `LinkedChunk` and `Updates` are not buggy, and assuming
// `Self::chunks` is correctly initialized, it is not possible to push items on
// a chunk that does not exist. If this predicate fails, it means `LinkedChunk`
// or `Updates` contain a bug.
.expect("Pushing items: The chunk is not found");
*chunk_length += items.len();
if reattaching {
// See `mute_push_items` to learn more.
if mute_push_items {
continue;
}
@@ -164,17 +292,23 @@ where
.find_map(|(chunk_identifier, length)| {
(*chunk_identifier == expected_chunk_identifier).then(|| length)
})
.expect("oops 3");
// SAFETY: Assuming `LinkedChunk` and `Updates` are not buggy, and assuming
// `Self::chunks` is correctly initialized, it is not possible to detach
// items from a chunk that does not exist. If this predicate fails, it means
// `LinkedChunk` or `Updates` contain a bug.
.expect("Detach last items: The chunk is not found");
*length = new_length;
}
Update::ReattachItems => {
reattaching = true;
// Entering the `reattaching` mode.
mute_push_items = true;
}
Update::ReattachItemsDone => {
reattaching = false;
// Exiting the `reattaching` mode.
mute_push_items = false;
}
}
}
@@ -199,40 +333,126 @@ mod tests {
assert_pending!(as_vector);
linked_chunk.push_items_back(['a', 'b']);
linked_chunk.push_items_back(['c', 'd', 'e']);
linked_chunk.push_items_back(['a', 'b', 'c', 'd']);
#[rustfmt::skip]
assert_items_eq!(linked_chunk, ['a', 'b', 'c'] ['d']);
// From an `ObservableVector` point of view, it would look like:
//
// 0 1 2 3 4
// +---+---+---+---+
// | a | b | c | d |
// +---+---+---+---+
// ^^^^^^^^^^^^^^^^
// |
// new
assert_next_eq!(
as_vector,
&[
VectorDiff::Append { values: vector!['a', 'b'] },
VectorDiff::Append { values: vector!['c'] },
VectorDiff::Append { values: vector!['d', 'e'] },
VectorDiff::Append { values: vector!['a', 'b', 'c'] },
VectorDiff::Append { values: vector!['d'] },
]
);
linked_chunk
.insert_items_at(['f', 'g'], linked_chunk.item_position(|item| *item == 'b').unwrap())
.insert_items_at(
['w', 'x', 'y', 'z'],
linked_chunk.item_position(|item| *item == 'b').unwrap(),
)
.unwrap();
assert_items_eq!(linked_chunk, ['a', 'w', 'x'] ['y', 'z', 'b'] ['c'] ['d']);
// From an `ObservableVector` point of view, it would look like:
//
// 0 1 2 3 4 5 6 7 8
// +---+---+---+---+---+---+---+---+
// | a | w | x | y | z | b | c | d |
// +---+---+---+---+---+---+---+---+
// ^^^^^^^^^^^^^^^^
// |
// new
assert_next_eq!(
as_vector,
&[
VectorDiff::Insert { index: 1, value: 'f' },
VectorDiff::Insert { index: 2, value: 'g' },
VectorDiff::Insert { index: 1, value: 'w' },
VectorDiff::Insert { index: 2, value: 'x' },
VectorDiff::Insert { index: 3, value: 'y' },
VectorDiff::Insert { index: 4, value: 'z' },
]
);
linked_chunk.push_gap_back(());
linked_chunk.push_items_back(['h', 'i']);
linked_chunk.push_items_back(['e', 'f', 'g', 'h']);
assert_items_eq!(
linked_chunk,
['a', 'w', 'x'] ['y', 'z', 'b'] ['c'] ['d'] [-] ['e', 'f', 'g'] ['h']
);
assert_next_eq!(as_vector, &[VectorDiff::Append { values: vector!['h', 'i'] }]);
// From an `ObservableVector` point of view, it would look like:
//
// 0 1 2 3 4 5 6 7 8 9 10 11 12
// +---+---+---+---+---+---+---+---+---+---+---+---+
// | a | w | x | y | z | b | c | d | e | f | g | h |
// +---+---+---+---+---+---+---+---+---+---+---+---+
// ^^^^^^^^^^^^^^^^
// |
// new
assert_next_eq!(
as_vector,
&[
VectorDiff::Append { values: vector!['e', 'f', 'g'] },
VectorDiff::Append { values: vector!['h'] }
]
);
linked_chunk
.replace_gap_at(['j'], linked_chunk.chunk_identifier(|chunk| chunk.is_gap()).unwrap())
.replace_gap_at(
['i', 'j', 'k', 'l'],
linked_chunk.chunk_identifier(|chunk| chunk.is_gap()).unwrap(),
)
.unwrap();
assert_items_eq!(
linked_chunk,
['a', 'w', 'x'] ['y', 'z', 'b'] ['c'] ['d'] ['i', 'j', 'k'] ['l'] ['e', 'f', 'g'] ['h']
);
assert_next_eq!(as_vector, &[VectorDiff::Insert { index: 7, value: 'j' }]);
// From an `ObservableVector` point of view, it would look like:
//
// 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16
// +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
// | a | w | x | y | z | b | c | d | i | j | k | l | e | f | g | h |
// +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
// ^^^^^^^^^^^^^^^^
// |
// new
assert_next_eq!(
as_vector,
vec![
VectorDiff::Insert { index: 8, value: 'i' },
VectorDiff::Insert { index: 9, value: 'j' },
VectorDiff::Insert { index: 10, value: 'k' },
VectorDiff::Insert { index: 11, value: 'l' },
]
);
linked_chunk
.insert_items_at(['m'], linked_chunk.item_position(|item| *item == 'a').unwrap())
.unwrap();
assert_items_eq!(
linked_chunk,
['m', 'a', 'w'] ['x'] ['y', 'z', 'b'] ['c'] ['d'] ['i', 'j', 'k'] ['l'] ['e', 'f', 'g'] ['h']
);
// From an `ObservableVector` point of view, it would look like:
//
// 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17
// +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
// | m | a | w | x | y | z | b | c | d | i | j | k | l | e | f | g | h |
// +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
// ^^^^
// |
// new
assert_next_eq!(as_vector, vec![VectorDiff::Insert { index: 0, value: 'm' }]);
drop(linked_chunk);
assert_closed!(as_vector);

8
crates/matrix-sdk/src/event_cache/linked_chunk/mod.rs
View File

@@ -732,9 +732,11 @@ impl<const CAP: usize, Item, Gap> LinkedChunk<CAP, Item, Gap> {
))
}
self.updates
.as_mut()
.map(|updates| AsVectorSubscriber::new(updates.subscribe(), initial_chunk_lengths))
self.updates.as_mut().map(|updates| {
// SAFETY: The order of chunk pairs inside `initial_chunk_lengths` is exactly
// the same as chunks in `Self`.
unsafe { AsVectorSubscriber::new(updates.subscribe(), initial_chunk_lengths) }
})
}
}