Commit 05ef287d1bfc
tarball8 files changed+238-54
@@ -66,6 +66,6 @@ pub enum HistoryGapStatus {
6666
},6767
#[doc = "Accessible history ended without reaching the retained boundary."]6868
Unavailable,69
- #[doc = "Pagination reached the retained boundary. The identity remains valid for stale callers."]69
+ #[doc = "Pagination reached the retained boundary. Only reported, a joined gap is removed."]7070
Joined,7171
}@@ -155,6 +155,11 @@ impl EventLinkedChunk {
155155
Ok(())156156
}157157
158
+ pub(super) fn remove_gap(&mut self, gap_identifier: ChunkIdentifier) {159
+ self.replace_gap_at(gap_identifier, Vec::new())160
+ .expect("gap_identifier is a valid gap chunk id we read previously");161
+ }162
+158163
/// Remove some events from the linked chunk.159164
///160165
/// If a chunk becomes empty, it's going to be removed.@@ -320,9 +325,19 @@ impl EventLinkedChunk {
320325
321326
/// Add the previous back-pagination token (if present), followed by the322327
/// timeline events themselves.328
+ ///329
+ /// A new gap supersedes a trailing one, since it paginates over the same330
+ /// interval from further ahead.323331
pub fn push_live_events(&mut self, new_gap: Option<Gap>, events: &[Event]) {324332
if let Some(new_gap) = new_gap {325333
self.push_gap(new_gap);334
+ let superseded = self335
+ .rchunks()336
+ .nth(1)337
+ .and_then(|chunk| chunk.is_gap().then_some(chunk.identifier()));338
+ if let Some(superseded) = superseded {339
+ self.remove_gap(superseded);340
+ }326341
}327342
self.chunks.push_items_back(events.iter().cloned());328343
}@@ -3,8 +3,8 @@ Ordinary oldest-history pagination and other internal gaps keep their cursors.
33
44
Only reaching the recorded older boundary verifies a join. Short or empty pages55
with advancing cursors leave the gap idle. A repeated cursor pauses it. Exhausting6
-accessible history leaves it unavailable and preserves retained events. Joined7
-identities remain persisted so repeated calls can return their terminal outcome.6
+accessible history leaves it unavailable and preserves retained events. A join7
+removes the gap, so later calls with its identity return `NotFound`.88
99
Dropping the future before the response cancels the request without advancing1010
the cursor. Once installation starts, the page and cursor commit together even@@ -52,14 +52,14 @@ pub(super) fn gap_positions(candidate: &EventLinkedChunk) -> Vec<HistoryGapPosit
5252
#[derive(Clone, Debug, PartialEq, Eq)]5353
#[doc = "A bounded gap fill never falls back to another gap when its target disappears."]5454
pub enum HistoryGapFillOutcome {55
- #[doc = "Also returned without a network request for a joined or unavailable interval."]55
+ #[doc = "Also returned without a network request for an unavailable interval."]5656
Updated {5757
#[doc = "The durable outcome, including its original boundary identities."]5858
gap: HistoryGap,5959
#[doc = "Newly inserted events after room-wide deduplication."]6060
events: usize,6161
},62
- #[doc = "The identity is unknown or was removed by an explicit cache replacement."]62
+ #[doc = "The identity is unknown, was joined, or was removed by a clear or a newer gap."]6363
NotFound,6464
#[doc = "Another history operation owns the room coordinator. No request was started."]6565
Busy,@@ -72,8 +72,19 @@ pub struct RecentHistoryOutcome {
7272
pub pages: u32,7373
#[doc = "Number of raw events returned by the server."]7474
pub events: usize,75
- #[doc = "The durable interval, absent when no retained boundary needs joining."]76
- pub gap: Option<HistoryGap>,75
+ #[doc = "How the window relates to the retained history before it."]76
+ pub continuity: RecentHistoryContinuity,77
+}78
+79
+#[derive(Clone, Debug, PartialEq, Eq)]80
+#[doc = "Only a remaining interval is persisted, a join leaves no gap behind."]81
+pub enum RecentHistoryContinuity {82
+ #[doc = "No retained event preceded the window."]83
+ Fresh,84
+ #[doc = "The window, or sync arrivals during its request, reached the newest retained event."]85
+ Joined,86
+ #[doc = "The durable interval between the retained history and the window."]87
+ Gap(HistoryGap),7788
}7889
7990
pub(super) struct HistoryRequest {@@ -267,42 +278,57 @@ impl RoomEventCache {
267278
let mut state = self.inner.state.write().await?;268279
let mut candidate = state.history_candidate().await?;269280
let baseline_count = candidate.events().count();270
- let fallback = {271
- let mut after_anchor = anchor.is_none();272
- candidate.events().find_map(|(_, event)| {273
- let id = event.event_id();274
- if after_anchor {275
- return id;281
+ let mut known = HashSet::new();282
+ let mut arrivals = Vec::new();283
+ let mut leading = Vec::new();284
+ let mut passed_anchor = anchor.is_none();285
+ for chunk in candidate.chunks() {286
+ match chunk.content() {287
+ ChunkContent::Gap(gap) if passed_anchor && arrivals.is_empty() => {288
+ leading.push((chunk.identifier(), gap.clone()));276289
}277
- if id == anchor {278
- after_anchor = true;290
+ ChunkContent::Gap(_) => {}291
+ ChunkContent::Items(items) => {292
+ for id in items.iter().filter_map(Event::event_id) {293
+ if passed_anchor {294
+ arrivals.push(id.clone());295
+ }296
+ passed_anchor |= Some(&id) == anchor.as_ref();297
+ known.insert(id);298
+ }279299
}280
- None281
- })282
- };283
- let newer_boundary = events.iter().find_map(Event::event_id).or(fallback.clone());284
- let history = anchor.map(|anchor| HistoryGap {285
- id: TransactionId::new(),286
- older_boundary: Some(anchor),287
- newer_boundary: newer_boundary.clone(),288
- status: if joined {289
- HistoryGapStatus::Joined290
- } else if cursor.is_none() {291
- HistoryGapStatus::Unavailable292
- } else if stalled {293
- HistoryGapStatus::Paused294
- } else {295
- HistoryGapStatus::Idle296
- },297
- visited_cursors: visited,300
+ }301
+ }302
+ let anchor = anchor.filter(|_| passed_anchor);303
+ if anchor.is_none() {304
+ joined = false;305
+ }306
+ let first_arrival = arrivals.first().cloned();307
+ let arrivals: HashSet<_> = arrivals.into_iter().collect();308
+ let fetched = events.iter().any(|event| event.event_id().is_some());309
+ let overlap = events310
+ .iter()311
+ .any(|event| event.event_id().is_some_and(|id| arrivals.contains(&id)));312
+ let newest_fetched = events.iter().rev().find_map(Event::event_id);313
+ let oldest_fetched = events.iter().find_map(Event::event_id);314
+315
+ // A gap that sync installed during the request survives only when the316
+ // window ended before the sync arrivals, and then it belongs between them.317
+ let mut kept = leading.pop_if(|(_, gap)| {318
+ first_arrival.is_some()319
+ && !overlap320
+ && gap.history.as_ref().is_none_or(|h| h.status != HistoryGapStatus::Joined)298321
});299
- let mut before = fallback;322
+ let moved = kept.take_if(|_| fetched);323
+ for (chunk, _) in leading.iter().chain(&moved) {324
+ candidate.remove_gap(*chunk);325
+ }326
+300327
let mut inserted = Vec::new();328
+ let mut before = first_arrival.clone();301329
for event in events.into_iter().rev() {302330
let id = event.event_id();303
- if id.as_ref().is_none_or(|id| {304
- !candidate.events().any(|(_, event)| event.event_id().as_ref() == Some(id))305
- }) {331
+ if id.as_ref().is_none_or(|id| known.insert(id.clone())) {306332
candidate.insert_history_before(before.as_deref(), vec![event.clone()], None);307333
inserted.push(event);308334
}@@ -310,16 +336,47 @@ impl RoomEventCache {
310336
before = id;311337
}312338
}313
- if history.is_some() || cursor.is_some() {339
+340
+ let history = anchor.as_ref().filter(|_| !joined).map(|anchor| HistoryGap {341
+ id: TransactionId::new(),342
+ older_boundary: Some(anchor.clone()),343
+ newer_boundary: oldest_fetched.or(first_arrival.clone()),344
+ status: if cursor.is_none() {345
+ HistoryGapStatus::Unavailable346
+ } else if stalled {347
+ HistoryGapStatus::Paused348
+ } else {349
+ HistoryGapStatus::Idle350
+ },351
+ visited_cursors: visited,352
+ });353
+ let history = history.filter(|_| fetched || first_arrival.is_none());354
+ if history.is_some() || cursor.is_some() && (fetched || first_arrival.is_none()) {314355
candidate.insert_history_before(315356
before.as_deref(),316357
Vec::new(),317358
Some(Gap { token: cursor.unwrap_or_default(), history: history.clone() }),318359
);319360
}361
+ if let Some((_, mut gap)) = moved {362
+ if let Some(history) = gap.history.as_mut() {363
+ history.older_boundary = newest_fetched;364
+ }365
+ candidate.insert_history_before(first_arrival.as_deref(), Vec::new(), Some(gap));366
+ }367
+368
+ let continuity = match (anchor, history, kept) {369
+ (None, _, _) => RecentHistoryContinuity::Fresh,370
+ (Some(_), Some(history), _) => RecentHistoryContinuity::Gap(history),371
+ (Some(_), None, Some((_, Gap { history: Some(history), .. }))) => {372
+ RecentHistoryContinuity::Gap(history)373
+ }374
+ (Some(_), None, _) => RecentHistoryContinuity::Joined,375
+ };376
+320377
inserted.reverse();321378
state.commit_history_candidate(candidate, inserted, baseline_count).await?;322
- Ok(RecentHistoryOutcome { pages, events: raw_count, gap: history })379
+ Ok(RecentHistoryOutcome { pages, events: raw_count, continuity })323380
}324381
}325382
@@ -546,6 +603,7 @@ fn apply_page(
546603
events.retain(|event| event.event_id().is_none_or(|id| known.insert(id)));547604
events.reverse();548605
let history = gap.history.as_ref().expect("history metadata was preserved").clone();549
- candidate.push_backwards_pagination_events(Some(chunk_id), Some(gap), &events);606
+ let gap = (history.status != HistoryGapStatus::Joined).then_some(gap);607
+ candidate.push_backwards_pagination_events(Some(chunk_id), gap, &events);550608
(history, events)551609
}@@ -14,7 +14,9 @@
1414
1515
mod history;1616
pub mod pagination;17
-pub use history::{HistoryGapFillOutcome, HistoryGapPosition, RecentHistoryOutcome};17
+pub use history::{18
+ HistoryGapFillOutcome, HistoryGapPosition, RecentHistoryContinuity, RecentHistoryOutcome,19
+};1820
mod state;1921
mod subscriber;2022
mod updates;@@ -70,8 +70,9 @@ pub use caches::{
7070
event_focused::EventFocusThreadMode,7171
pagination::{BackPaginationOutcome, PaginationStatus},7272
room::{73
- HistoryGapFillOutcome, HistoryGapPosition, RecentHistoryOutcome, RoomEventCache, RoomEventCacheGenericUpdate,74
- RoomEventCacheSubscriber, RoomEventCacheUpdate, pagination::RoomPagination,73
+ HistoryGapFillOutcome, HistoryGapPosition, RecentHistoryContinuity, RecentHistoryOutcome,74
+ RoomEventCache, RoomEventCacheGenericUpdate, RoomEventCacheSubscriber,75
+ RoomEventCacheUpdate, pagination::RoomPagination,7576
},7677
thread::pagination::ThreadPagination,7778
};@@ -1,7 +1,9 @@
11
use std::time::Duration;22
33
use matrix_sdk::{4
- event_cache::HistoryGapStatus,4
+ event_cache::{5
+ HistoryGap, HistoryGapStatus, RecentHistoryContinuity, RecentHistoryOutcome, RoomEventCache,6
+ },57
test_utils::mocks::{MatrixMockServer, RoomMessagesResponseTemplate},68
};79
use matrix_sdk_test::{JoinedRoomBuilder, async_test, event_factory::EventFactory};@@ -11,6 +13,113 @@ fn event_factory(room_id: &ruma::RoomId) -> EventFactory {
1113
EventFactory::new().room(room_id).sender(user_id!("@alice:example.org"))1214
}1315
16
+fn gap(outcome: &RecentHistoryOutcome) -> &HistoryGap {17
+ let RecentHistoryContinuity::Gap(gap) = &outcome.continuity else {18
+ panic!("expected a remaining interval, got {:?}", outcome.continuity)19
+ };20
+ gap21
+}22
+23
+async fn load_with_limited_sync_arrival(24
+ server: &MatrixMockServer,25
+ client: &matrix_sdk::Client,26
+ cache: &RoomEventCache,27
+ f: &EventFactory,28
+) -> RecentHistoryOutcome {29
+ let room_id = cache.room_id().to_owned();30
+ let mut load = Box::pin(cache.load_recent_history());31
+ tokio::select! {32
+ biased;33
+ result = &mut load => panic!("request finished before sync {result:?}"),34
+ _ = tokio::time::sleep(Duration::from_millis(20)) => {}35
+ }36
+ server37
+ .sync_room(38
+ client,39
+ JoinedRoomBuilder::new(&room_id)40
+ .add_timeline_event(f.text_msg("live").event_id(event_id!("$live")))41
+ .set_timeline_limited()42
+ .set_timeline_prev_batch("sync-edge"),43
+ )44
+ .await;45
+ load.await.unwrap()46
+}47
+48
+#[async_test]49
+async fn test_recent_history_overlapping_a_limited_sync_drops_its_stale_gap() {50
+ let server = MatrixMockServer::new().await;51
+ let client = server.client_builder().build().await;52
+ client.event_cache().subscribe().unwrap();53
+ let room_id = room_id!("!history:example.org");54
+ let room = server.sync_joined_room(&client, room_id).await;55
+ let (cache, _handles) = room.event_cache().await.unwrap();56
+ let f = event_factory(room_id);57
+ cache58
+ .reseed(vec![f.text_msg("old").event_id(event_id!("$old")).into_event()], None)59
+ .await60
+ .unwrap();61
+ server62
+ .mock_room_messages()63
+ .match_limit(50)64
+ .ok(RoomMessagesResponseTemplate::default()65
+ .events(vec![66
+ f.text_msg("live").event_id(event_id!("$live")),67
+ f.text_msg("recent").event_id(event_id!("$recent")),68
+ ])69
+ .with_delay(Duration::from_millis(100)))70
+ .mock_once()71
+ .mount()72
+ .await;73
+74
+ let outcome = load_with_limited_sync_arrival(&server, &client, &cache, &f).await;75
+76
+ let (events, gaps) = cache.history_snapshot().await.unwrap();77
+ let ids: Vec<_> = events.iter().filter_map(|event| event.event_id()).collect();78
+ assert_eq!(ids, vec![event_id!("$old"), event_id!("$recent"), event_id!("$live")]);79
+ assert_eq!(gaps.len(), 1);80
+ assert_eq!(&gaps[0].gap, gap(&outcome));81
+ assert_eq!(gaps[0].gap.older_boundary.as_deref(), Some(event_id!("$old")));82
+ assert_eq!(gaps[0].next_event_id.as_deref(), Some(event_id!("$recent")));83
+}84
+85
+#[async_test]86
+async fn test_recent_history_ending_before_a_limited_sync_keeps_its_gap_after_the_window() {87
+ let server = MatrixMockServer::new().await;88
+ let client = server.client_builder().build().await;89
+ client.event_cache().subscribe().unwrap();90
+ let room_id = room_id!("!history:example.org");91
+ let room = server.sync_joined_room(&client, room_id).await;92
+ let (cache, _handles) = room.event_cache().await.unwrap();93
+ let f = event_factory(room_id);94
+ cache95
+ .reseed(vec![f.text_msg("old").event_id(event_id!("$old")).into_event()], None)96
+ .await97
+ .unwrap();98
+ server99
+ .mock_room_messages()100
+ .match_limit(50)101
+ .ok(RoomMessagesResponseTemplate::default()102
+ .events(vec![103
+ f.text_msg("recent").event_id(event_id!("$recent")),104
+ f.text_msg("old").event_id(event_id!("$old")),105
+ ])106
+ .with_delay(Duration::from_millis(100)))107
+ .mock_once()108
+ .mount()109
+ .await;110
+111
+ let outcome = load_with_limited_sync_arrival(&server, &client, &cache, &f).await;112
+ assert_eq!(outcome.continuity, RecentHistoryContinuity::Joined);113
+114
+ let (events, gaps) = cache.history_snapshot().await.unwrap();115
+ let ids: Vec<_> = events.iter().filter_map(|event| event.event_id()).collect();116
+ assert_eq!(ids, vec![event_id!("$old"), event_id!("$recent"), event_id!("$live")]);117
+ assert_eq!(gaps.len(), 1);118
+ assert_eq!(gaps[0].gap.older_boundary.as_deref(), Some(event_id!("$recent")));119
+ assert_eq!(gaps[0].gap.newer_boundary.as_deref(), Some(event_id!("$live")));120
+ assert_eq!(gaps[0].next_event_id.as_deref(), Some(event_id!("$live")));121
+}122
+14123
#[async_test]15124
async fn test_recent_history_is_bounded_and_keeps_the_retained_window() {16125
let server = MatrixMockServer::new().await;@@ -68,7 +177,7 @@ async fn test_recent_history_is_bounded_and_keeps_the_retained_window() {
68177
let outcome = cache.load_recent_history().await.unwrap();69178
assert_eq!(outcome.pages, 4);70179
assert_eq!(outcome.events, 4);71
- assert_eq!(outcome.gap.as_ref().unwrap().status, HistoryGapStatus::Idle);180
+ assert_eq!(gap(&outcome).status, HistoryGapStatus::Idle);72181
let ids: Vec<_> =73182
cache.events().await.unwrap().iter().filter_map(|event| event.event_id()).collect();74183
assert_eq!(@@ -110,7 +219,8 @@ async fn test_recent_history_joins_at_the_retained_boundary() {
110219
111220
let outcome = cache.load_recent_history().await.unwrap();112221
assert_eq!(outcome.pages, 1);113
- assert_eq!(outcome.gap.as_ref().unwrap().status, HistoryGapStatus::Joined);222
+ assert_eq!(outcome.continuity, RecentHistoryContinuity::Joined);223
+ assert!(cache.history_gaps().await.unwrap().is_empty());114224
assert_eq!(115225
cache116226
.events()@@ -211,7 +321,7 @@ async fn test_recent_history_continues_empty_pages_and_pauses_on_a_cursor_cycle(
211321
let outcome = cache.load_recent_history().await.unwrap();212322
assert_eq!(outcome.pages, 3);213323
assert_eq!(outcome.events, 1);214
- assert_eq!(outcome.gap.as_ref().unwrap().status, HistoryGapStatus::Paused);324
+ assert_eq!(gap(&outcome).status, HistoryGapStatus::Paused);215325
assert_eq!(216326
cache217327
.events()@@ -251,7 +361,7 @@ async fn test_concurrent_recent_history_calls_share_the_request_and_gap() {
251361
let (first, second) = tokio::join!(cache.load_recent_history(), cache.load_recent_history());252362
let first = first.unwrap();253363
let second = second.unwrap();254
- assert_eq!(first.gap.as_ref().unwrap().id, second.gap.as_ref().unwrap().id);364
+ assert_eq!(gap(&first).id, gap(&second).id);255365
assert_eq!(first.pages, 1);256366
assert_eq!(second.pages, 1);257367
assert_eq!(first.events, 1);@@ -294,7 +404,7 @@ async fn test_cancelling_one_recent_history_waiter_keeps_the_shared_request_aliv
294404
drop(first);295405
let outcome = second.await.unwrap().unwrap();296406
assert_eq!(outcome.pages, 1);297
- assert_eq!(outcome.gap.unwrap().status, HistoryGapStatus::Unavailable);407
+ assert_eq!(gap(&outcome).status, HistoryGapStatus::Unavailable);298408
assert_eq!(299409
cache300410
.events()@@ -361,7 +471,7 @@ async fn test_cancelling_all_recent_history_waiters_allows_a_fresh_retry() {
361471
362472
let outcome = cache.load_recent_history().await.unwrap();363473
assert_eq!(outcome.pages, 1);364
- assert_eq!(outcome.gap.unwrap().status, HistoryGapStatus::Unavailable);474
+ assert_eq!(gap(&outcome).status, HistoryGapStatus::Unavailable);365475
assert_eq!(366476
cache367477
.events()@@ -113,10 +113,8 @@ async fn test_segmented_history_resumes_after_restart_and_joins_only_at_boundary
113113
};114114
assert_eq!(gap.status, HistoryGapStatus::Joined);115115
assert_eq!(events, 0);116
- assert_eq!(117
- cache.fill_history_gap_once(gap_id).await.unwrap(),118
- HistoryGapFillOutcome::Updated { gap, events: 0 }119
- );116
+ assert!(cache.history_gaps().await.unwrap().is_empty());117
+ assert_eq!(cache.fill_history_gap_once(gap_id).await.unwrap(), HistoryGapFillOutcome::NotFound);120118
let ids: Vec<_> =121119
cache.events().await.unwrap().iter().filter_map(|event| event.event_id()).collect();122120
assert_eq!(ids, vec![event_id!("$old"), event_id!("$middle"), event_id!("$new")]);