Skip to content

Commit ff4b7a8

Browse files
committed
feat(event cache): add basic support for the latest event in the thread summary
1 parent a152f9c commit ff4b7a8

File tree

8 files changed

+145
-31
lines changed

8 files changed

+145
-31
lines changed

crates/matrix-sdk-common/src/deserialized_responses.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,10 @@ impl<'de> Deserialize<'de> for EncryptionInfo {
372372
/// - whether the user participated or not to this thread.
373373
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
374374
pub struct ThreadSummary {
375+
/// The event id for the latest reply to the thread.
376+
#[serde(skip_serializing_if = "Option::is_none")]
377+
pub latest_reply: Option<OwnedEventId>,
378+
375379
/// The number of replies to the thread.
376380
///
377381
/// This doesn't include the thread root event itself. It can be zero if no
@@ -1370,8 +1374,9 @@ mod tests {
13701374
// When creating a timeline event from a raw event, the thread summary is always
13711375
// extracted, if available.
13721376
let timeline_event = TimelineEvent::new(raw);
1373-
assert_matches!(timeline_event.thread_summary, ThreadSummaryStatus::Some(ThreadSummary { num_replies }) => {
1377+
assert_matches!(timeline_event.thread_summary, ThreadSummaryStatus::Some(ThreadSummary { num_replies, latest_reply }) => {
13741378
assert_eq!(num_replies, 2);
1379+
assert_eq!(latest_reply.as_deref(), Some(event_id!("$latest_event:example.com")));
13751380
});
13761381

13771382
// When deserializing an old serialized timeline event, the thread summary is
@@ -1386,8 +1391,9 @@ mod tests {
13861391

13871392
let timeline_event: TimelineEvent =
13881393
serde_json::from_value(serialized_timeline_item).unwrap();
1389-
assert_matches!(timeline_event.thread_summary, ThreadSummaryStatus::Some(ThreadSummary { num_replies }) => {
1394+
assert_matches!(timeline_event.thread_summary, ThreadSummaryStatus::Some(ThreadSummary { num_replies, latest_reply }) => {
13901395
assert_eq!(num_replies, 2);
1396+
assert_eq!(latest_reply.as_deref(), Some(event_id!("$latest_event:example.com")));
13911397
});
13921398
}
13931399

@@ -1651,7 +1657,10 @@ mod tests {
16511657
)])),
16521658
}),
16531659
push_actions: Default::default(),
1654-
thread_summary: ThreadSummaryStatus::Some(ThreadSummary { num_replies: 2 }),
1660+
thread_summary: ThreadSummaryStatus::Some(ThreadSummary {
1661+
num_replies: 2,
1662+
latest_reply: None,
1663+
}),
16551664
};
16561665

16571666
with_settings!({ sort_maps => true, prepend_module_to_snapshot => false }, {

crates/matrix-sdk-common/src/serde_helpers.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,10 @@ pub fn extract_bundled_thread_summary(event: &Raw<AnySyncTimelineEvent>) -> Thre
8282
// to happen to have that many events in real-world threads.
8383
let count = bundled_thread.count.try_into().unwrap_or(UInt::MAX.try_into().unwrap());
8484

85-
ThreadSummaryStatus::Some(ThreadSummary { num_replies: count })
85+
let latest_reply =
86+
bundled_thread.latest_event.get_field::<OwnedEventId>("event_id").ok().flatten();
87+
88+
ThreadSummaryStatus::Some(ThreadSummary { num_replies: count, latest_reply })
8689
}
8790
Ok(_) => ThreadSummaryStatus::None,
8891
Err(_) => ThreadSummaryStatus::Unknown,

crates/matrix-sdk-ui/src/timeline/controller/state_transaction.rs

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ use super::{
3737
};
3838
use crate::timeline::{
3939
event_handler::{FailedToParseEvent, RemovedItem, TimelineAction},
40-
ThreadSummary, TimelineDetails, VirtualTimelineItem,
40+
ThreadSummary, ThreadSummaryLatestEvent, TimelineDetails, VirtualTimelineItem,
4141
};
4242

4343
pub(in crate::timeline) struct TimelineStateTransaction<'a> {
@@ -555,13 +555,54 @@ impl<'a> TimelineStateTransaction<'a> {
555555
settings: &TimelineSettings,
556556
date_divider_adjuster: &mut DateDividerAdjuster,
557557
) -> RemovedItem {
558-
// TODO: do something with the thread summary!
559558
let TimelineEvent { push_actions, kind, thread_summary } = event;
560559

561-
let thread_summary = thread_summary.summary().map(|summary| ThreadSummary {
562-
latest_event: TimelineDetails::Unavailable,
563-
num_replies: summary.num_replies,
564-
});
560+
let thread_summary = if let Some(summary) = thread_summary.summary() {
561+
let latest_reply_item =
562+
if let Some(event_id) = summary.latest_reply.as_ref() {
563+
// Attempt to load the timeline event, either from the event cache or the
564+
// storage.
565+
let event = room_data_provider
566+
.load_event(event_id)
567+
.await
568+
.inspect_err(|err| {
569+
warn!("Failed to load thread latest event: {err}");
570+
})
571+
.ok();
572+
573+
if let Some(event) = event {
574+
// lol @ hack
575+
crate::timeline::RepliedToEvent::try_from_timeline_event(
576+
event,
577+
room_data_provider,
578+
&self.items,
579+
&mut self.meta,
580+
)
581+
.await
582+
.inspect_err(|err| {
583+
warn!("Failed to extract thread event into a timeline item content: {err}");
584+
})
585+
.ok()
586+
.flatten()
587+
.map(|replied_to| Box::new(ThreadSummaryLatestEvent {
588+
content: replied_to.content().clone(),
589+
sender: replied_to.sender().to_owned(),
590+
sender_profile: replied_to.sender_profile().clone(),
591+
}))
592+
} else {
593+
None
594+
}
595+
} else {
596+
None
597+
};
598+
599+
Some(ThreadSummary {
600+
latest_event: TimelineDetails::from_initial_value(latest_reply_item),
601+
num_replies: summary.num_replies,
602+
})
603+
} else {
604+
None
605+
};
565606

566607
let encryption_info = kind.encryption_info().cloned();
567608

crates/matrix-sdk-ui/src/timeline/tests/mod.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,4 +437,8 @@ impl RoomDataProvider for TestRoomDataProvider {
437437
) -> Result<Relations, matrix_sdk::Error> {
438438
unimplemented!();
439439
}
440+
441+
async fn load_event<'a>(&'a self, _event_id: &'a EventId) -> matrix_sdk::Result<TimelineEvent> {
442+
unimplemented!();
443+
}
440444
}

crates/matrix-sdk-ui/src/timeline/traits.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,12 @@ pub(super) trait RoomDataProvider:
145145
) -> impl Future<Output = Option<Arc<EncryptionInfo>>> + SendOutsideWasm;
146146

