Commit ea600a4bd56a
tarball4 files changed+130-19
@@ -461,7 +461,9 @@ impl RoomEventCache {
461461
history.status = HistoryGapStatus::BudgetSpent;462462
let history = history.clone();463463
candidate.push_backwards_pagination_events(Some(chunk), Some(gap), &[]);464
- state.commit_history_candidate(candidate, Vec::new(), HistoryCommitScope::Loaded).await?;464
+ state465
+ .commit_history_candidate(candidate, Vec::new(), HistoryCommitScope::Loaded)466
+ .await?;465467
return Ok(HistoryGapFillOutcome::Updated { gap: history, events: 0 });466468
}467469
Ok(HistoryGapFillOutcome::Updated { gap: history.clone(), events: 0 })@@ -685,9 +687,8 @@ fn apply_page(
685687
.flatten()686688
.chain(history.older_boundary.clone())687689
.collect();688
- let overlap = events689
- .iter()690
- .position(|event| event.event_id().is_some_and(|id| below.contains(&id)));690
+ let overlap =691
+ events.iter().position(|event| event.event_id().is_some_and(|id| below.contains(&id)));691692
history.status = if let Some(message) = failure {692693
HistoryGapStatus::Failed { message }693694
} else if let Some(index) = overlap {@@ -201,20 +201,20 @@ impl PaginatedCache for Arc<RoomEventCacheInner> {
201201
202202
// a sync that drops its prev_batch (not limited, or every event a duplicate)203203
// leaves retained events with no gap before them, so only the creation event204
- // proves the start. otherwise paginate from the end, which dedups what we hold205
- let created = state.room_linked_chunk().events().next().is_some_and(|(_, event)| {206
- event.raw().get_field::<String>("type").ok().flatten().as_deref()207
- == Some("m.room.create")208
- });209
- if created {204
+ // proves the start. otherwise the network side asks /context for a token just205
+ // before the oldest event, and there is no sync token worth waiting for206
+ let oldest = state.room_linked_chunk().events().next().map(|(_, event)| event);207
+ if oldest.is_some_and(is_creation) {210208
trace!("chunk is fully loaded and starts at creation: reached_start=true");211209
return Ok(LoadMoreEventsBackwardsOutcome::StartOfTimeline);212210
}211
+ let waited_for_initial_prev_token =212
+ oldest.is_some() || state.waited_for_initial_prev_token();213213
214214
// Otherwise, start back-pagination from the end of the room.215215
return Ok(LoadMoreEventsBackwardsOutcome::Gap {216216
prev_token: None,217
- waited_for_initial_prev_token: state.waited_for_initial_prev_token(),217
+ waited_for_initial_prev_token,218218
});219219
}220220
@@ -301,7 +301,39 @@ impl PaginatedCache for Arc<RoomEventCacheInner> {
301301
return Ok(None);302302
};303303
304
- let mut options = MessagesOptions::new(Direction::Backward).from(prev_token.as_deref());304
+ let from = match prev_token {305
+ Some(token) => Some(token.clone()),306
+ None => {307
+ let oldest = self308
+ .state309
+ .read()310
+ .await?311
+ .room_linked_chunk()312
+ .events()313
+ .next()314
+ .and_then(|(_, event)| event.event_id());315
+ match oldest {316
+ // a page from the live end would land in front of the oldest retained event317
+ // while belonging after it, so start right before that event instead318
+ Some(event_id) => {319
+ let mut request = ruma::api::client::context::get_context::v3::Request::new(320
+ self.room_id.clone(),321
+ event_id,322
+ );323
+ request.limit = ruma::uint!(0);324
+ let response = room.client().send(request).await.map_err(|err| {325
+ EventCacheError::PaginationError(Arc::new(err.into()))326
+ })?;327
+ match response.start {328
+ Some(start) => Some(start),329
+ None => return Ok(Some((Vec::new(), None))),330
+ }331
+ }332
+ None => None,333
+ }334
+ }335
+ };336
+ let mut options = MessagesOptions::new(Direction::Backward).from(from.as_deref());305337
options.limit = batch_size.into();306338
307339
let response = room@@ -355,6 +387,7 @@ impl PaginatedCache for Arc<RoomEventCacheInner> {
355387
}356388
357389
let from_end = prev_token.is_none();390
+ let server_ended = new_token.is_none();358391
// Check that the previous token still exists; otherwise it's a sign that the359392
// room's timeline has been cleared.360393
let prev_gap_id = if let Some(token) = prev_token {@@ -428,11 +461,18 @@ impl PaginatedCache for Arc<RoomEventCacheInner> {
428461
let topo_ordered_events = events.iter().rev().cloned().collect::<Vec<_>>();429462
430463
let new_gap = new_token.map(|prev_token| Gap { token: prev_token, history: None });464
+ // no gap left only proves the start when the server ran out or the room's creation is465
+ // held. a token whose page we already held is dropped above without either431466
let reached_start = state.room_linked_chunk_mut().push_backwards_pagination_events(432467
prev_gap_id,433468
new_gap,434469
&topo_ordered_events,435
- );470
+ ) && (server_ended471
+ || state472
+ .room_linked_chunk()473
+ .events()474
+ .next()475
+ .is_some_and(|(_, event)| is_creation(event)));436476
437477
// A back-pagination can't include new read receipt events, as those are438478
// ephemeral events not included in /messages responses, so we can@@ -468,3 +508,7 @@ impl PaginatedCache for Arc<RoomEventCacheInner> {
468508
Ok(Some(BackPaginationOutcome { events, reached_start }))469509
}470510
}511
+512
+fn is_creation(event: &Event) -> bool {513
+ event.raw().get_field::<String>("type").ok().flatten().as_deref() == Some("m.room.create")514
+}@@ -1136,7 +1136,8 @@ async fn test_no_gap_stored_after_deduplicated_backpagination() {
11361136
// Run pagination a second time: it will consume prev-batch, which is the least11371137
// recent token.11381138
let outcome = pagination.run_backwards_once(20).await.unwrap();1139
- assert!(outcome.reached_start);1139
+ // the server still handed out prev-batch3, so dropping the useless gap is no start1140
+ assert!(!outcome.reached_start);11401141
assert!(outcome.events.is_empty());11411142
assert!(stream.is_empty());11421143
@@ -612,7 +612,9 @@ async fn test_recent_history_joins_on_any_retained_event() {
612612
}613613
614614
#[async_test]615
-async fn test_retained_events_without_a_gap_paginate_from_the_end_until_creation() {615
+async fn test_retained_events_without_a_gap_paginate_from_just_before_the_oldest() {616
+ use matrix_sdk::test_utils::mocks::RoomContextResponseTemplate;617
+616618
let server = MatrixMockServer::new().await;617619
let client = server.client_builder().build().await;618620
client.event_cache().subscribe().unwrap();@@ -628,11 +630,21 @@ async fn test_retained_events_without_a_gap_paginate_from_the_end_until_creation
628630
)629631
.await;630632
server633
+ .mock_room_event_context()634
+ .match_event_id()635
+ .ok(RoomContextResponseTemplate::new(636
+ f.text_msg("quiet").event_id(event_id!("$quiet")).into_event(),637
+ )638
+ .start("before-quiet"))639
+ .mock_once()640
+ .mount()641
+ .await;642
+ server631643
.mock_room_messages()632
- .ok(RoomMessagesResponseTemplate::default().end_token("older").events(vec![633
- f.text_msg("quiet").event_id(event_id!("$quiet")),634
- f.text_msg("days ago").event_id(event_id!("$days-ago")),635
- ]))644
+ .match_from("before-quiet")645
+ .ok(RoomMessagesResponseTemplate::default()646
+ .end_token("older")647
+ .events(vec![f.text_msg("days ago").event_id(event_id!("$days-ago"))]))636648
.mock_once()637649
.mount()638650
.await;@@ -643,3 +655,56 @@ async fn test_retained_events_without_a_gap_paginate_from_the_end_until_creation
643655
cache.events().await.unwrap().iter().filter_map(|event| event.event_id()).collect();644656
assert_eq!(ids, [event_id!("$days-ago"), event_id!("$quiet")]);645657
}658
+659
+#[async_test]660
+async fn test_a_gap_whose_page_is_all_known_does_not_claim_the_start() {661
+ use matrix_sdk::test_utils::mocks::RoomContextResponseTemplate;662
+663
+ let server = MatrixMockServer::new().await;664
+ let client = server.client_builder().build().await;665
+ client.event_cache().subscribe().unwrap();666
+ let room_id = room_id!("!stale:example.org");667
+ let room = server.sync_joined_room(&client, room_id).await;668
+ let (cache, _handles) = room.event_cache().await.unwrap();669
+ let f = event_factory(room_id);670
+ server671
+ .sync_room(672
+ &client,673
+ JoinedRoomBuilder::new(room_id)674
+ .add_timeline_event(f.text_msg("held").event_id(event_id!("$held")))675
+ .set_timeline_limited()676
+ .set_timeline_prev_batch("stale"),677
+ )678
+ .await;679
+ server680
+ .mock_room_messages()681
+ .match_from("stale")682
+ .ok(RoomMessagesResponseTemplate::default()683
+ .end_token("past-held")684
+ .events(vec![f.text_msg("held").event_id(event_id!("$held"))]))685
+ .mock_once()686
+ .mount()687
+ .await;688
+ let outcome = cache.pagination().run_backwards_once(20).await.unwrap();689
+ assert!(!outcome.reached_start);690
+691
+ server692
+ .mock_room_event_context()693
+ .match_event_id()694
+ .ok(RoomContextResponseTemplate::new(695
+ f.text_msg("held").event_id(event_id!("$held")).into_event(),696
+ )697
+ .start("before-held"))698
+ .mock_once()699
+ .mount()700
+ .await;701
+ server702
+ .mock_room_messages()703
+ .match_from("before-held")704
+ .ok(RoomMessagesResponseTemplate::default())705
+ .mock_once()706
+ .mount()707
+ .await;708
+ let outcome = cache.pagination().run_backwards_once(20).await.unwrap();709
+ assert!(outcome.reached_start);710
+}