chore(sdk): Address PR feedback.

This commit is contained in:
Ivan Enderlin
2023-02-16 11:43:25 +01:00
parent b05d6874c0
commit 46006cf108
2 changed files with 38 additions and 32 deletions

View File

@@ -336,19 +336,20 @@ impl SlidingSyncRoom {
// There is timeline updates.
if !timeline_updates.is_empty() {
// If we come from a cold storage, we overwrite the timeline queue with the
// timeline updates.
if self.is_cold.load(Ordering::SeqCst) {
// If we come from a cold storage, we overwrite the timeline queue with the
// timeline updates.
self.timeline_queue.lock_mut().replace_cloned(timeline_updates);
self.is_cold.store(false, Ordering::SeqCst);
}
// The server alerted us that we missed items in between.
else if *limited {
} else if *limited {
// The server alerted us that we missed items in between.
self.timeline_queue.lock_mut().replace_cloned(timeline_updates);
}
// It's the hot path. We have new updates that must be added to the existing timeline
// queue.
else {
} else {
// It's the hot path. We have new updates that must be added to the existing
// timeline queue.
let mut timeline_queue = self.timeline_queue.lock_mut();
// If the `timeline_queue` contains:
@@ -384,8 +385,7 @@ impl SlidingSyncRoom {
.iter()
.rev()
.zip(timeline_updates.iter().rev())
.map(|(queue, update)| (queue.event_id(), update.event_id()))
.position(|(queue, update)| queue != update)
.position(|(queue, update)| queue.event_id() != update.event_id())
{
// We have found a suffix that equals the size of `timeline_queue` or
// `timeline_update`, typically:
@@ -394,26 +394,32 @@ impl SlidingSyncRoom {
// or
// timeline_queue = [A, B, C, D, E, F]
// timeline_update = [D, E, F]
// in both case, `position` will return `None` because we are looking (from
// the end) an item that is different.
// in both case, `position` will return `None` because we are looking for
// (from the end) an item that is different.
None => std::cmp::min(timeline_queue_len, timeline_updates_len),
// We have found a suffix but it doesn't cover all `timeline_queue` or
// `timeline_update`, typically:
// We may have found a suffix.
//
// If we have `Some(0)`, it means we don't have found a suffix. That's the
// hot path, `timeline_updates` will just be appended to `timeline_queue`.
//
// If we have `Some(n)` with `n > 0`, it means we have a prefix but it
// doesn't cover all `timeline_queue` or `timeline_update`, typically:
// timeline_queue = [B, D, E, F]
// timeline_update = [A, B, C, D, E, F]
// in this case, `position` will return `Some(3)` instead of `None`.
// in this case, `position` will return `Some(3)`.
// That's annoying because it means we have an invalid `timeline_queue` or
// `timeline_update`.
// `timeline_update`, but let's try to do our best.
Some(position) => position,
};
// No prefix found.
if position == 0 {
// No prefix found.
timeline_queue.extend(timeline_updates);
}
// One prefix found
else {
} else {
// Prefix found.
let new_timeline_updates =
&timeline_updates[..timeline_updates_len - position];

View File

@@ -335,8 +335,8 @@ mod tests {
let build_view = |name| {
SlidingSyncViewBuilder::default()
.sync_mode(SlidingSyncMode::Selective)
.add_range(0u32, 10u32)
.sort(vec!["by_recency".to_string(), "by_name".to_string()])
.set_range(0u32, 10u32)
.sort(vec!["by_recency".to_owned(), "by_name".to_owned()])
.name(name)
.build()
};
@@ -429,7 +429,7 @@ mod tests {
SlidingSyncViewBuilder::default()
.sync_mode(SlidingSyncMode::Selective)
.set_range(0u32, 10u32)
.sort(vec!["by_recency".to_string(), "by_name".to_string()])
.sort(vec!["by_recency".to_owned(), "by_name".to_owned()])
.name(name)
.build()
};
@@ -552,14 +552,14 @@ mod tests {
let sliding_window_view = SlidingSyncViewBuilder::default()
.sync_mode(SlidingSyncMode::Selective)
.set_range(0u32, 10u32)
.sort(vec!["by_recency".to_string(), "by_name".to_string()])
.sort(vec!["by_recency".to_owned(), "by_name".to_owned()])
.name("sliding")
.build()?;
let full = SlidingSyncViewBuilder::default()
.sync_mode(SlidingSyncMode::GrowingFullSync)
.batch_size(10u32)
.sort(vec!["by_recency".to_string(), "by_name".to_string()])
.sort(vec!["by_recency".to_owned(), "by_name".to_owned()])
.name("full")
.build()?;
let sync_proxy =
@@ -728,7 +728,7 @@ mod tests {
let sliding_window_view = SlidingSyncViewBuilder::default()
.sync_mode(SlidingSyncMode::Selective)
.set_range(1u32, 10u32)
.sort(vec!["by_recency".to_string(), "by_name".to_string()])
.sort(vec!["by_recency".to_owned(), "by_name".to_owned()])
.name("sliding")
.build()?;
let sync_proxy = sync_proxy_builder.add_view(sliding_window_view).build().await?;
@@ -904,13 +904,13 @@ mod tests {
let sliding_window_view = SlidingSyncViewBuilder::default()
.sync_mode(SlidingSyncMode::Selective)
.set_range(1u32, 10u32)
.sort(vec!["by_recency".to_string(), "by_name".to_string()])
.sort(vec!["by_recency".to_owned(), "by_name".to_owned()])
.name("sliding")
.build()?;
let growing_sync = SlidingSyncViewBuilder::default()
.sync_mode(SlidingSyncMode::GrowingFullSync)
.limit(100)
.sort(vec!["by_recency".to_string(), "by_name".to_string()])
.sort(vec!["by_recency".to_owned(), "by_name".to_owned()])
.name("growing")
.build()?;
anyhow::Ok((sliding_window_view, growing_sync))
@@ -968,7 +968,7 @@ mod tests {
let growing_sync = SlidingSyncViewBuilder::default()
.sync_mode(SlidingSyncMode::GrowingFullSync)
.batch_size(10u32)
.sort(vec!["by_recency".to_string(), "by_name".to_string()])
.sort(vec!["by_recency".to_owned(), "by_name".to_owned()])
.name("growing")
.build()?;
@@ -1096,7 +1096,7 @@ mod tests {
let growing_sync = SlidingSyncViewBuilder::default()
.sync_mode(SlidingSyncMode::GrowingFullSync)
.limit(100)
.sort(vec!["by_recency".to_string(), "by_name".to_string()])
.sort(vec!["by_recency".to_owned(), "by_name".to_owned()])
.name("growing")
.build()?;
@@ -1186,7 +1186,7 @@ mod tests {
let growing_sync = SlidingSyncViewBuilder::default()
.sync_mode(SlidingSyncMode::GrowingFullSync)
.limit(100)
.sort(vec!["by_recency".to_string(), "by_name".to_string()])
.sort(vec!["by_recency".to_owned(), "by_name".to_owned()])
.name("growing")
.build()?;