Skip to content

Commit aa29107

Browse files
committed
feat(event cache): include the reply count in the ThreadSummary
1 parent 672bb9f commit aa29107

File tree

9 files changed

+113
-26
lines changed

9 files changed

+113
-26
lines changed

bindings/matrix-sdk-ffi/src/timeline/msg_like.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@ pub struct PollAnswer {
242242
#[derive(Clone, uniffi::Object)]
243243
pub struct ThreadSummary {
244244
pub latest_event: ThreadSummaryLatestEventDetails,
245+
pub num_replies: usize,
245246
}
246247

247248
#[matrix_sdk_ffi_macros::export]
@@ -275,6 +276,6 @@ impl From<matrix_sdk_ui::timeline::ThreadSummary> for ThreadSummary {
275276
}
276277
};
277278

278-
Self { latest_event }
279+
Self { latest_event, num_replies: value.num_replies }
279280
}
280281
}

crates/matrix-sdk-base/src/event_cache/store/traits.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,11 @@ pub trait EventCacheStore: AsyncTraitDeps {
124124

125125
/// Find all the events that relate to a given event.
126126
///
127+
/// Note: it doesn't process relations recursively: for instance, if
128+
/// requesting only thread events, it will NOT return the aggregated
129+
/// events affecting the returned events. It is the responsibility of
130+
/// the caller to do so, if needed.
131+
///
127132
/// An additional filter can be provided to only retrieve related events for
128133
/// a certain relationship.
129134
async fn find_event_relations(

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

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -370,10 +370,15 @@ impl<'de> Deserialize<'de> for EncryptionInfo {
370370
/// - the number of replies to the thread,
371371
/// - the full event of the latest reply to the thread,
372372
/// - whether the user participated or not to this thread.
373-
///
374-
/// At the moment, it contains nothing.
375373
#[derive(Clone, Debug, Serialize, Deserialize)]
376-
pub struct ThreadSummary {}
374+
pub struct ThreadSummary {
375+
/// The number of replies to the thread.
376+
///
377+
/// This doesn't include the thread root event itself. It can be zero if no
378+
/// events in the thread are considered to be meaningful (or they've all
379+
/// been redacted).
380+
pub num_replies: usize,
381+
}
377382

378383
/// The status of a thread summary.
379384
#[derive(Clone, Debug, Default, Serialize, Deserialize)]
@@ -1365,7 +1370,9 @@ mod tests {
13651370
// When creating a timeline event from a raw event, the thread summary is always
13661371
// extracted, if available.
13671372
let timeline_event = TimelineEvent::new(raw);
1368-
assert_matches!(timeline_event.thread_summary, ThreadSummaryStatus::Some(ThreadSummary {}));
1373+
assert_matches!(timeline_event.thread_summary, ThreadSummaryStatus::Some(ThreadSummary { num_replies }) => {
1374+
assert_eq!(num_replies, 2);
1375+
});
13691376

13701377
// When deserializing an old serialized timeline event, the thread summary is
13711378
// also extracted, if it wasn't serialized.
@@ -1379,7 +1386,9 @@ mod tests {
13791386

13801387
let timeline_event: TimelineEvent =
13811388
serde_json::from_value(serialized_timeline_item).unwrap();
1382-
assert_matches!(timeline_event.thread_summary, ThreadSummaryStatus::Some(ThreadSummary {}));
1389+
assert_matches!(timeline_event.thread_summary, ThreadSummaryStatus::Some(ThreadSummary { num_replies }) => {
1390+
assert_eq!(num_replies, 2);
1391+
});
13831392
}
13841393

13851394
#[test]
@@ -1642,7 +1651,7 @@ mod tests {
16421651
)])),
16431652
}),
16441653
push_actions: Default::default(),
1645-
thread_summary: ThreadSummaryStatus::Some(ThreadSummary {}),
1654+
thread_summary: ThreadSummaryStatus::Some(ThreadSummary { num_replies: 2 }),
16461655
};
16471656

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

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
use ruma::{
1919
events::{relation::BundledThread, AnySyncTimelineEvent},
2020
serde::Raw,
21-
OwnedEventId,
21+
OwnedEventId, UInt,
2222
};
2323
use serde::Deserialize;
2424

