diff --git a/bindings/matrix-sdk-ffi/src/room_info.rs b/bindings/matrix-sdk-ffi/src/room_info.rs index 2d9531658..580db3554 100644 --- a/bindings/matrix-sdk-ffi/src/room_info.rs +++ b/bindings/matrix-sdk-ffi/src/room_info.rs @@ -31,7 +31,14 @@ pub struct RoomInfo { user_defined_notification_mode: Option, has_room_call: bool, active_room_call_participants: Vec, + /// "Interesting" messages received in that room, independently of the + /// notification settings. num_unread_messages: u64, + /// Events that will notify the user, according to their + /// notification settings. + num_unread_notifications: u64, + /// Events causing mentions/highlights for the user, according to their + /// notification settings. num_unread_mentions: u64, } @@ -78,6 +85,7 @@ impl RoomInfo { .map(|u| u.to_string()) .collect(), num_unread_messages: room.num_unread_messages(), + num_unread_notifications: room.num_unread_notifications(), num_unread_mentions: room.num_unread_mentions(), }) } diff --git a/crates/matrix-sdk-base/src/read_receipts.rs b/crates/matrix-sdk-base/src/read_receipts.rs index 9a145951b..e94f27b45 100644 --- a/crates/matrix-sdk-base/src/read_receipts.rs +++ b/crates/matrix-sdk-base/src/read_receipts.rs @@ -100,7 +100,7 @@ pub(crate) async fn compute_notifications( // There's no new read-receipt here. We assume the cached events have been // properly processed, and we only need to process the new events based // on the previous receipt. - trace!("Couldn't find the event attached to the latest receipt; looking if the past latest known receipt refers to a new event..."); + trace!("No new receipts, or couldn't find attached event; looking if the past latest known receipt refers to a new event..."); if find_and_count_events(&receipt_event_id, user_id, new_events.iter(), room_info) { // We found the event to which the previous receipt attached to, so our work is // done here. @@ -114,7 +114,7 @@ pub(crate) async fn compute_notifications( // // In that case, accumulate all events as part of the current batch, and wait // for the next receipt. - trace!("All other ways failed, including all new events for the receipts count."); + trace!("Default path: including all new events for the receipts count."); for event in new_events { count_unread_and_mentions(event, user_id, room_info); } @@ -128,9 +128,12 @@ fn count_unread_and_mentions( user_id: &UserId, room_info: &mut RoomInfo, ) { + if marks_as_unread(&event.event, user_id) { + room_info.read_receipts.num_unread += 1; + } 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.should_notify() { + room_info.read_receipts.num_notifications += 1; } if action.is_highlight() { room_info.read_receipts.num_mentions += 1; @@ -158,6 +161,7 @@ fn find_and_count_events<'a>( // previous counts. trace!("Found the event the receipt was referring to! Starting to count."); room_info.read_receipts.num_unread = 0; + room_info.read_receipts.num_notifications = 0; room_info.read_receipts.num_mentions = 0; counting_receipts = true; } diff --git a/crates/matrix-sdk-base/src/rooms/normal.rs b/crates/matrix-sdk-base/src/rooms/normal.rs index 027a2de2d..81cab7931 100644 --- a/crates/matrix-sdk-base/src/rooms/normal.rs +++ b/crates/matrix-sdk-base/src/rooms/normal.rs @@ -195,6 +195,14 @@ impl Room { self.inner.read().read_receipts.num_unread } + /// Get the number of unread notifications (computed client-side). + /// + /// This might be more precise than [`Self::unread_notification_counts`] for + /// encrypted rooms. + pub fn num_unread_notifications(&self) -> u64 { + self.inner.read().read_receipts.num_notifications + } + /// Get the number of unread mentions (computed client-side), that is, /// messages causing a highlight in a room. /// @@ -733,6 +741,9 @@ pub struct RoomReadReceipts { /// Does the room have unread messages? pub(crate) num_unread: u64, + /// Does the room have unread events that should notify? + pub(crate) num_notifications: u64, + /// Does the room have messages causing highlights for the users? (aka /// mentions) pub(crate) num_mentions: u64, @@ -1345,6 +1356,7 @@ mod tests { "read_receipts": { "num_unread": 0, "num_mentions": 0, + "num_notifications": 0, "latest_read_receipt_event_id": null, } }); diff --git a/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs b/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs index 1ecc95a82..fab7c2918 100644 --- a/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs +++ b/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs @@ -9,7 +9,9 @@ use matrix_sdk::{ api::client::{ receipt::create_receipt::v3::ReceiptType, room::create_room::v3::Request as CreateRoomRequest, - sync::sync_events::v4::{E2EEConfig, ReceiptsConfig, ToDeviceConfig}, + sync::sync_events::v4::{ + AccountDataConfig, E2EEConfig, ReceiptsConfig, ToDeviceConfig, + }, }, assign, events::{ @@ -237,6 +239,9 @@ async fn test_room_notification_count() -> Result<()> { let sync = alice .sliding_sync("main")? .with_receipt_extension(assign!(ReceiptsConfig::default(), { enabled: Some(true) })) + .with_account_data_extension( + assign!(AccountDataConfig::default(), { enabled: Some(true) }), + ) .add_list( SlidingSyncList::builder("all") .sync_mode(SlidingSyncMode::new_selective().add_range(0..=20)), @@ -306,6 +311,7 @@ async fn test_room_notification_count() -> Result<()> { // At first, nothing has happened, so we shouldn't have any notifications. assert_eq!(alice_room.num_unread_messages(), 0); assert_eq!(alice_room.num_unread_mentions(), 0); + assert_eq!(alice_room.num_unread_notifications(), 0); assert_pending!(info_updates); @@ -316,6 +322,7 @@ async fn test_room_notification_count() -> Result<()> { assert_eq!(alice_room.num_unread_messages(), 0); assert_eq!(alice_room.num_unread_mentions(), 0); + assert_eq!(alice_room.num_unread_notifications(), 0); assert!(alice_room.latest_event().is_none()); assert_pending!(info_updates); @@ -328,12 +335,12 @@ async fn test_room_notification_count() -> Result<()> { assert!(info_updates.next().await.is_some()); assert_eq!(alice_room.num_unread_messages(), 1); + assert_eq!(alice_room.num_unread_notifications(), 1); assert_eq!(alice_room.num_unread_mentions(), 0); assert_pending!(info_updates); // Bob sends a mention message. - let bob_room = bob.get_room(&room_id).expect("bob knows about alice's room"); bob_room .send( RoomMessageEventContent::text_plain("Hello my dear friend Alice!") @@ -352,6 +359,7 @@ async fn test_room_notification_count() -> Result<()> { // The highlight also counts as a notification. assert_eq!(alice_room.num_unread_messages(), 2); + assert_eq!(alice_room.num_unread_notifications(), 2); // One new highlight. assert_eq!(alice_room.num_unread_mentions(), 1); break; @@ -377,6 +385,7 @@ async fn test_room_notification_count() -> Result<()> { } assert_eq!(alice_room.num_unread_messages(), 0); + assert_eq!(alice_room.num_unread_notifications(), 0); assert_eq!(alice_room.num_unread_mentions(), 0); break; } @@ -390,15 +399,71 @@ async fn test_room_notification_count() -> Result<()> { assert!(info_updates.next().await.is_some()); assert_eq!(alice_room.num_unread_messages(), 0); + assert_eq!(alice_room.num_unread_notifications(), 0); assert_eq!(alice_room.num_unread_mentions(), 0); // Remote echo for our own message. assert!(info_updates.next().await.is_some()); assert_eq!(alice_room.num_unread_messages(), 0); + assert_eq!(alice_room.num_unread_notifications(), 0); assert_eq!(alice_room.num_unread_mentions(), 0); assert_pending!(info_updates); + // Now Alice is only interesting in mentions of their name. + let settings = alice.notification_settings().await; + + tracing::warn!("Updating room notification mode to mentions and keywords only..."); + settings + .set_room_notification_mode( + alice_room.room_id(), + matrix_sdk::notification_settings::RoomNotificationMode::MentionsAndKeywordsOnly, + ) + .await?; + tracing::warn!("Done!"); + + // Wait for remote echo. + let _ = settings.subscribe_to_changes().recv().await; + + bob_room.send(RoomMessageEventContent::text_plain("I said hello!")).await?; + + assert!(info_updates.next().await.is_some()); + + // The message doesn't contain a mention, so it doesn't notify Alice. But it + // exists. + assert_eq!(alice_room.num_unread_messages(), 1); + assert_eq!(alice_room.num_unread_notifications(), 0); + // One new highlight. + assert_eq!(alice_room.num_unread_mentions(), 0); + + assert_pending!(info_updates); + + // Bob sends a mention message. + bob_room + .send( + RoomMessageEventContent::text_plain("Why, hello there Alice!") + .set_mentions(Mentions::with_user_ids([alice.user_id().unwrap().to_owned()])), + ) + .await?; + + loop { + assert!(info_updates.next().await.is_some()); + + // 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; + } + + // The highlight also counts as a notification. + assert_eq!(alice_room.num_unread_messages(), 2); + assert_eq!(alice_room.num_unread_notifications(), 1); + assert_eq!(alice_room.num_unread_mentions(), 1); + break; + } + + assert_pending!(info_updates); + Ok(()) }