read receipts: move the unread messages and mentions counts to separate fields of RoomInfo

This commit is contained in:
Benjamin Bouvier
2023-12-19 12:03:40 +01:00
parent 73af3d9cfa
commit b3a8f34655
5 changed files with 94 additions and 62 deletions
+4
View File
@@ -31,6 +31,8 @@ pub struct RoomInfo {
user_defined_notification_mode: Option<RoomNotificationMode>,
has_room_call: bool,
active_room_call_participants: Vec<String>,
num_unread_messages: u64,
num_unread_mentions: u64,
}
impl RoomInfo {
@@ -75,6 +77,8 @@ impl RoomInfo {
.iter()
.map(|u| u.to_string())
.collect(),
num_unread_messages: room.num_unread_messages(),
num_unread_mentions: room.num_unread_mentions(),
})
}
}
+24 -27
View File
@@ -10,7 +10,7 @@ use ruma::{
serde::Raw,
EventId, OwnedEventId, RoomId, UserId,
};
use tracing::{field::display, instrument, trace};
use tracing::{instrument, trace};
use super::BaseClient;
use crate::{error::Result, store::StateChanges, RoomInfo};
@@ -29,7 +29,7 @@ impl PreviousEventsProvider for () {
}
}
#[instrument(skip_all, fields(room_id))]
#[instrument(skip_all, fields(room_id = %room_info.room_id))]
pub(crate) async fn compute_notifications(
client: &BaseClient,
changes: &StateChanges,
@@ -37,16 +37,8 @@ pub(crate) async fn compute_notifications(
new_events: &[SyncTimelineEvent],
room_info: &mut RoomInfo,
) -> Result<()> {
// Only apply the algorithm to encrypted rooms, since unencrypted rooms' unread
// notification counts ought to be properly computed by the server.
if !room_info.is_encrypted() {
return Ok(());
}
tracing::Span::current().record("room_id", display(&room_info.room_id));
let user_id = &client.session_meta().unwrap().user_id;
let prev_latest_receipt_event_id = room_info.latest_read_receipt_event_id.clone();
let prev_latest_receipt_event_id = room_info.read_receipts.latest_read_receipt_event_id.clone();
if let Some(receipt_event) = changes.receipts.get(room_info.room_id()) {
trace!("Got a new receipt event!");
@@ -76,7 +68,7 @@ pub(crate) async fn compute_notifications(
// about.
// First, save the event id as the latest one that has a read receipt.
room_info.latest_read_receipt_event_id = Some(receipt_event_id.clone());
room_info.read_receipts.latest_read_receipt_event_id = Some(receipt_event_id.clone());
// Try to find if the read receipts refers to an event from the current sync, to
// avoid searching the cached timeline events.
@@ -124,17 +116,28 @@ pub(crate) async fn compute_notifications(
// for the next receipt.
trace!("All other ways failed, including all new events for the receipts count.");
for event in new_events {
if event.push_actions.iter().any(ruma::push::Action::is_highlight) {
room_info.notification_counts.highlight_count += 1;
}
if marks_as_unread(&event.event, user_id) {
room_info.notification_counts.notification_count += 1;
}
count_unread_and_mentions(event, user_id, room_info);
}
Ok(())
}
#[inline(always)]
fn count_unread_and_mentions(
event: &SyncTimelineEvent,
user_id: &UserId,
room_info: &mut RoomInfo,
) {
for action in &event.push_actions {
if action.should_notify() && marks_as_unread(&event.event, user_id) {
room_info.read_receipts.num_unread += 1;
}
if action.is_highlight() {
room_info.read_receipts.num_mentions += 1;
}
}
}
/// Try to find the event to which the receipt attaches to, and if found, will
/// update the notification count in the room.
///
@@ -148,20 +151,14 @@ fn find_and_count_events<'a>(
let mut counting_receipts = false;
for event in events {
if counting_receipts {
for action in &event.push_actions {
if action.is_highlight() {
room_info.notification_counts.highlight_count += 1;
}
if action.should_notify() && marks_as_unread(&event.event, user_id) {
room_info.notification_counts.notification_count += 1;
}
}
count_unread_and_mentions(event, user_id, room_info);
} else if let Ok(Some(event_id)) = event.event.get_field::<OwnedEventId>("event_id") {
if event_id == receipt_event_id {
// Bingo! Switch over to the counting state, after resetting the
// previous counts.
trace!("Found the event the receipt was referring to! Starting to count.");
room_info.notification_counts = Default::default();
room_info.read_receipts.num_unread = 0;
room_info.read_receipts.num_mentions = 0;
counting_receipts = true;
}
}
+46 -7
View File
@@ -187,6 +187,23 @@ impl Room {
self.inner.read().notification_counts
}
/// Get the number of unread messages (computed client-side).
///
/// This might be more precise than [`Self::unread_notification_counts`] for
/// encrypted rooms.
pub fn num_unread_messages(&self) -> u64 {
self.inner.read().read_receipts.num_unread
}
/// Get the number of unread mentions (computed client-side), that is,
/// messages causing a highlight in a room.
///
/// This might be more precise than [`Self::unread_notification_counts`] for
/// encrypted rooms.
pub fn num_unread_mentions(&self) -> u64 {
self.inner.read().read_receipts.num_mentions
}
/// Check if the room has its members fully synced.
///
/// Members might be missing if lazy member loading was enabled for the
@@ -710,6 +727,22 @@ impl Room {
}
}
/// Information about read receipts collected during processing of that room.
#[derive(Clone, Debug, Serialize, Deserialize, Default)]
pub struct RoomReadReceipts {
/// Does the room have unread messages?
pub(crate) num_unread: u64,
/// Does the room have messages causing highlights for the users? (aka
/// mentions)
pub(crate) num_mentions: u64,
/// The id of the event the last unthreaded (or main-threaded, for better
/// compatibility with clients that have thread support) read receipt is
/// attached to.
pub(crate) latest_read_receipt_event_id: Option<OwnedEventId>,
}
/// The underlying pure data structure for joined and left rooms.
///
/// Holds all the info needed to persist a room into the state store.
@@ -721,7 +754,10 @@ pub struct RoomInfo {
/// The state of the room.
pub(crate) room_state: RoomState,
/// The unread notifications counts.
/// The unread notifications counts, as returned by the server.
///
/// These might be incorrect for encrypted rooms, since the server doesn't
/// have access to the content of the encrypted events.
pub(crate) notification_counts: UnreadNotificationsCount,
/// The summary of this room.
@@ -743,11 +779,9 @@ pub struct RoomInfo {
#[cfg(feature = "experimental-sliding-sync")]
pub(crate) latest_event: Option<Box<LatestEvent>>,
/// The id of the event the last unthreaded (or main-threaded, for better
/// compatibility with clients that have thread support) read receipt is
/// attached to.
/// Information about read receipts for this room.
#[serde(default)]
pub(crate) latest_read_receipt_event_id: Option<OwnedEventId>,
pub(crate) read_receipts: RoomReadReceipts,
/// Base room info which holds some basic event contents important for the
/// room state.
@@ -785,7 +819,7 @@ impl RoomInfo {
encryption_state_synced: false,
#[cfg(feature = "experimental-sliding-sync")]
latest_event: None,
latest_read_receipt_event_id: None,
read_receipts: Default::default(),
base_info: Box::new(BaseRoomInfo::new()),
}
}
@@ -1267,7 +1301,7 @@ mod tests {
Raw::from_json_string(json!({"sender": "@u:i.uk"}).to_string()).unwrap().into(),
))),
base_info: Box::new(BaseRoomInfo::new()),
latest_read_receipt_event_id: None,
read_receipts: Default::default(),
};
let info_json = json!({
@@ -1307,6 +1341,11 @@ mod tests {
"name": null,
"tombstone": null,
"topic": null,
},
"read_receipts": {
"num_unread": 0,
"num_mentions": 0,
"latest_read_receipt_event_id": null,
}
});
@@ -118,7 +118,7 @@ impl RoomInfoV1 {
encryption_state_synced,
#[cfg(feature = "experimental-sliding-sync")]
latest_event: latest_event.map(|ev| Box::new(LatestEvent::new(ev))),
latest_read_receipt_event_id: None,
read_receipts: Default::default(),
base_info: base_info.migrate(create),
}
}
@@ -304,9 +304,8 @@ async fn test_room_notification_count() -> Result<()> {
let mut info_updates = alice_room.subscribe_info();
// At first, nothing has happened, so we shouldn't have any notifications.
let count = alice_room.unread_notification_counts();
assert_eq!(count.highlight_count, 0);
assert_eq!(count.notification_count, 0);
assert_eq!(alice_room.num_unread_messages(), 0);
assert_eq!(alice_room.num_unread_mentions(), 0);
assert_pending!(info_updates);
@@ -315,9 +314,8 @@ async fn test_room_notification_count() -> Result<()> {
assert!(info_updates.next().await.is_some());
let count = alice_room.unread_notification_counts();
assert_eq!(count.highlight_count, 0);
assert_eq!(count.notification_count, 0);
assert_eq!(alice_room.num_unread_messages(), 0);
assert_eq!(alice_room.num_unread_mentions(), 0);
assert!(alice_room.latest_event().is_none());
assert_pending!(info_updates);
@@ -329,10 +327,8 @@ async fn test_room_notification_count() -> Result<()> {
assert!(info_updates.next().await.is_some());
let count = alice_room.unread_notification_counts();
assert_eq!(count.highlight_count, 0);
assert_eq!(count.notification_count, 1);
let mut prev_count = count;
assert_eq!(alice_room.num_unread_messages(), 1);
assert_eq!(alice_room.num_unread_mentions(), 0);
assert_pending!(info_updates);
@@ -348,17 +344,16 @@ async fn test_room_notification_count() -> Result<()> {
loop {
assert!(info_updates.next().await.is_some());
let count = alice_room.unread_notification_counts();
if count == prev_count {
// Sometimes we get notified for changes to unrelated, other fields of
// `info_updates`.
// FIXME we receive multiple spurious room info updates.
if alice_room.num_unread_messages() == 1 && alice_room.num_unread_mentions() == 0 {
tracing::warn!("ignoring");
continue;
}
assert_eq!(count.highlight_count, 1); // one new highlight
assert_eq!(count.notification_count, 2); // the highlight counts as a new notification
prev_count = count;
// The highlight also counts as a notification.
assert_eq!(alice_room.num_unread_messages(), 2);
// One new highlight.
assert_eq!(alice_room.num_unread_mentions(), 1);
break;
}
@@ -374,16 +369,15 @@ async fn test_room_notification_count() -> Result<()> {
loop {
assert!(info_updates.next().await.is_some());
let count = alice_room.unread_notification_counts();
if count == prev_count {
if alice_room.num_unread_messages() == 2 && alice_room.num_unread_mentions() == 1 {
// Sometimes we get notified for changes to unrelated, other fields of
// `info_updates`.
tracing::warn!("ignoring");
continue;
}
assert_eq!(count.highlight_count, 0, "{count:?}");
assert_eq!(count.notification_count, 0, "{count:?}");
assert_eq!(alice_room.num_unread_messages(), 0);
assert_eq!(alice_room.num_unread_mentions(), 0);
break;
}
@@ -395,16 +389,14 @@ async fn test_room_notification_count() -> Result<()> {
// Local echo for our own message.
assert!(info_updates.next().await.is_some());
let count = alice_room.unread_notification_counts();
assert_eq!(count.highlight_count, 0, "{count:?}");
assert_eq!(count.notification_count, 0, "{count:?}");
assert_eq!(alice_room.num_unread_messages(), 0);
assert_eq!(alice_room.num_unread_mentions(), 0);
// Remote echo for our own message.
assert!(info_updates.next().await.is_some());
let count = alice_room.unread_notification_counts();
assert_eq!(count.highlight_count, 0, "{count:?}");
assert_eq!(count.notification_count, 0, "{count:?}");
assert_eq!(alice_room.num_unread_messages(), 0);
assert_eq!(alice_room.num_unread_mentions(), 0);
assert_pending!(info_updates);