@@ -77,9 +77,12 @@ struct Unsigned {
7777
pub fn extract_bundled_thread_summary(event: &Raw<AnySyncTimelineEvent>) -> ThreadSummaryStatus {
7878
match event.get_field::<Unsigned>("unsigned") {
7979
Ok(Some(Unsigned { relations: Some(Relations { thread: Some(bundled_thread) }) })) => {
80-
// Currently, we don't do anything with the bundled thread, but later we will!
81-
let _ = bundled_thread;
82-
ThreadSummaryStatus::Some(ThreadSummary {})
80+
// Take the count from the bundled thread summary, if available. If it can't be
81+
// converted to a `u64`, we use `UInt::MAX` as a fallback, as this is unlikely
82+
// to happen to have that many events in real-world threads.
83+
let count = bundled_thread.count.try_into().unwrap_or(UInt::MAX.try_into().unwrap());
84+
85+
ThreadSummaryStatus::Some(ThreadSummary { num_replies: count })
8386
}
8487
Ok(_) => ThreadSummaryStatus::None,
8588
Err(_) => ThreadSummaryStatus::Unknown,

crates/matrix-sdk-common/src/snapshots/snapshot_test_sync_timeline_event.snap

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ expression: "serde_json::to_value(&room_event).unwrap()"
4646
}
4747
},
4848
"thread_summary": {
49-
"Some": {}
49+
"Some": {
50+
"num_replies": 2
51+
}
5052
}
5153
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -558,9 +558,9 @@ impl<'a> TimelineStateTransaction<'a> {
558558
// TODO: do something with the thread summary!
559559
let TimelineEvent { push_actions, kind, thread_summary } = event;
560560

561-
let thread_summary = thread_summary.summary().map(|_common_summary| {
562-
// TODO: later, fill the latest event in the thread summary!
563-
ThreadSummary { latest_event: TimelineDetails::Unavailable }
561+
let thread_summary = thread_summary.summary().map(|summary| ThreadSummary {
562+
latest_event: TimelineDetails::Unavailable,
563+
num_replies: summary.num_replies,
564564
});
565565

566566
let encryption_info = kind.encryption_info().cloned();

crates/matrix-sdk-ui/src/timeline/event_item/content/msg_like.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,15 @@ pub enum MsgLikeKind {
3939
#[derive(Clone, Debug)]
4040
pub struct ThreadSummary {
4141
pub latest_event: TimelineDetails<Box<ThreadSummaryLatestEvent>>,
42+
43+
/// The number of events in the thread, except for the thread root.
44+
///
45+
/// This can be zero if all the events in the thread have been redacted.
46+
///
47+
/// Note: this doesn't interact with the timeline filter; so opening a
48+
/// thread-focused timeline with the same timeline filter may result in
49+
/// *fewer* events than this number.
50+
pub num_replies: usize,
4251
}
4352

4453
#[derive(Clone, Debug)]

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

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,7 @@ async fn test_extract_bundled_thread_summary() {
236236
assert_let!(Some(summary) = event_item.content().thread_summary());
237237
// Soon™, Stefan, soon™.
238238
assert!(summary.latest_event.is_unavailable());
239+
assert_eq!(summary.num_replies, 42);
239240

240241
assert_let!(VectorDiff::PushFront { value } = &timeline_updates[1]);
241242
assert!(value.is_date_divider());
@@ -278,8 +279,8 @@ async fn test_new_thread_reply_causes_thread_summary() {
278279
// When I receive a threaded reply to this event,
279280
let reply_event_id = event_id!("$thread_reply");
280281
let event = f
281-
.sender(&BOB)
282282
.text_msg("thread reply")
283+
.sender(&BOB)
283284
.in_thread(thread_event_id, thread_event_id)
284285
.event_id(reply_event_id);
285286

@@ -323,4 +324,57 @@ async fn test_new_thread_reply_causes_thread_summary() {
323324
assert_let!(Some(summary) = event_item.content().thread_summary());
324325
// Soon™, Stefan, soon™.
325326
assert!(summary.latest_event.is_unavailable());
327+
assert_eq!(summary.num_replies, 1);
328+
329+
assert_pending!(stream);
330+
331+
// A new thread reply updates the number of replies in the thread.
332+
let another_reply_event_id = event_id!("$another_thread_reply");
333+
server
334+
.sync_room(
335+
&client,
336+
JoinedRoomBuilder::new(room_id).add_timeline_event(
337+
f.text_msg("another thread reply")
338+
.sender(&BOB)
339+
.in_thread(thread_event_id, reply_event_id)
340+
.event_id(another_reply_event_id),
341+
),
342+
)
343+
.await;
344+
345+
assert_let_timeout!(Some(timeline_updates) = stream.next());
346+
assert_eq!(timeline_updates.len(), 4);
347+
348+
// (Read receipt from Bob moves.)
349+
assert_let!(VectorDiff::Set { index: 2, .. } = &timeline_updates[0]);
350+
351+
// Then we're seeing the new thread reply.
352+
assert_let!(VectorDiff::PushBack { value } = &timeline_updates[1]);
353+
let event_item = value.as_event().unwrap();
354+
assert_eq!(event_item.event_id().unwrap(), another_reply_event_id);
355+
assert!(event_item.content().thread_summary().is_none());
356+
assert_eq!(event_item.content().thread_root().as_deref(), Some(thread_event_id));
357+
358+
// Then the first thread reply is updated with the up-to-date thread summary in
359+
// the replied-to event.
360+
assert_let!(VectorDiff::Set { index: 2, value } = &timeline_updates[2]);
361+
let event_item = value.as_event().unwrap();
362+
assert_eq!(event_item.event_id().unwrap(), reply_event_id);
363+
let replied_to_details = value.as_event().unwrap().content().in_reply_to().unwrap().event;
364+
assert_let!(TimelineDetails::Ready(replied_to_event) = replied_to_details);
365+
// Spoiling a bit here…
366+
assert_eq!(replied_to_event.content().thread_summary().unwrap().num_replies, 2);
367+
368+
// Then, we receive an update for the thread root itself, which now has an
369+
// up-to-date thread summary.
370+
assert_let!(VectorDiff::Set { index: 1, value } = &timeline_updates[3]);
371+
let event_item = value.as_event().unwrap();
372+
assert_eq!(event_item.event_id().unwrap(), thread_event_id);
373+
assert!(event_item.content().thread_root().is_none());
374+
375+
assert_let!(Some(summary) = event_item.content().thread_summary());
376+
// Soon™, Stefan, soon™.
377+
assert!(summary.latest_event.is_unavailable());
378+
// The number of replies has been updated.
379+
assert_eq!(summary.num_replies, 2);
326380
}

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

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1210,17 +1210,21 @@ mod private {
12101210
return Ok(());
12111211
};
12121212

1213-
match target_event.thread_summary {
1214-
ThreadSummaryStatus::Unknown | ThreadSummaryStatus::None => {}
1215-
ThreadSummaryStatus::Some(_thread_summary) => {
1216-
// The event already had a thread summary; ignore.
1217-
// TODO: later, we might want to recompute the thread summary here, and cause
1218-
// an update if it's changed.
1219-
return Ok(());
1220-
}
1221-
}
1213+
// Read the latest number of thread replies from the store.
1214+
//
1215+
// Implementation note: since this is based on the `m.relates_to` field, and
1216+
// that field can only be present on room messages, we don't have to
1217+
// worry about filtering out aggregation events (like
1218+
// reactions/edits/etc.). Pretty neat, huh?
1219+
let num_replies = {
1220+
let store_guard = &*self.store.lock().await?;
1221+
let related_thread_events = store_guard
1222+
.find_event_relations(&self.room, &thread_root, Some(&[RelationType::Thread]))
1223+
.await?;
1224+
related_thread_events.len()
1225+
};
12221226

1223-
target_event.thread_summary = ThreadSummaryStatus::Some(ThreadSummary {});
1227+
target_event.thread_summary = ThreadSummaryStatus::Some(ThreadSummary { num_replies });
12241228

12251229
self.replace_event_at(location, target_event).await?;
12261230

0 commit comments

Comments
 (0)