From 606fa62dc8bdd8382a2dbc0229d1b6a045d0377e Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Fri, 19 Jan 2024 12:47:00 +0200 Subject: [PATCH] Add support for MSC2867 - Manually marking rooms as unread - also fixes how room account data is processed and adds tests for both when rooms and extensions are present or just extensions --- Cargo.lock | 18 +-- Cargo.toml | 4 +- bindings/matrix-sdk-ffi/src/room.rs | 34 ++++- bindings/matrix-sdk-ffi/src/room_info.rs | 3 + bindings/matrix-sdk-ffi/src/timeline/mod.rs | 6 + crates/matrix-sdk-base/src/client.rs | 16 ++- crates/matrix-sdk-base/src/rooms/mod.rs | 4 + crates/matrix-sdk-base/src/rooms/normal.rs | 7 + crates/matrix-sdk-base/src/sliding_sync.rs | 12 +- .../src/store/migration_helpers.rs | 1 + crates/matrix-sdk/src/room/mod.rs | 24 +++- crates/matrix-sdk/src/sliding_sync/mod.rs | 123 ++++++++++++++++++ .../tests/integration/room/joined.rs | 26 ++++ 13 files changed, 253 insertions(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 25236cafbcb..6a640ab851a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4737,7 +4737,7 @@ dependencies = [ [[package]] name = "ruma" version = "0.9.4" -source = "git+https://github.com/ruma/ruma?rev=684ffc789877355bd25269451b2356817c17cc3f#684ffc789877355bd25269451b2356817c17cc3f" +source = "git+https://github.com/ruma/ruma?rev=68c9bb0930f2195fa8672fbef9633ef62737df5d#68c9bb0930f2195fa8672fbef9633ef62737df5d" dependencies = [ "assign", "js_int", @@ -4753,7 +4753,7 @@ dependencies = [ [[package]] name = "ruma-client-api" version = "0.17.4" -source = "git+https://github.com/ruma/ruma?rev=684ffc789877355bd25269451b2356817c17cc3f#684ffc789877355bd25269451b2356817c17cc3f" +source = "git+https://github.com/ruma/ruma?rev=68c9bb0930f2195fa8672fbef9633ef62737df5d#68c9bb0930f2195fa8672fbef9633ef62737df5d" dependencies = [ "as_variant", "assign", @@ -4772,7 +4772,7 @@ dependencies = [ [[package]] name = "ruma-common" version = "0.12.1" -source = "git+https://github.com/ruma/ruma?rev=684ffc789877355bd25269451b2356817c17cc3f#684ffc789877355bd25269451b2356817c17cc3f" +source = "git+https://github.com/ruma/ruma?rev=68c9bb0930f2195fa8672fbef9633ef62737df5d#68c9bb0930f2195fa8672fbef9633ef62737df5d" dependencies = [ "as_variant", "base64 0.21.5", @@ -4802,7 +4802,7 @@ dependencies = [ [[package]] name = "ruma-events" version = "0.27.11" -source = "git+https://github.com/ruma/ruma?rev=684ffc789877355bd25269451b2356817c17cc3f#684ffc789877355bd25269451b2356817c17cc3f" +source = "git+https://github.com/ruma/ruma?rev=68c9bb0930f2195fa8672fbef9633ef62737df5d#68c9bb0930f2195fa8672fbef9633ef62737df5d" dependencies = [ "as_variant", "indexmap 2.1.0", @@ -4826,7 +4826,7 @@ dependencies = [ [[package]] name = "ruma-federation-api" version = "0.8.0" -source = "git+https://github.com/ruma/ruma?rev=684ffc789877355bd25269451b2356817c17cc3f#684ffc789877355bd25269451b2356817c17cc3f" +source = "git+https://github.com/ruma/ruma?rev=68c9bb0930f2195fa8672fbef9633ef62737df5d#68c9bb0930f2195fa8672fbef9633ef62737df5d" dependencies = [ "js_int", "ruma-common", @@ -4838,7 +4838,7 @@ dependencies = [ [[package]] name = "ruma-html" version = "0.1.0" -source = "git+https://github.com/ruma/ruma?rev=684ffc789877355bd25269451b2356817c17cc3f#684ffc789877355bd25269451b2356817c17cc3f" +source = "git+https://github.com/ruma/ruma?rev=68c9bb0930f2195fa8672fbef9633ef62737df5d#68c9bb0930f2195fa8672fbef9633ef62737df5d" dependencies = [ "as_variant", "html5ever", @@ -4850,7 +4850,7 @@ dependencies = [ [[package]] name = "ruma-identifiers-validation" version = "0.9.3" -source = "git+https://github.com/ruma/ruma?rev=684ffc789877355bd25269451b2356817c17cc3f#684ffc789877355bd25269451b2356817c17cc3f" +source = "git+https://github.com/ruma/ruma?rev=68c9bb0930f2195fa8672fbef9633ef62737df5d#68c9bb0930f2195fa8672fbef9633ef62737df5d" dependencies = [ "js_int", "thiserror", @@ -4859,7 +4859,7 @@ dependencies = [ [[package]] name = "ruma-macros" version = "0.12.0" -source = "git+https://github.com/ruma/ruma?rev=684ffc789877355bd25269451b2356817c17cc3f#684ffc789877355bd25269451b2356817c17cc3f" +source = "git+https://github.com/ruma/ruma?rev=68c9bb0930f2195fa8672fbef9633ef62737df5d#68c9bb0930f2195fa8672fbef9633ef62737df5d" dependencies = [ "once_cell", "proc-macro-crate 2.0.1", @@ -4874,7 +4874,7 @@ dependencies = [ [[package]] name = "ruma-push-gateway-api" version = "0.8.0" -source = "git+https://github.com/ruma/ruma?rev=684ffc789877355bd25269451b2356817c17cc3f#684ffc789877355bd25269451b2356817c17cc3f" +source = "git+https://github.com/ruma/ruma?rev=68c9bb0930f2195fa8672fbef9633ef62737df5d#68c9bb0930f2195fa8672fbef9633ef62737df5d" dependencies = [ "js_int", "ruma-common", diff --git a/Cargo.toml b/Cargo.toml index 11eb9fbc7e3..372fb59e842 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,8 +36,8 @@ futures-executor = "0.3.21" futures-util = { version = "0.3.26", default-features = false, features = ["alloc"] } http = "0.2.6" itertools = "0.12.0" -ruma = { git = "https://github.com/ruma/ruma", rev = "684ffc789877355bd25269451b2356817c17cc3f", features = ["client-api-c", "compat-upload-signatures", "compat-user-id", "compat-arbitrary-length-ids", "unstable-msc3401"] } -ruma-common = { git = "https://github.com/ruma/ruma", rev = "684ffc789877355bd25269451b2356817c17cc3f"} +ruma = { git = "https://github.com/ruma/ruma", rev = "68c9bb0930f2195fa8672fbef9633ef62737df5d", features = ["client-api-c", "compat-upload-signatures", "compat-user-id", "compat-arbitrary-length-ids", "unstable-msc3401", "unstable-msc2867"] } +ruma-common = { git = "https://github.com/ruma/ruma", rev = "68c9bb0930f2195fa8672fbef9633ef62737df5d"} once_cell = "1.16.0" rand = "0.8.5" serde = "1.0.151" diff --git a/bindings/matrix-sdk-ffi/src/room.rs b/bindings/matrix-sdk-ffi/src/room.rs index 865f7803902..75b6beceabf 100644 --- a/bindings/matrix-sdk-ffi/src/room.rs +++ b/bindings/matrix-sdk-ffi/src/room.rs @@ -20,7 +20,7 @@ use crate::{ room_info::RoomInfo, room_member::{MessageLikeEventType, RoomMember, StateEventType}, ruma::ImageInfo, - timeline::{EventTimelineItem, Timeline}, + timeline::{EventTimelineItem, ReceiptType, Timeline}, utils::u64_to_uint, TaskHandle, }; @@ -516,6 +516,38 @@ impl Room { pub async fn typing_notice(&self, is_typing: bool) -> Result<(), ClientError> { Ok(self.inner.typing_notice(is_typing).await?) } + + /// Sets a flag on the room to indicate that the user has explicitly marked + /// it as unread + pub async fn mark_as_unread(&self) -> Result<(), ClientError> { + Ok(self.inner.mark_unread(true).await?) + } + + /// Reverts a previously set unread flag. + pub async fn mark_as_read(&self) -> Result<(), ClientError> { + Ok(self.inner.mark_unread(false).await?) + } + + /// Reverts a previously set unread flag and sends a read receipt to the + /// latest event in the room. Sending read receipts is useful when + /// executing this from the room list but shouldn't be use when entering + /// the room, the timeline should be left to its own devices in that + /// case. + pub async fn mark_as_read_and_send_read_receipt( + &self, + receipt_type: ReceiptType, + ) -> Result<(), ClientError> { + let timeline = self.timeline().await?; + + if let Some(event) = timeline.latest_event().await { + if let Err(error) = timeline.send_read_receipt(receipt_type, event.event_id().unwrap()) + { + error!("Failed to send read receipt: {error}"); + } + } + + self.mark_as_read().await + } } #[uniffi::export(callback_interface)] diff --git a/bindings/matrix-sdk-ffi/src/room_info.rs b/bindings/matrix-sdk-ffi/src/room_info.rs index 580db3554a5..bda2f486656 100644 --- a/bindings/matrix-sdk-ffi/src/room_info.rs +++ b/bindings/matrix-sdk-ffi/src/room_info.rs @@ -31,6 +31,8 @@ pub struct RoomInfo { user_defined_notification_mode: Option, has_room_call: bool, active_room_call_participants: Vec, + /// Whether this room has been explicitly marked as unread + is_marked_unread: bool, /// "Interesting" messages received in that room, independently of the /// notification settings. num_unread_messages: u64, @@ -84,6 +86,7 @@ impl RoomInfo { .iter() .map(|u| u.to_string()) .collect(), + is_marked_unread: room.is_marked_unread(), num_unread_messages: room.num_unread_messages(), num_unread_notifications: room.num_unread_notifications(), num_unread_mentions: room.num_unread_mentions(), diff --git a/bindings/matrix-sdk-ffi/src/timeline/mod.rs b/bindings/matrix-sdk-ffi/src/timeline/mod.rs index 1e019bd4d5f..53d3a65b05c 100644 --- a/bindings/matrix-sdk-ffi/src/timeline/mod.rs +++ b/bindings/matrix-sdk-ffi/src/timeline/mod.rs @@ -534,6 +534,12 @@ impl Timeline { Ok(Arc::new(RoomMessageEventContentWithoutRelation::new(msgtype))) }) } + + pub async fn latest_event(&self) -> Option> { + let latest_event = self.inner.latest_event().await; + + latest_event.map(|item| Arc::new(EventTimelineItem(item))) + } } #[derive(uniffi::Record)] diff --git a/crates/matrix-sdk-base/src/client.rs b/crates/matrix-sdk-base/src/client.rs index 558a0d11ae4..d67c75a9829 100644 --- a/crates/matrix-sdk-base/src/client.rs +++ b/crates/matrix-sdk-base/src/client.rs @@ -539,7 +539,21 @@ impl BaseClient { ) { for raw_event in events { if let Ok(event) = raw_event.deserialize() { - changes.add_room_account_data(room_id, event, raw_event.clone()); + changes.add_room_account_data(room_id, event.clone(), raw_event.clone()); + + // Rooms can either appear in the current request or already be + // known to the store. If neither of + // those are true then the room is `unknown` and we cannot + // process its account data + if let AnyRoomAccountDataEvent::MarkedUnread(e) = event { + if let Some(room) = changes.room_infos.get_mut(room_id) { + room.base_info.is_marked_unread = e.content.unread; + } else if let Some(room) = self.store.get_room(room_id) { + let mut info = room.clone_info(); + info.base_info.is_marked_unread = e.content.unread; + changes.add_room(info); + } + } } } } diff --git a/crates/matrix-sdk-base/src/rooms/mod.rs b/crates/matrix-sdk-base/src/rooms/mod.rs index 42a993bbd4f..041b2c26968 100644 --- a/crates/matrix-sdk-base/src/rooms/mod.rs +++ b/crates/matrix-sdk-base/src/rooms/mod.rs @@ -104,6 +104,9 @@ pub struct BaseRoomInfo { /// memberships. #[serde(skip_serializing_if = "BTreeMap::is_empty", default)] pub(crate) rtc_member: BTreeMap>, + // Whether this room has been manually marked as unread + #[serde(default)] + pub(crate) is_marked_unread: bool, } impl BaseRoomInfo { @@ -317,6 +320,7 @@ impl Default for BaseRoomInfo { tombstone: None, topic: None, rtc_member: BTreeMap::new(), + is_marked_unread: false, } } } diff --git a/crates/matrix-sdk-base/src/rooms/normal.rs b/crates/matrix-sdk-base/src/rooms/normal.rs index eca13358325..c13b088f4d9 100644 --- a/crates/matrix-sdk-base/src/rooms/normal.rs +++ b/crates/matrix-sdk-base/src/rooms/normal.rs @@ -768,6 +768,12 @@ impl Room { .get_event_room_receipt_events(self.room_id(), receipt_type, thread, event_id) .await } + + /// Returns a boolean indicating if this room has been manually marked as + /// unread + pub fn is_marked_unread(&self) -> bool { + self.inner.read().base_info.is_marked_unread + } } /// The underlying pure data structure for joined and left rooms. @@ -1382,6 +1388,7 @@ mod tests { "encryption": null, "guest_access": null, "history_visibility": null, + "is_marked_unread": false, "join_rules": null, "max_power_level": 100, "name": null, diff --git a/crates/matrix-sdk-base/src/sliding_sync.rs b/crates/matrix-sdk-base/src/sliding_sync.rs index 9a6bb0b5d6b..f25b19f4a9a 100644 --- a/crates/matrix-sdk-base/src/sliding_sync.rs +++ b/crates/matrix-sdk-base/src/sliding_sync.rs @@ -220,8 +220,7 @@ impl BaseClient { .push(raw.clone().cast()); } - // Handles the remaining rooms account data not handled by - // process_sliding_sync_room. + // Handle room account data for (room_id, raw) in &rooms_account_data { self.handle_room_account_data(room_id, raw, &mut changes).await; @@ -358,13 +357,6 @@ impl BaseClient { .await?; } - let room_account_data = if let Some(events) = rooms_account_data.remove(room_id) { - self.handle_room_account_data(room_id, &events, changes).await; - Some(events) - } else { - None - }; - process_room_properties(room_data, &mut room_info); let timeline = self @@ -408,6 +400,8 @@ impl BaseClient { let notification_count = room_data.unread_notifications.clone().into(); room_info.update_notification_count(notification_count); + let room_account_data = rooms_account_data.get(room_id).map(|events| events.clone()); + match room_info.state() { RoomState::Joined => { // Ephemeral events are added separately, because we might not diff --git a/crates/matrix-sdk-base/src/store/migration_helpers.rs b/crates/matrix-sdk-base/src/store/migration_helpers.rs index 6db9e2ebbe7..695f75d3a8a 100644 --- a/crates/matrix-sdk-base/src/store/migration_helpers.rs +++ b/crates/matrix-sdk-base/src/store/migration_helpers.rs @@ -204,6 +204,7 @@ impl BaseRoomInfoV1 { tombstone, topic, rtc_member: BTreeMap::new(), + is_marked_unread: false, }) } } diff --git a/crates/matrix-sdk/src/room/mod.rs b/crates/matrix-sdk/src/room/mod.rs index e2a26ed8f9b..91b52c1d8d3 100644 --- a/crates/matrix-sdk/src/room/mod.rs +++ b/crates/matrix-sdk/src/room/mod.rs @@ -17,12 +17,12 @@ use matrix_sdk_common::timeout::timeout; use mime::Mime; #[cfg(feature = "e2e-encryption")] use ruma::events::{ - room::encrypted::OriginalSyncRoomEncryptedEvent, AnySyncMessageLikeEvent, AnySyncTimelineEvent, - SyncMessageLikeEvent, + marked_unread::MarkedUnreadEventContent, room::encrypted::OriginalSyncRoomEncryptedEvent, + AnySyncMessageLikeEvent, AnySyncTimelineEvent, SyncMessageLikeEvent, }; use ruma::{ api::client::{ - config::set_global_account_data, + config::{set_global_account_data, set_room_account_data}, context, error::ErrorKind, filter::LazyLoadOptions, @@ -2394,6 +2394,24 @@ impl Room { ); Ok(self.client.send(request, None).await?) } + + /// Sets a flag on the room to indicate that the user has explicitly marked + /// it as (un)read + pub async fn mark_unread(&self, unread: bool) -> Result<()> { + let user_id = + self.client.user_id().ok_or_else(|| Error::from(HttpError::AuthenticationRequired))?; + + let content = MarkedUnreadEventContent::new(unread); + + let request = set_room_account_data::v3::Request::new( + user_id.to_owned(), + self.inner.room_id().to_owned(), + &content, + )?; + + self.client.send(request, None).await?; + Ok(()) + } } /// Details of the (latest) invite. diff --git a/crates/matrix-sdk/src/sliding_sync/mod.rs b/crates/matrix-sdk/src/sliding_sync/mod.rs index ca2d61fc20c..17c7f4869cb 100644 --- a/crates/matrix-sdk/src/sliding_sync/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/mod.rs @@ -2193,6 +2193,129 @@ mod tests { Ok(()) } + #[async_test] + async fn test_process_marked_unread_room_account_data() -> Result<()> { + let room_id = owned_room_id!("!unicorn:example.org"); + + let server = MockServer::start().await; + let client = logged_in_client(Some(server.uri())).await; + client.base_client().get_or_create_room(&room_id, RoomState::Joined); + + // Setup sliding sync with with one room and one list + + let sliding_sync = client + .sliding_sync("test")? + .with_account_data_extension( + assign!(AccountDataConfig::default(), { enabled: Some(true) }), + ) + .add_list( + SlidingSyncList::builder("all") + .sync_mode(SlidingSyncMode::new_selective().add_range(0..=100)), + ) + .build() + .await?; + + let list = + sliding_sync.on_list("all", |list| ready(list.clone())).await.expect("found list all"); + + { + // Pretend the list knows that one room that will receive the account data. + list.set_maximum_number_of_rooms(Some(1)); + list.set_filled_rooms(vec![room_id.clone()]); + } + + let (_, list_stream) = list.room_list_stream(); + + pin_mut!(list_stream); + + assert_pending!(list_stream); + + // Simulate a response that only changes the marked unread state of the room to + // true + + let server_response = make_mark_unread_response("1", room_id.clone(), true, false); + + let update_summary = { + let mut pos_guard = sliding_sync.inner.position.clone().lock_owned().await; + sliding_sync.handle_response(server_response.clone(), &mut pos_guard).await? + }; + + // Check that the list list and entry received the update + + assert!(update_summary.rooms.contains(&room_id)); + assert!(update_summary.lists.contains(&String::from("all"))); + + // The room has had an update in the room list stream too. + assert_let!(Some(Some(updates)) = list_stream.next().now_or_never()); + assert_eq!(updates.len(), 1); + assert_let!( + VectorDiff::Set { index: 0, value: RoomListEntry::Filled(updated_room_id) } = + &updates[0] + ); + assert_eq!(*updated_room_id, room_id); + + let room = client.get_room(&updated_room_id).unwrap(); + + // Check the actual room data, this powers RoomInfo + + assert_eq!(room.is_marked_unread(), true); + + // Change it back to false and check if it updates + + let server_response = make_mark_unread_response("2", room_id.clone(), false, true); + + let mut pos_guard = sliding_sync.inner.position.clone().lock_owned().await; + sliding_sync.handle_response(server_response.clone(), &mut pos_guard).await?; + + let room = client.get_room(&updated_room_id).unwrap(); + + assert_eq!(room.is_marked_unread(), false); + + Ok(()) + } + + fn make_mark_unread_response( + response_number: &str, + room_id: OwnedRoomId, + unread: bool, + add_rooms_section: bool, + ) -> v4::Response { + let rooms = if add_rooms_section { + BTreeMap::from([( + room_id.clone(), + assign!(v4::SlidingSyncRoom::default(), { + name: Some("Marked as unread".to_owned()), + timeline: Vec::new(), + }), + )]) + } else { + BTreeMap::new() + }; + + let extensions = assign!(v4::Extensions::default(), { + account_data: assign!(v4::AccountData::default(), { + rooms: BTreeMap::from([ + ( + room_id.clone(), + vec![ + Raw::from_json_string( + json!({ + "content": { + "unread": unread + }, + "type": "com.famedly.marked_unread" + }) + .to_string(), + ).unwrap() + ] + ) + ]) + }) + }); + + assign!(v4::Response::new(response_number.to_owned()), { rooms: rooms, extensions: extensions }) + } + #[async_test] async fn test_process_rooms_account_data() -> Result<()> { let room = owned_room_id!("!pony:example.org"); diff --git a/crates/matrix-sdk/tests/integration/room/joined.rs b/crates/matrix-sdk/tests/integration/room/joined.rs index 84ab142c4cf..3c2dd4ac419 100644 --- a/crates/matrix-sdk/tests/integration/room/joined.rs +++ b/crates/matrix-sdk/tests/integration/room/joined.rs @@ -152,6 +152,32 @@ async fn unban_user() { room.unban_user(user, None).await.unwrap(); } +#[async_test] +async fn mark_as_unread() { + let (client, server) = logged_in_client().await; + + Mock::given(method("PUT")) + .and(path_regex( + r"^/_matrix/client/r0/user/.*/rooms/.*/account_data/com.famedly.marked_unread", + )) + .and(header("authorization", "Bearer 1234")) + .respond_with(ResponseTemplate::new(200).set_body_json(&*test_json::EMPTY)) + .mount(&server) + .await; + + mock_sync(&server, &*test_json::SYNC, None).await; + + let sync_settings = SyncSettings::new().timeout(Duration::from_millis(3000)); + + let _response = client.sync_once(sync_settings).await.unwrap(); + + let room = client.get_room(&DEFAULT_TEST_ROOM_ID).unwrap(); + + room.mark_unread(true).await.unwrap(); + + room.mark_unread(false).await.unwrap(); +} + #[async_test] async fn kick_user() { let (client, server) = logged_in_client().await;