147147
async fn relations(&self, event_id: OwnedEventId, opts: RelationsOptions) -> Result<Relations>;
148+
149+
/// Loads an event from the cache or network.
150+
fn load_event<'a>(
151+
&'a self,
152+
event_id: &'a EventId,
153+
) -> impl Future<Output = Result<TimelineEvent>> + SendOutsideWasm + 'a;
148154
}
149155

150156
impl RoomDataProvider for Room {
@@ -294,6 +300,10 @@ impl RoomDataProvider for Room {
294300
async fn relations(&self, event_id: OwnedEventId, opts: RelationsOptions) -> Result<Relations> {
295301
self.relations(event_id, opts).await
296302
}
303+
304+
async fn load_event<'a>(&'a self, event_id: &'a EventId) -> Result<TimelineEvent> {
305+
self.load_or_fetch_event(event_id, None).await
306+
}
297307
}
298308

299309
// Internal helper to make most of retry_event_decryption independent of a room

crates/matrix-sdk-ui/tests/integration/timeline/thread.rs

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -213,16 +213,18 @@ async fn test_extract_bundled_thread_summary() {
213213
let f = EventFactory::new().room(room_id).sender(&ALICE);
214214
let thread_event_id = event_id!("$thread_root");
215215
let latest_event_id = event_id!("$latest_event");
216+
let latest_event = f.text_msg("the last one!").event_id(latest_event_id).into_event();
216217

217218
let event = f
218219
.text_msg("thready thread mcthreadface")
219-
.with_bundled_thread_summary(
220-
f.text_msg("the last one!").event_id(latest_event_id).into_raw(),
221-
42,
222-
false,
223-
)
220+
.with_bundled_thread_summary(latest_event.raw().cast_ref().clone(), 42, false)
224221
.event_id(thread_event_id);
225222

223+
// Set up the /event for the latest thread event.
224+
// FIXME(bnjbvr): shouldn't be necessary, the event cache could save the bundled
225+
// latest event instead.
226+
server.mock_room_event().match_event_id().ok(latest_event).mock_once().mount().await;
227+
226228
server.sync_room(&client, JoinedRoomBuilder::new(room_id).add_timeline_event(event)).await;
227229

228230
assert_let_timeout!(Some(timeline_updates) = stream.next());
@@ -234,8 +236,15 @@ async fn test_extract_bundled_thread_summary() {
234236
let event_item = value.as_event().unwrap();
235237
assert_eq!(event_item.event_id().unwrap(), thread_event_id);
236238
assert_let!(Some(summary) = event_item.content().thread_summary());
237-
// Soon™, Stefan, soon™.
238-
assert!(summary.latest_event.is_unavailable());
239+
240+
// We get the latest event from the bundled thread summary.
241+
assert!(summary.latest_event.is_ready());
242+
assert_let!(TimelineDetails::Ready(latest_event) = summary.latest_event);
243+
assert_eq!(latest_event.content.as_message().unwrap().body(), "the last one!");
244+
assert_eq!(latest_event.sender, *ALICE);
245+
assert!(latest_event.sender_profile.is_unavailable());
246+
247+
// We get the count from the bundled thread summary.
239248
assert_eq!(summary.num_replies, 42);
240249

241250
assert_let!(VectorDiff::PushFront { value } = &timeline_updates[1]);
@@ -321,9 +330,15 @@ async fn test_new_thread_reply_causes_thread_summary() {
321330
assert_eq!(event_item.event_id().unwrap(), thread_event_id);
322331
assert!(event_item.content().thread_root().is_none());
323332

333+
// The thread summary contains the detailed information about the latest event.
324334
assert_let!(Some(summary) = event_item.content().thread_summary());
325-
// Soon™, Stefan, soon™.
326-
assert!(summary.latest_event.is_unavailable());
335+
assert!(summary.latest_event.is_ready());
336+
assert_let!(TimelineDetails::Ready(latest_event) = summary.latest_event);
337+
assert_eq!(latest_event.content.as_message().unwrap().body(), "thread reply");
338+
assert_eq!(latest_event.sender, *BOB);
339+
assert!(latest_event.sender_profile.is_unavailable());
340+
341+
// The thread summary contains the number of replies.
327342
assert_eq!(summary.num_replies, 1);
328343

329344
assert_pending!(stream);
@@ -373,8 +388,14 @@ async fn test_new_thread_reply_causes_thread_summary() {
373388
assert!(event_item.content().thread_root().is_none());
374389

375390
assert_let!(Some(summary) = event_item.content().thread_summary());
376-
// Soon™, Stefan, soon™.
377-
assert!(summary.latest_event.is_unavailable());
391+
392+
// The latest event has been updated.
393+
assert!(summary.latest_event.is_ready());
394+
assert_let!(TimelineDetails::Ready(latest_event) = summary.latest_event);
395+
assert_eq!(latest_event.content.as_message().unwrap().body(), "another thread reply");
396+
assert_eq!(latest_event.sender, *BOB);
397+
assert!(latest_event.sender_profile.is_unavailable());
398+
378399
// The number of replies has been updated.
379400
assert_eq!(summary.num_replies, 2);
380401
}

crates/matrix-sdk/src/event_cache/pagination.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ impl RoomPagination {
352352
};
353353

354354
let next_diffs = state
355-
.with_events_mut(|room_events| {
355+
.with_events_mut(false, |room_events| {
356356
// Reverse the order of the events as `/messages` has been called with `dir=b`
357357
// (backwards). The `RoomEvents` API expects the first event to be the oldest.
358358
// Let's re-order them for this block.

crates/matrix-sdk/src/event_cache/room/mod.rs

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,7 @@ impl RoomEventCacheInner {
429429
// Add the previous back-pagination token (if present), followed by the timeline
430430
// events themselves.
431431
let new_timeline_event_diffs = state
432-
.with_events_mut(|room_events| {
432+
.with_events_mut(true, |room_events| {
433433
// If we only received duplicated events, we don't need to store the gap: if
434434
// there was a gap, we'd have received an unknown event at the tail of
435435
// the room's timeline (unless the server reordered sync events since the last
@@ -1161,6 +1161,7 @@ mod private {
11611161
#[instrument(skip_all, fields(room_id = %self.room))]
11621162
pub async fn with_events_mut<F>(
11631163
&mut self,
1164+
is_live_sync: bool,
11641165
func: F,
11651166
) -> Result<Vec<VectorDiff<TimelineEvent>>, EventCacheError>
11661167
where
@@ -1173,7 +1174,7 @@ mod private {
11731174

11741175
for event in &events_to_post_process {
11751176
self.maybe_apply_new_redaction(event).await?;
1176-
self.analyze_thread_root(event).await?;
1177+
self.analyze_thread_root(event, is_live_sync).await?;
11771178
}
11781179

11791180
// If we've never waited for an initial previous-batch token, and we now have at
@@ -1192,7 +1193,11 @@ mod private {
11921193
/// If the event is a threaded reply, ensure the related thread's root
11931194
/// event (i.e. first thread event) has a thread summary.
11941195
#[instrument(skip_all)]
1195-
async fn analyze_thread_root(&mut self, event: &Event) -> Result<(), EventCacheError> {
1196+
async fn analyze_thread_root(
1197+
&mut self,
1198+
event: &Event,
1199+
is_live_sync: bool,
1200+
) -> Result<(), EventCacheError> {
11961201
let Some(thread_root) = extract_thread_root(event.raw()) else {
11971202
// No thread root, carry on.
11981203
return Ok(());
@@ -1219,13 +1224,34 @@ mod private {
12191224
related_thread_events.len()
12201225
};
12211226

1222-
let new_summary = ThreadSummary { num_replies };
1227+
let prev_summary = target_event.thread_summary.summary();
1228+
let mut latest_reply =
1229+
prev_summary.as_ref().and_then(|summary| summary.latest_reply.clone());
12231230

1224-
if let ThreadSummaryStatus::Some(existing) = &target_event.thread_summary {
1225-
if existing == &new_summary {
1226-
trace!("thread summary is already up-to-date");
1227-
return Ok(());
1228-
}
1231+
// If we're live-syncing, then the latest event is always the event we're
1232+
// currently processing. We're processing the sync events from oldest to newest,
1233+
// so a a single sync response containing multiple thread events
1234+
// will correctly override the latest event to the most recent one.
1235+
//
1236+
// If we're back-paginating, then we shouldn't update the latest event
1237+
// information if it's set. If it's not set, then we should update
1238+
// it to the last event in the batch. TODO(bnjbvr): the code is
1239+
// wrong here in this particular case, because a single pagination
1240+
// batch may include multiple events in the same thread, and they're
1241+
// processed from oldest to newest; so the first in-thread event seen in that
1242+
// batch will be marked as the latest reply, which is incorrect.
1243+
// This will be fixed Later™ by using a proper linked chunk per
1244+
// thread.
1245+
1246+
if is_live_sync || latest_reply.is_none() {
1247+
latest_reply = event.event_id();
1248+
}
1249+
1250+
let new_summary = ThreadSummary { num_replies, latest_reply };
1251+
1252+
if prev_summary == Some(&new_summary) {
1253+
trace!("thread summary is already up-to-date");
1254+
return Ok(());
12291255
}
12301256

12311257
// Cause an update to observers.

0 commit comments

Comments
 (0)