diff --git a/crates/matrix-sdk/src/sliding_sync.rs b/crates/matrix-sdk/src/sliding_sync.rs index 48be28a11..d563e96d6 100644 --- a/crates/matrix-sdk/src/sliding_sync.rs +++ b/crates/matrix-sdk/src/sliding_sync.rs @@ -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]; diff --git a/testing/sliding-sync-integration-test/src/lib.rs b/testing/sliding-sync-integration-test/src/lib.rs index 48f990077..8238ffbea 100644 --- a/testing/sliding-sync-integration-test/src/lib.rs +++ b/testing/sliding-sync-integration-test/src/lib.rs @@ -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()?;