matrix-org
/
matrix-rust-sdk
mirror of https://github.com/matrix-org/matrix-rust-sdk.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

feat(sdk): Reverse the indices order of `ItemPosition`. 

Previously, in a chunk with items `a`, `b` and `c`, their indices were
2, 1 and 0. It creates a problem when we push new items: all indices
must be shifted to the left inside the same chunk. That's not optimised.
This patch reverses the order, thus now `a` has index 0, `b` has index 1
and `c` has index 2.

It also removes a possible bug where we use `item_index` without
“reversing” it. This is now obvious that we don't need to re-compute the
`item_index`, we can use it directly.
This commit is contained in:
Ivan Enderlin 2024-03-19 10:40:36 +01:00
parent 628374b8d8
commit ffacbe8666
No known key found for this signature in database
1 changed files with 45 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -148,11 +148,6 @@ impl<Item, Gap, const CAP: usize> LinkedChunk<Item, Gap, CAP> {
return Err(LinkedChunkError::InvalidItemIndex { index: item_index });
}
// The `ItemPosition` is computed from the latest items. Here, we manipulate the
// items in their original order: the last item comes last. Let's adjust
// `item_index`.
let item_index = current_items_length - 1 - item_index;
// Split the items.
let detached_items = current_items.split_off(item_index);
@ -213,11 +208,6 @@ impl<Item, Gap, const CAP: usize> LinkedChunk<Item, Gap, CAP> {
return Err(LinkedChunkError::InvalidItemIndex { index: item_index });
}
// The `ItemPosition` is computed from the latest items. Here, we manipulate the
// items in their original order: the last item comes last. Let's adjust
// `item_index`.
let item_index = current_items_length - 1 - item_index;
// Split the items.
let detached_items = current_items.split_off(item_index);
@ -432,13 +422,21 @@ impl<Item, Gap, const CAP: usize> LinkedChunk<Item, Gap, CAP> {
.filter_map(|chunk| match &chunk.content {
ChunkContent::Gap(..) => None,
ChunkContent::Items(items) => {
Some(items.iter().rev().enumerate().map(move |(item_index, item)| {
(ItemPosition(chunk.identifier(), item_index), item)
let identifier = chunk.identifier();
Some(items.iter().enumerate().rev().map(move |(item_index, item)| {
(ItemPosition(identifier, item_index), item)
}))
}
})
.flatten()
.skip(position.item_index()))
.skip_while({
let expected_index = position.item_index();
move |(ItemPosition(_chunk_identifier, item_index), _item)| {
*item_index != expected_index
}
}))
}
/// Iterate over the items, starting from `position`, forward.
@ -453,12 +451,15 @@ impl<Item, Gap, const CAP: usize> LinkedChunk<Item, Gap, CAP> {
.filter_map(|chunk| match &chunk.content {
ChunkContent::Gap(..) => None,
ChunkContent::Items(items) => {
Some(items.iter().rev().enumerate().rev().map(move |(item_index, item)| {
(ItemPosition(chunk.identifier(), item_index), item)
let identifier = chunk.identifier();
Some(items.iter().enumerate().map(move |(item_index, item)| {
(ItemPosition(identifier, item_index), item)
}))
}
})
.flatten())
.flatten()
.skip(position.item_index()))
}
/// Get the first chunk, as an immutable reference.
@ -740,9 +741,14 @@ impl<Item, Gap, const CAPACITY: usize> Chunk<Item, Gap, CAPACITY> {
}
/// Get the [`ItemPosition`] of the first item.
pub fn first_item_position(&self) -> ItemPosition {
ItemPosition(self.identifier(), 0)
}
/// Get the [`ItemPosition`] of the last item.
///
/// If it's a `Gap` chunk, the last part of the tuple will always be `0`.
pub fn first_item_position(&self) -> ItemPosition {
pub fn last_item_position(&self) -> ItemPosition {
let identifier = self.identifier();
match &self.content {
@ -751,11 +757,6 @@ impl<Item, Gap, const CAPACITY: usize> Chunk<Item, Gap, CAPACITY> {
}
}
/// Get the [`ItemPosition`] of the last item.
pub fn last_item_position(&self) -> ItemPosition {
ItemPosition(self.identifier(), 0)
}
/// The length of the chunk, i.e. how many items are in it.
///
/// It will always return 0 if it's a gap chunk.
@ -996,14 +997,15 @@ mod tests {
{ $( $all )* }
{
let mut iterator = $linked_chunk
.chunks_from(ChunkIdentifierGenerator::FIRST_IDENTIFIER)
.unwrap()
.chunks()
.enumerate()
.filter_map(|(chunk_index, chunk)| match &chunk.content {
ChunkContent::Gap(..) => None,
ChunkContent::Items(items) => {
let identifier = chunk.identifier();
Some(items.iter().enumerate().map(move |(item_index, item)| {
(chunk_index, ItemPosition(chunk.identifier(), item_index), item)
(chunk_index, ItemPosition(identifier, item_index), item)
}))
}
})
@ -1236,10 +1238,10 @@ mod tests {
let mut iterator = linked_chunk.ritems();
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(3), 0), 'e')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 0), 'd')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 1), 'c')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 0), 'b')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 1), 'a')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 1), 'd')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 0), 'c')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 1), 'b')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 0), 'a')));
assert_matches!(iterator.next(), None);
}
@ -1252,10 +1254,10 @@ mod tests {
let mut iterator = linked_chunk.items();
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 1), 'a')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 0), 'b')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 1), 'c')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 0), 'd')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 0), 'a')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 1), 'b')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 0), 'c')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 1), 'd')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(3), 0), 'e')));
assert_matches!(iterator.next(), None);
}
@ -1270,9 +1272,9 @@ mod tests {
let mut iterator =
linked_chunk.ritems_from(linked_chunk.item_position(|item| *item == 'c').unwrap())?;
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 1), 'c')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 0), 'b')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 1), 'a')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 0), 'c')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 1), 'b')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 0), 'a')));
assert_matches!(iterator.next(), None);
Ok(())
@ -1288,8 +1290,8 @@ mod tests {
let mut iterator =
linked_chunk.items_from(linked_chunk.item_position(|item| *item == 'c').unwrap())?;
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 1), 'c')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 0), 'd')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 0), 'c')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 1), 'd')));
assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(3), 0), 'e')));
assert_matches!(iterator.next(), None);
@ -1367,7 +1369,7 @@ mod tests {
);
assert_matches!(
linked_chunk.insert_items_at(['u', 'v'], ItemPosition(ChunkIdentifier(6), 0),),
linked_chunk.insert_items_at(['u', 'v'], ItemPosition(ChunkIdentifier(6), 0)),
Err(LinkedChunkError::ChunkIsAGap { identifier: ChunkIdentifier(6) })
);
}
@ -1488,15 +1490,15 @@ mod tests {
// First chunk.
{
let chunk = iterator.next().unwrap();
assert_eq!(chunk.first_item_position(), ItemPosition(ChunkIdentifier(0), 2));
assert_eq!(chunk.last_item_position(), ItemPosition(ChunkIdentifier(0), 0));
assert_eq!(chunk.first_item_position(), ItemPosition(ChunkIdentifier(0), 0));
assert_eq!(chunk.last_item_position(), ItemPosition(ChunkIdentifier(0), 2));
}
// Second chunk.
{
let chunk = iterator.next().unwrap();
assert_eq!(chunk.first_item_position(), ItemPosition(ChunkIdentifier(1), 1));
assert_eq!(chunk.last_item_position(), ItemPosition(ChunkIdentifier(1), 0));
assert_eq!(chunk.first_item_position(), ItemPosition(ChunkIdentifier(1), 0));
assert_eq!(chunk.last_item_position(), ItemPosition(ChunkIdentifier(1), 1));
}
// Gap.