From 44029009e4026d0fda23acacaa995f32ca98f550 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 18 Mar 2024 12:18:41 +0100 Subject: [PATCH 01/14] feat(sdk): Implement `ChunkIdentifier::to_last_item_position`. This patch is about an internal thing, but it makes the code easier to understand. --- crates/matrix-sdk/src/event_cache/linked_chunk.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index c13a2a7d293..5173f45bca5 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -402,7 +402,7 @@ impl LinkedChunk { /// /// It iterates from the last to the first item. pub fn ritems(&self) -> impl Iterator { - self.ritems_from(ItemPosition(self.latest_chunk().identifier(), 0)) + self.ritems_from(self.latest_chunk().identifier().to_last_item_position()) .expect("`iter_items_from` cannot fail because at least one empty chunk must exist") } @@ -553,6 +553,14 @@ impl ChunkIdentifierGenerator { #[repr(transparent)] pub struct ChunkIdentifier(u64); +impl ChunkIdentifier { + /// Transform the `ChunkIdentifier` into an `ItemPosition` representing the + /// last item position. + fn to_last_item_position(self) -> ItemPosition { + ItemPosition(self, 0) + } +} + /// The position of an item in a [`LinkedChunk`]. /// /// It's a pair of a chunk position and an item index. `(…, 0)` represents From 2bb07d6a4e1292b61b8ff550c19fcf9dafbc1a7a Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 18 Mar 2024 12:19:24 +0100 Subject: [PATCH 02/14] feat(sdk): Implement `LinkedChunk::items`. This patch implements the new `LinkedChunk::items` method that returns a forward iterator over items. --- .../src/event_cache/linked_chunk.rs | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 5173f45bca5..029d7c8464f 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -406,6 +406,19 @@ impl LinkedChunk { .expect("`iter_items_from` cannot fail because at least one empty chunk must exist") } + /// Iterate over the items, forward. + /// + /// It iterates from the first to the last item. + pub fn items(&self) -> impl Iterator { + let first_chunk = self.first_chunk(); + let ChunkContent::Items(items) = first_chunk.content() else { + unreachable!("The first chunk is necessarily an `Items`"); + }; + + self.items_from(ItemPosition(ChunkIdentifierGenerator::FIRST_IDENTIFIER, items.len())) + .expect("`items` cannot fail because at least one empty chunk must exist") + } + /// Iterate over the items, starting from `position`, backward. /// /// It iterates from the item at `position` to the first item. @@ -447,6 +460,11 @@ impl LinkedChunk { .flatten()) } + /// Get the first chunk, as an immutable reference. + fn first_chunk(&self) -> &Chunk { + unsafe { self.first.as_ref() } + } + /// Get the latest chunk, as an immutable reference. fn latest_chunk(&self) -> &Chunk { unsafe { self.last.unwrap_or(self.first).as_ref() } @@ -1165,6 +1183,23 @@ mod tests { assert_matches!(iterator.next(), None); } + #[test] + fn test_items() { + let mut linked_chunk = LinkedChunk::::new(); + linked_chunk.push_items_back(['a', 'b']); + linked_chunk.push_gap_back(()); + linked_chunk.push_items_back(['c', 'd', 'e']); + + 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(3), 0), 'e'))); + assert_matches!(iterator.next(), None); + } + #[test] fn test_ritems_from() -> Result<(), LinkedChunkError> { let mut linked_chunk = LinkedChunk::::new(); From 9c4318d191cdcdcec22dbae0b1c2f0d1dc148d6a Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 18 Mar 2024 12:21:21 +0100 Subject: [PATCH 03/14] feat(sdk): Convert `ChunkIdentifier` to `ItemPosition`. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch implements `From` for `ItemPosition`. It's useful when we get a `ChunkIdentifier` and we need to `insert_… _at(item_position)`. --- crates/matrix-sdk/src/event_cache/linked_chunk.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 029d7c8464f..e11f38f34e2 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -598,6 +598,12 @@ impl ItemPosition { } } +impl From for ItemPosition { + fn from(chunk_identifier: ChunkIdentifier) -> Self { + Self(chunk_identifier, 0) + } +} + /// An iterator over a [`LinkedChunk`] that traverses the chunk in backward /// direction (i.e. it calls `previous` on each chunk to make progress). pub struct LinkedChunkIterBackward<'a, Item, Gap, const CAP: usize> { From 4774cc8e65bef225787f0197786a4f1ef38cdeea Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 18 Mar 2024 12:22:16 +0100 Subject: [PATCH 04/14] feat(sdk): Implement `Chunk::content`. This patch implements `Chunk::content` to get an immutable reference to the content of a chunk. --- crates/matrix-sdk/src/event_cache/linked_chunk.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index e11f38f34e2..7490e0dad69 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -736,6 +736,11 @@ impl Chunk { self.identifier } + /// Get the content of the chunk. + pub fn content(&self) -> &ChunkContent { + &self.content + } + /// The length of the chunk, i.e. how many items are in it. /// /// It will always return 0 if it's a gap chunk. From 9dcab4ed30527707fb53b887d5ffcb87cb3d3e79 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 18 Mar 2024 12:57:18 +0100 Subject: [PATCH 05/14] feat(sdk): `ItemPosition` has the copy semantics. This patch implements `Copy` and `Clone` for `ItemPosition`. --- crates/matrix-sdk/src/event_cache/linked_chunk.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 7490e0dad69..5810b907110 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -583,7 +583,7 @@ impl ChunkIdentifier { /// /// It's a pair of a chunk position and an item index. `(…, 0)` represents /// the last item in the chunk. -#[derive(Debug, PartialEq)] +#[derive(Copy, Clone, Debug, PartialEq)] pub struct ItemPosition(ChunkIdentifier, usize); impl ItemPosition { From a8e522c16439d09970953cdd12a20652ef7c77c5 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 18 Mar 2024 14:11:55 +0100 Subject: [PATCH 06/14] feat(sdk): Implement `LinkedChunk::chunks`. This patch implements the `LinkedChunk::chunks` method that returns a forward iterator over the chunks. --- .../src/event_cache/linked_chunk.rs | 46 ++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 5810b907110..ed39073d680 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -367,7 +367,15 @@ impl LinkedChunk { /// It iterates from the last to the first chunk. pub fn rchunks(&self) -> LinkedChunkIterBackward<'_, Item, Gap, CAP> { self.rchunks_from(self.latest_chunk().identifier()) - .expect("`iter_chunks_from` cannot fail because at least one empty chunk must exist") + .expect("`rchunks_from` cannot fail because at least one empty chunk must exist") + } + + /// Iterate over the chunks, forward. + /// + /// It iterates from the first to the last chunk. + pub fn chunks(&self) -> LinkedChunkIter<'_, Item, Gap, CAP> { + self.chunks_from(ChunkIdentifierGenerator::FIRST_IDENTIFIER) + .expect("`chunks_from` cannot fail because at least one empty chunk must exist") } /// Iterate over the chunks, starting from `identifier`, backward. @@ -403,7 +411,7 @@ impl LinkedChunk { /// It iterates from the last to the first item. pub fn ritems(&self) -> impl Iterator { self.ritems_from(self.latest_chunk().identifier().to_last_item_position()) - .expect("`iter_items_from` cannot fail because at least one empty chunk must exist") + .expect("`ritems_from` cannot fail because at least one empty chunk must exist") } /// Iterate over the items, forward. @@ -1117,6 +1125,40 @@ mod tests { assert_matches!(iterator.next(), None); } + #[test] + fn test_chunks() { + let mut linked_chunk = LinkedChunk::::new(); + linked_chunk.push_items_back(['a', 'b']); + linked_chunk.push_gap_back(()); + linked_chunk.push_items_back(['c', 'd', 'e']); + + let mut iterator = linked_chunk.chunks(); + + assert_matches!( + iterator.next(), + Some(Chunk { identifier: ChunkIdentifier(0), content: ChunkContent::Items(items), .. }) => { + assert_eq!(items, &['a', 'b']); + } + ); + assert_matches!( + iterator.next(), + Some(Chunk { identifier: ChunkIdentifier(1), content: ChunkContent::Gap(..), .. }) + ); + assert_matches!( + iterator.next(), + Some(Chunk { identifier: ChunkIdentifier(2), content: ChunkContent::Items(items), .. }) => { + assert_eq!(items, &['c', 'd']); + } + ); + assert_matches!( + iterator.next(), + Some(Chunk { identifier: ChunkIdentifier(3), content: ChunkContent::Items(items), .. }) => { + assert_eq!(items, &['e']); + } + ); + assert_matches!(iterator.next(), None); + } + #[test] fn test_rchunks_from() -> Result<(), LinkedChunkError> { let mut linked_chunk = LinkedChunk::::new(); From 88f75a85bb0031f10403b85029cf261935678eb6 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 18 Mar 2024 14:30:30 +0100 Subject: [PATCH 07/14] feat(sdk): `Chunk::is_gap` and `::is_items` are now public. --- crates/matrix-sdk/src/event_cache/linked_chunk.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index ed39073d680..0b8a6d9d9c2 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -720,12 +720,12 @@ impl Chunk { } /// Check whether this current chunk is a gap chunk. - fn is_gap(&self) -> bool { + pub fn is_gap(&self) -> bool { matches!(self.content, ChunkContent::Gap(..)) } /// Check whether this current chunk is an items chunk. - fn is_items(&self) -> bool { + pub fn is_items(&self) -> bool { !self.is_gap() } From 6b754acd811556c05dbacd17811510850547d0e9 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 19 Mar 2024 09:28:57 +0100 Subject: [PATCH 08/14] feat(sdk): Add `Chunk::as_ptr`. This patch adds the new `Chunk::as_ptr` method. It simplifies the code a little bit. --- crates/matrix-sdk/src/event_cache/linked_chunk.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 0b8a6d9d9c2..38f9dcdcb7b 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -98,7 +98,7 @@ impl LinkedChunk { if last_chunk.is_first_chunk().not() { // Maybe `last_chunk` is the same as the previous `self.last` chunk, but it's // OK. - self.last = Some(NonNull::from(last_chunk)); + self.last = Some(last_chunk.as_ptr()); } self.length += number_of_items; @@ -176,7 +176,7 @@ impl LinkedChunk { if chunk.is_first_chunk().not() && chunk.is_last_chunk() { // Maybe `chunk` is the same as the previous `self.last` chunk, but it's // OK. - self.last = Some(NonNull::from(chunk)); + self.last = Some(chunk.as_ptr()); } self.length += number_of_items; @@ -241,7 +241,7 @@ impl LinkedChunk { if chunk.is_first_chunk().not() && chunk.is_last_chunk() { // Maybe `chunk` is the same as the previous `self.last` chunk, but it's // OK. - self.last = Some(NonNull::from(chunk)); + self.last = Some(chunk.as_ptr()); } Ok(()) @@ -719,6 +719,11 @@ impl Chunk { NonNull::from(Box::leak(chunk_box)) } + /// Get the pointer to `Self`. + pub fn as_ptr(&self) -> NonNull { + NonNull::from(self) + } + /// Check whether this current chunk is a gap chunk. pub fn is_gap(&self) -> bool { matches!(self.content, ChunkContent::Gap(..)) @@ -848,7 +853,7 @@ impl Chunk { // Link to the new chunk. self.next = Some(new_chunk_ptr); // Link the new chunk to this one. - new_chunk.previous = Some(NonNull::from(self)); + new_chunk.previous = Some(self.as_ptr()); new_chunk } From 213dac2d306fd0668f0cac46235fcbe25255ed29 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 19 Mar 2024 09:30:33 +0100 Subject: [PATCH 09/14] feat(sdk): Rewrite `LinkedChunk::replace_gap_at`. This patch rewrites `LinkedChunk::replace_gap_at`. Instead of inserting new items on the _previous_ chunk of the gap and doing all the stuff here, a new items chunk is created _after_ the gap (where items are pushed), to finally unlink and remove the gap. First off, it removes an `unwrap`. Second, if the previous chunk was an items chunk, and had free space, the items would have been added in there, which is not the intended behaviour. The tests have been updated accordingly. --- .../src/event_cache/linked_chunk.rs | 50 +++++++++---------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 38f9dcdcb7b..4969cac9654 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -270,40 +270,38 @@ impl LinkedChunk { debug_assert!(chunk.is_first_chunk().not(), "A gap cannot be the first chunk"); - let (previous, number_of_items) = match &mut chunk.content { + let (maybe_last_chunk_ptr, number_of_items) = match &mut chunk.content { ChunkContent::Gap(..) => { let items = items.into_iter(); let number_of_items = items.len(); - // Find the previous chunk… - // - // SAFETY: `unwrap` is safe because we are ensured `chunk` is not the first - // chunk, so a previous chunk always exists. - let previous = chunk.previous_mut().unwrap(); + let last_inserted_chunk = chunk + // Insert a new items chunk… + .insert_next(Chunk::new_items_leaked( + chunk_identifier_generator.generate_next().unwrap(), + )) + // … and insert the items. + .push_items(items, &chunk_identifier_generator); - // … and insert the items on it. - (previous.push_items(items, &chunk_identifier_generator), number_of_items) + ( + last_inserted_chunk.is_last_chunk().then(|| last_inserted_chunk.as_ptr()), + number_of_items, + ) } ChunkContent::Items(..) => { return Err(LinkedChunkError::ChunkIsItems { identifier: chunk_identifier }) } }; - // Get the pointer to `chunk` via `previous`. - // - // SAFETY: `unwrap` is safe because we are ensured the next of the previous - // chunk is `chunk` itself. - chunk_ptr = previous.next.unwrap(); - - // Get the pointer to the `previous` via `chunk`. - let previous_ptr = chunk.previous; - // Now that new items have been pushed, we can unlink the gap chunk. chunk.unlink(); + // Get the pointer to `chunk`. + chunk_ptr = chunk.as_ptr(); + // Update `self.last` if the gap chunk was the last chunk. - if chunk.is_last_chunk() { - self.last = previous_ptr; + if let Some(last_chunk_ptr) = maybe_last_chunk_ptr { + self.last = Some(last_chunk_ptr); } self.length += number_of_items; @@ -1432,10 +1430,10 @@ mod tests { #[test] fn test_replace_gap_at() -> Result<(), LinkedChunkError> { let mut linked_chunk = LinkedChunk::::new(); - linked_chunk.push_items_back(['a', 'b', 'c']); + linked_chunk.push_items_back(['a', 'b']); linked_chunk.push_gap_back(()); - linked_chunk.push_items_back(['l', 'm', 'n']); - assert_items_eq!(linked_chunk, ['a', 'b', 'c'] [-] ['l', 'm', 'n']); + linked_chunk.push_items_back(['l', 'm']); + assert_items_eq!(linked_chunk, ['a', 'b'] [-] ['l', 'm']); // Replace a gap in the middle of the linked chunk. { @@ -1445,7 +1443,7 @@ mod tests { linked_chunk.replace_gap_at(['d', 'e', 'f', 'g', 'h'], gap_identifier)?; assert_items_eq!( linked_chunk, - ['a', 'b', 'c'] ['d', 'e', 'f'] ['g', 'h'] ['l', 'm', 'n'] + ['a', 'b'] ['d', 'e', 'f'] ['g', 'h'] ['l', 'm'] ); } @@ -1454,7 +1452,7 @@ mod tests { linked_chunk.push_gap_back(()); assert_items_eq!( linked_chunk, - ['a', 'b', 'c'] ['d', 'e', 'f'] ['g', 'h'] ['l', 'm', 'n'] [-] + ['a', 'b'] ['d', 'e', 'f'] ['g', 'h'] ['l', 'm'] [-] ); let gap_identifier = linked_chunk.chunk_identifier(Chunk::is_gap).unwrap(); @@ -1463,11 +1461,11 @@ mod tests { linked_chunk.replace_gap_at(['w', 'x', 'y', 'z'], gap_identifier)?; assert_items_eq!( linked_chunk, - ['a', 'b', 'c'] ['d', 'e', 'f'] ['g', 'h'] ['l', 'm', 'n'] ['w', 'x', 'y'] ['z'] + ['a', 'b'] ['d', 'e', 'f'] ['g', 'h'] ['l', 'm'] ['w', 'x', 'y'] ['z'] ); } - assert_eq!(linked_chunk.len(), 15); + assert_eq!(linked_chunk.len(), 13); Ok(()) } From 06e212c11dc56690a1279ba177d0dfec817f8fdf Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 19 Mar 2024 09:48:21 +0100 Subject: [PATCH 10/14] !fix rebase issue --- crates/matrix-sdk/src/event_cache/linked_chunk.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 4969cac9654..f27811bb29b 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -415,7 +415,7 @@ impl LinkedChunk { /// Iterate over the items, forward. /// /// It iterates from the first to the last item. - pub fn items(&self) -> impl Iterator { + pub fn items(&self) -> impl Iterator { let first_chunk = self.first_chunk(); let ChunkContent::Items(items) = first_chunk.content() else { unreachable!("The first chunk is necessarily an `Items`"); @@ -467,7 +467,7 @@ impl LinkedChunk { } /// Get the first chunk, as an immutable reference. - fn first_chunk(&self) -> &Chunk { + fn first_chunk(&self) -> &Chunk { unsafe { self.first.as_ref() } } @@ -748,7 +748,7 @@ impl Chunk { } /// Get the content of the chunk. - pub fn content(&self) -> &ChunkContent { + pub fn content(&self) -> &ChunkContent { &self.content } From 454d49aa64168f2809d941a4ab616bc16b0d9979 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 19 Mar 2024 10:05:12 +0100 Subject: [PATCH 11/14] feat(sdk): Add `Chunk::first_item_position` and `::last_item_position`. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch implements `Chunk::first_item_position` and `Chunk::last_item_position` as a way to _position_ the “cursor” (`ItemPosition`) in the correct way when we want to do some particular insertion (at the beginning or at the end of a chunk). --- .../src/event_cache/linked_chunk.rs | 67 ++++++++++++++++--- 1 file changed, 58 insertions(+), 9 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index f27811bb29b..91424af88be 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -408,7 +408,7 @@ impl LinkedChunk { /// /// It iterates from the last to the first item. pub fn ritems(&self) -> impl Iterator { - self.ritems_from(self.latest_chunk().identifier().to_last_item_position()) + self.ritems_from(self.latest_chunk().last_item_position()) .expect("`ritems_from` cannot fail because at least one empty chunk must exist") } @@ -577,14 +577,6 @@ impl ChunkIdentifierGenerator { #[repr(transparent)] pub struct ChunkIdentifier(u64); -impl ChunkIdentifier { - /// Transform the `ChunkIdentifier` into an `ItemPosition` representing the - /// last item position. - fn to_last_item_position(self) -> ItemPosition { - ItemPosition(self, 0) - } -} - /// The position of an item in a [`LinkedChunk`]. /// /// It's a pair of a chunk position and an item index. `(…, 0)` represents @@ -752,6 +744,23 @@ impl Chunk { &self.content } + /// Get the [`ItemPosition`] of the first item. + /// + /// If it's a `Gap` chunk, the last part of the tuple will always be `0`. + pub fn first_item_position(&self) -> ItemPosition { + let identifier = self.identifier(); + + match &self.content { + ChunkContent::Gap(..) => ItemPosition(identifier, 0), + ChunkContent::Items(items) => ItemPosition(identifier, items.len() - 1), + } + } + + /// 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. @@ -1469,4 +1478,44 @@ mod tests { Ok(()) } + + #[test] + fn test_chunk_item_positions() { + let mut linked_chunk = LinkedChunk::::new(); + linked_chunk.push_items_back(['a', 'b', 'c', 'd', 'e']); + linked_chunk.push_gap_back(()); + linked_chunk.push_items_back(['f']); + + assert_items_eq!(linked_chunk, ['a', 'b', 'c'] ['d', 'e'] [-] ['f']); + + let mut iterator = linked_chunk.chunks(); + + // 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)); + } + + // 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)); + } + + // Gap. + { + let chunk = iterator.next().unwrap(); + assert_eq!(chunk.first_item_position(), ItemPosition(ChunkIdentifier(2), 0)); + assert_eq!(chunk.last_item_position(), ItemPosition(ChunkIdentifier(2), 0)); + } + + // Last chunk. + { + let chunk = iterator.next().unwrap(); + assert_eq!(chunk.first_item_position(), ItemPosition(ChunkIdentifier(3), 0)); + assert_eq!(chunk.last_item_position(), ItemPosition(ChunkIdentifier(3), 0)); + } + } } From 628374b8d86653e733649a506f5ae70385cd4de1 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 19 Mar 2024 10:11:58 +0100 Subject: [PATCH 12/14] feat(sdk): Optimise `LinkedChunk` iterators. This patch optimises `LinkedChunk::rchunks` and `chunks` by _not_ using `rchunks_from` and `chunks_from`. Indeed, it's faster to not call the inner `chunk` method + `unwrap`ping the result and so on. Just a tiny optimisation. This patch also uses the new `Chunk::last_item_position` method for `LinkedChunk::items`. Abstraction for the win! --- crates/matrix-sdk/src/event_cache/linked_chunk.rs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 91424af88be..8d6326c4f0a 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -349,7 +349,7 @@ impl LinkedChunk { where P: FnMut(&'a Chunk) -> bool, { - self.rchunks().find_map(|chunk| predicate(chunk).then_some(chunk.identifier())) + self.rchunks().find_map(|chunk| predicate(chunk).then(|| chunk.identifier())) } /// Search for an item, and return its position. @@ -364,16 +364,14 @@ impl LinkedChunk { /// /// It iterates from the last to the first chunk. pub fn rchunks(&self) -> LinkedChunkIterBackward<'_, Item, Gap, CAP> { - self.rchunks_from(self.latest_chunk().identifier()) - .expect("`rchunks_from` cannot fail because at least one empty chunk must exist") + LinkedChunkIterBackward::new(self.latest_chunk()) } /// Iterate over the chunks, forward. /// /// It iterates from the first to the last chunk. pub fn chunks(&self) -> LinkedChunkIter<'_, Item, Gap, CAP> { - self.chunks_from(ChunkIdentifierGenerator::FIRST_IDENTIFIER) - .expect("`chunks_from` cannot fail because at least one empty chunk must exist") + LinkedChunkIter::new(self.first_chunk()) } /// Iterate over the chunks, starting from `identifier`, backward. @@ -417,11 +415,8 @@ impl LinkedChunk { /// It iterates from the first to the last item. pub fn items(&self) -> impl Iterator { let first_chunk = self.first_chunk(); - let ChunkContent::Items(items) = first_chunk.content() else { - unreachable!("The first chunk is necessarily an `Items`"); - }; - self.items_from(ItemPosition(ChunkIdentifierGenerator::FIRST_IDENTIFIER, items.len())) + self.items_from(first_chunk.first_item_position()) .expect("`items` cannot fail because at least one empty chunk must exist") } From ffacbe866667e7f4a60bd2bb1a1d30b11161022b Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 19 Mar 2024 10:40:36 +0100 Subject: [PATCH 13/14] feat(sdk): Reverse the indices order of `ItemPosition`. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../src/event_cache/linked_chunk.rs | 88 ++++++++++--------- 1 file changed, 45 insertions(+), 43 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 8d6326c4f0a..5f87e30c37a 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -148,11 +148,6 @@ impl LinkedChunk { 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 LinkedChunk { 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 LinkedChunk { .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 LinkedChunk { .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 Chunk { } /// 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 Chunk { } } - /// 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. From e2644829547207761e50115d132e8249956d0143 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 19 Mar 2024 20:22:08 +0100 Subject: [PATCH 14/14] feat(sdk): Rename `ItemPosition` to `Position`. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The “position” can be placed inside a `Gap`, so naming it `Item…` is a non-sense. This patch removes the `Item` prefix. --- .../src/event_cache/linked_chunk.rs | 161 +++++++++--------- crates/matrix-sdk/src/event_cache/store.rs | 22 +-- 2 files changed, 91 insertions(+), 92 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 5f87e30c37a..49702fc0b16 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -122,14 +122,14 @@ impl LinkedChunk { pub fn insert_items_at( &mut self, items: I, - position: ItemPosition, + position: Position, ) -> Result<(), LinkedChunkError> where I: IntoIterator, I::IntoIter: ExactSizeIterator, { let chunk_identifier = position.chunk_identifier(); - let item_index = position.item_index(); + let item_index = position.index(); let chunk_identifier_generator = self.chunk_identifier_generator.clone(); @@ -185,11 +185,11 @@ impl LinkedChunk { /// `Result`. pub fn insert_gap_at( &mut self, - position: ItemPosition, content: Gap, + position: Position, ) -> Result<(), LinkedChunkError> { let chunk_identifier = position.chunk_identifier(); - let item_index = position.item_index(); + let item_index = position.index(); let chunk_identifier_generator = self.chunk_identifier_generator.clone(); @@ -343,7 +343,7 @@ impl LinkedChunk { } /// Search for an item, and return its position. - pub fn item_position<'a, P>(&'a self, mut predicate: P) -> Option + pub fn item_position<'a, P>(&'a self, mut predicate: P) -> Option where P: FnMut(&'a Item) -> bool, { @@ -395,18 +395,18 @@ impl LinkedChunk { /// Iterate over the items, backward. /// /// It iterates from the last to the first item. - pub fn ritems(&self) -> impl Iterator { - self.ritems_from(self.latest_chunk().last_item_position()) + pub fn ritems(&self) -> impl Iterator { + self.ritems_from(self.latest_chunk().last_position()) .expect("`ritems_from` cannot fail because at least one empty chunk must exist") } /// Iterate over the items, forward. /// /// It iterates from the first to the last item. - pub fn items(&self) -> impl Iterator { + pub fn items(&self) -> impl Iterator { let first_chunk = self.first_chunk(); - self.items_from(first_chunk.first_item_position()) + self.items_from(first_chunk.first_position()) .expect("`items` cannot fail because at least one empty chunk must exist") } @@ -415,8 +415,8 @@ impl LinkedChunk { /// It iterates from the item at `position` to the first item. pub fn ritems_from( &self, - position: ItemPosition, - ) -> Result, LinkedChunkError> { + position: Position, + ) -> Result, LinkedChunkError> { Ok(self .rchunks_from(position.chunk_identifier())? .filter_map(|chunk| match &chunk.content { @@ -424,16 +424,18 @@ impl LinkedChunk { ChunkContent::Items(items) => { let identifier = chunk.identifier(); - Some(items.iter().enumerate().rev().map(move |(item_index, item)| { - (ItemPosition(identifier, item_index), item) - })) + Some( + items.iter().enumerate().rev().map(move |(item_index, item)| { + (Position(identifier, item_index), item) + }), + ) } }) .flatten() .skip_while({ - let expected_index = position.item_index(); + let expected_index = position.index(); - move |(ItemPosition(_chunk_identifier, item_index), _item)| { + move |(Position(_chunk_identifier, item_index), _item)| { *item_index != expected_index } })) @@ -444,8 +446,8 @@ impl LinkedChunk { /// It iterates from the item at `position` to the last item. pub fn items_from( &self, - position: ItemPosition, - ) -> Result, LinkedChunkError> { + position: Position, + ) -> Result, LinkedChunkError> { Ok(self .chunks_from(position.chunk_identifier())? .filter_map(|chunk| match &chunk.content { @@ -453,13 +455,15 @@ impl LinkedChunk { ChunkContent::Items(items) => { let identifier = chunk.identifier(); - Some(items.iter().enumerate().map(move |(item_index, item)| { - (ItemPosition(identifier, item_index), item) - })) + Some( + items.iter().enumerate().map(move |(item_index, item)| { + (Position(identifier, item_index), item) + }), + ) } }) .flatten() - .skip(position.item_index())) + .skip(position.index())) } /// Get the first chunk, as an immutable reference. @@ -573,31 +577,24 @@ impl ChunkIdentifierGenerator { #[repr(transparent)] pub struct ChunkIdentifier(u64); -/// The position of an item in a [`LinkedChunk`]. +/// The position of something inside a [`Chunk`]. /// -/// It's a pair of a chunk position and an item index. `(…, 0)` represents -/// the last item in the chunk. +/// It's a pair of a chunk position and an item index. #[derive(Copy, Clone, Debug, PartialEq)] -pub struct ItemPosition(ChunkIdentifier, usize); +pub struct Position(ChunkIdentifier, usize); -impl ItemPosition { +impl Position { /// Get the chunk identifier of the item. pub fn chunk_identifier(&self) -> ChunkIdentifier { self.0 } - /// Get the item index inside its chunk. - pub fn item_index(&self) -> usize { + /// Get the index inside the chunk. + pub fn index(&self) -> usize { self.1 } } -impl From for ItemPosition { - fn from(chunk_identifier: ChunkIdentifier) -> Self { - Self(chunk_identifier, 0) - } -} - /// An iterator over a [`LinkedChunk`] that traverses the chunk in backward /// direction (i.e. it calls `previous` on each chunk to make progress). pub struct LinkedChunkIterBackward<'a, Item, Gap, const CAP: usize> { @@ -740,20 +737,22 @@ impl Chunk { &self.content } - /// Get the [`ItemPosition`] of the first item. - pub fn first_item_position(&self) -> ItemPosition { - ItemPosition(self.identifier(), 0) + /// Get the [`Position`] of the first item if any. + /// + /// If the `Chunk` is a `Gap`, it returns `0` for the index. + pub fn first_position(&self) -> Position { + Position(self.identifier(), 0) } - /// Get the [`ItemPosition`] of the last item. + /// Get the [`Position`] of the last item if any. /// - /// If it's a `Gap` chunk, the last part of the tuple will always be `0`. - pub fn last_item_position(&self) -> ItemPosition { + /// If the `Chunk` is a `Gap`, it returns `0` for the index. + pub fn last_position(&self) -> Position { let identifier = self.identifier(); match &self.content { - ChunkContent::Gap(..) => ItemPosition(identifier, 0), - ChunkContent::Items(items) => ItemPosition(identifier, items.len() - 1), + ChunkContent::Gap(..) => Position(identifier, 0), + ChunkContent::Items(items) => Position(identifier, items.len() - 1), } } @@ -937,8 +936,8 @@ mod tests { use assert_matches::assert_matches; use super::{ - Chunk, ChunkContent, ChunkIdentifier, ChunkIdentifierGenerator, ItemPosition, LinkedChunk, - LinkedChunkError, + Chunk, ChunkContent, ChunkIdentifier, ChunkIdentifierGenerator, LinkedChunk, + LinkedChunkError, Position, }; macro_rules! assert_items_eq { @@ -965,7 +964,7 @@ mod tests { $( assert_matches!( $iterator .next(), - Some((chunk_index, ItemPosition(chunk_identifier, item_index), & $item )) => { + Some((chunk_index, Position(chunk_identifier, item_index), & $item )) => { // Ensure the chunk index (from the enumeration) is correct. assert_eq!(chunk_index, $chunk_index); // Ensure the chunk identifier is the same for all items in this chunk. @@ -1005,7 +1004,7 @@ mod tests { let identifier = chunk.identifier(); Some(items.iter().enumerate().map(move |(item_index, item)| { - (chunk_index, ItemPosition(identifier, item_index), item) + (chunk_index, Position(identifier, item_index), item) })) } }) @@ -1096,7 +1095,7 @@ mod tests { assert_eq!(linked_chunk.chunk_identifier(Chunk::is_gap), Some(ChunkIdentifier(2))); assert_eq!( linked_chunk.item_position(|item| *item == 'e'), - Some(ItemPosition(ChunkIdentifier(1), 1)) + Some(Position(ChunkIdentifier(1), 1)) ); } @@ -1237,11 +1236,11 @@ 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), 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(), Some((Position(ChunkIdentifier(3), 0), 'e'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(2), 1), 'd'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(2), 0), 'c'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(0), 1), 'b'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(0), 0), 'a'))); assert_matches!(iterator.next(), None); } @@ -1254,11 +1253,11 @@ mod tests { let mut iterator = linked_chunk.items(); - 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(), Some((Position(ChunkIdentifier(0), 0), 'a'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(0), 1), 'b'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(2), 0), 'c'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(2), 1), 'd'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(3), 0), 'e'))); assert_matches!(iterator.next(), None); } @@ -1272,9 +1271,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), 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(), Some((Position(ChunkIdentifier(2), 0), 'c'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(0), 1), 'b'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(0), 0), 'a'))); assert_matches!(iterator.next(), None); Ok(()) @@ -1290,9 +1289,9 @@ 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), 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(), Some((Position(ChunkIdentifier(2), 0), 'c'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(2), 1), 'd'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(3), 0), 'e'))); assert_matches!(iterator.next(), None); Ok(()) @@ -1346,7 +1345,7 @@ mod tests { // Insert in a chunk that does not exist. { assert_matches!( - linked_chunk.insert_items_at(['u', 'v'], ItemPosition(ChunkIdentifier(128), 0)), + linked_chunk.insert_items_at(['u', 'v'], Position(ChunkIdentifier(128), 0)), Err(LinkedChunkError::InvalidChunkIdentifier { identifier: ChunkIdentifier(128) }) ); } @@ -1354,7 +1353,7 @@ mod tests { // Insert in a chunk that exists, but at an item that does not exist. { assert_matches!( - linked_chunk.insert_items_at(['u', 'v'], ItemPosition(ChunkIdentifier(0), 128)), + linked_chunk.insert_items_at(['u', 'v'], Position(ChunkIdentifier(0), 128)), Err(LinkedChunkError::InvalidItemIndex { index: 128 }) ); } @@ -1369,7 +1368,7 @@ mod tests { ); assert_matches!( - linked_chunk.insert_items_at(['u', 'v'], ItemPosition(ChunkIdentifier(6), 0)), + linked_chunk.insert_items_at(['u', 'v'], Position(ChunkIdentifier(6), 0)), Err(LinkedChunkError::ChunkIsAGap { identifier: ChunkIdentifier(6) }) ); } @@ -1388,7 +1387,7 @@ mod tests { // Insert in the middle of a chunk. { let position_of_b = linked_chunk.item_position(|item| *item == 'b').unwrap(); - linked_chunk.insert_gap_at(position_of_b, ())?; + linked_chunk.insert_gap_at((), position_of_b)?; assert_items_eq!(linked_chunk, ['a'] [-] ['b', 'c'] ['d', 'e', 'f']); } @@ -1396,7 +1395,7 @@ mod tests { // Insert at the beginning of a chunk. { let position_of_a = linked_chunk.item_position(|item| *item == 'a').unwrap(); - linked_chunk.insert_gap_at(position_of_a, ())?; + linked_chunk.insert_gap_at((), position_of_a)?; assert_items_eq!(linked_chunk, [] [-] ['a'] [-] ['b', 'c'] ['d', 'e', 'f']); } @@ -1404,7 +1403,7 @@ mod tests { // Insert in a chunk that does not exist. { assert_matches!( - linked_chunk.insert_items_at(['u', 'v'], ItemPosition(ChunkIdentifier(128), 0)), + linked_chunk.insert_items_at(['u', 'v'], Position(ChunkIdentifier(128), 0)), Err(LinkedChunkError::InvalidChunkIdentifier { identifier: ChunkIdentifier(128) }) ); } @@ -1412,7 +1411,7 @@ mod tests { // Insert in a chunk that exists, but at an item that does not exist. { assert_matches!( - linked_chunk.insert_items_at(['u', 'v'], ItemPosition(ChunkIdentifier(0), 128)), + linked_chunk.insert_items_at(['u', 'v'], Position(ChunkIdentifier(0), 128)), Err(LinkedChunkError::InvalidItemIndex { index: 128 }) ); } @@ -1421,9 +1420,9 @@ mod tests { { // It is impossible to get the item position inside a gap. It's only possible if // the item position is crafted by hand or is outdated. - let position_of_a_gap = ItemPosition(ChunkIdentifier(4), 0); + let position_of_a_gap = Position(ChunkIdentifier(4), 0); assert_matches!( - linked_chunk.insert_gap_at(position_of_a_gap, ()), + linked_chunk.insert_gap_at((), position_of_a_gap), Err(LinkedChunkError::ChunkIsAGap { identifier: ChunkIdentifier(4) }) ); } @@ -1490,29 +1489,29 @@ mod tests { // First chunk. { let chunk = iterator.next().unwrap(); - assert_eq!(chunk.first_item_position(), ItemPosition(ChunkIdentifier(0), 0)); - assert_eq!(chunk.last_item_position(), ItemPosition(ChunkIdentifier(0), 2)); + assert_eq!(chunk.first_position(), Position(ChunkIdentifier(0), 0)); + assert_eq!(chunk.last_position(), Position(ChunkIdentifier(0), 2)); } // Second chunk. { let chunk = iterator.next().unwrap(); - assert_eq!(chunk.first_item_position(), ItemPosition(ChunkIdentifier(1), 0)); - assert_eq!(chunk.last_item_position(), ItemPosition(ChunkIdentifier(1), 1)); + assert_eq!(chunk.first_position(), Position(ChunkIdentifier(1), 0)); + assert_eq!(chunk.last_position(), Position(ChunkIdentifier(1), 1)); } // Gap. { let chunk = iterator.next().unwrap(); - assert_eq!(chunk.first_item_position(), ItemPosition(ChunkIdentifier(2), 0)); - assert_eq!(chunk.last_item_position(), ItemPosition(ChunkIdentifier(2), 0)); + assert_eq!(chunk.first_position(), Position(ChunkIdentifier(2), 0)); + assert_eq!(chunk.last_position(), Position(ChunkIdentifier(2), 0)); } // Last chunk. { let chunk = iterator.next().unwrap(); - assert_eq!(chunk.first_item_position(), ItemPosition(ChunkIdentifier(3), 0)); - assert_eq!(chunk.last_item_position(), ItemPosition(ChunkIdentifier(3), 0)); + assert_eq!(chunk.first_position(), Position(ChunkIdentifier(3), 0)); + assert_eq!(chunk.last_position(), Position(ChunkIdentifier(3), 0)); } } } diff --git a/crates/matrix-sdk/src/event_cache/store.rs b/crates/matrix-sdk/src/event_cache/store.rs index eede3c3c4d3..f5d12ae97e8 100644 --- a/crates/matrix-sdk/src/event_cache/store.rs +++ b/crates/matrix-sdk/src/event_cache/store.rs @@ -21,8 +21,8 @@ use tokio::sync::RwLock; use super::{ linked_chunk::{ - Chunk, ChunkIdentifier, ItemPosition, LinkedChunk, LinkedChunkError, LinkedChunkIter, - LinkedChunkIterBackward, + Chunk, ChunkIdentifier, LinkedChunk, LinkedChunkError, LinkedChunkIter, + LinkedChunkIterBackward, Position, }, Result, }; @@ -255,7 +255,7 @@ impl RoomEvents { pub fn insert_events_at( &mut self, events: I, - position: ItemPosition, + position: Position, ) -> StdResult<(), LinkedChunkError> where I: IntoIterator, @@ -267,10 +267,10 @@ impl RoomEvents { /// Insert a gap at a specified position. pub fn insert_gap_at( &mut self, - position: ItemPosition, gap: Gap, + position: Position, ) -> StdResult<(), LinkedChunkError> { - self.chunks.insert_gap_at(position, gap) + self.chunks.insert_gap_at(gap, position) } /// Search for a chunk, and return its identifier. @@ -282,7 +282,7 @@ impl RoomEvents { } /// Search for an item, and return its position. - pub fn event_position<'a, P>(&'a self, predicate: P) -> Option + pub fn event_position<'a, P>(&'a self, predicate: P) -> Option where P: FnMut(&'a SyncTimelineEvent) -> bool, { @@ -324,15 +324,15 @@ impl RoomEvents { /// Iterate over the events, backward. /// /// The most recent event comes first. - pub fn revents(&self) -> impl Iterator { + pub fn revents(&self) -> impl Iterator { self.chunks.ritems() } /// Iterate over the events, starting from `position`, backward. pub fn revents_from( &self, - position: ItemPosition, - ) -> StdResult, LinkedChunkError> { + position: Position, + ) -> StdResult, LinkedChunkError> { self.chunks.ritems_from(position) } @@ -340,8 +340,8 @@ impl RoomEvents { /// to the latest event. pub fn events_from( &self, - position: ItemPosition, - ) -> StdResult, LinkedChunkError> { + position: Position, + ) -> StdResult, LinkedChunkError> { self.chunks.items_from(position) } }