From fab1c1c2991c0d24e7ffa58452df66315d120c54 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 8 Feb 2024 08:59:11 +0100 Subject: [PATCH 01/13] chore(base): Introduce a small helper to clarify the code. I believe this small refactoring makes the code simpler to read, esp. when more event type will be added. --- crates/matrix-sdk-base/src/client.rs | 49 +++++++++++++++++++++------- 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/crates/matrix-sdk-base/src/client.rs b/crates/matrix-sdk-base/src/client.rs index a5ade66828d..38a572edfde 100644 --- a/crates/matrix-sdk-base/src/client.rs +++ b/crates/matrix-sdk-base/src/client.rs @@ -538,22 +538,49 @@ impl BaseClient { events: &[Raw], changes: &mut StateChanges, ) { + // Small helper to make the code easier to read. + // + // It finds the appropriate `RoomInfo`, allowing the caller to modify it, and + // save it in the correct place. + fn on_rooms_infos( + room_id: &RoomId, + changes: &mut StateChanges, + client: &BaseClient, + on_room_info: F, + ) where + F: Fn(&mut RoomInfo), + { + // `StateChanges` has the `RoomInfo`. + if let Some(room) = changes.room_infos.get_mut(room_id) { + // Show time. + on_room_info(room); + } + // The `BaseClient` has the `Room`, which has the `RoomInfo`. + else if let Some(room) = client.store.get_room(room_id) { + // Clone the `RoomInfo`. + let mut room_info = room.clone_info(); + + // Show time. + on_room_info(&mut room_info); + + // Update the `RoomInfo` via `StateChanges`. + changes.add_room(room_info); + } + } + for raw_event in events { if let Ok(event) = raw_event.deserialize() { 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); + match event { + AnyRoomAccountDataEvent::MarkedUnread(event) => { + on_rooms_infos(room_id, changes, self, |room_info| { + room_info.base_info.is_marked_unread = event.content.unread; + }); } + + // Nothing. + _ => {} } } } From 385e6933d2c7f7b542c10a14df914f8075ee77c0 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 8 Feb 2024 10:32:04 +0100 Subject: [PATCH 02/13] chore(base): Update `bitflags`. --- crates/matrix-sdk-base/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/matrix-sdk-base/Cargo.toml b/crates/matrix-sdk-base/Cargo.toml index 0e280b43702..00718034f0a 100644 --- a/crates/matrix-sdk-base/Cargo.toml +++ b/crates/matrix-sdk-base/Cargo.toml @@ -39,7 +39,7 @@ as_variant = { workspace = true } assert_matches = { workspace = true, optional = true } assert_matches2 = { workspace = true, optional = true } async-trait = { workspace = true } -bitflags = "2.1.0" +bitflags = "2.4.0" eyeball = { workspace = true } eyeball-im = { workspace = true } futures-util = { workspace = true } From fd716bcd8172d7e31e78523a51bc41a072e1f926 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 8 Feb 2024 15:14:03 +0100 Subject: [PATCH 03/13] feat(base): Add the `NotableTags` type. This patch is the first step to remove the [`RoomNotableTags`] API. The idea is to compute the notable tags as a single 8-bit unsigned integer (`u8`) and to store it inside `RoomInfo`. The benefits are numerous: 1. The type is smaller that `RoomNotableTags`, 2. It's part of `RoomInfo` so: - We don't need an async API to fetch the tags from the store and to compute the notable tags, - It can be put in the store, - The observable/subscribe mechanism is supported by `RoomInfo` behind observable instead of introducing a new subscribe mechanism. --- Cargo.lock | 3 ++ crates/matrix-sdk-base/Cargo.toml | 2 +- crates/matrix-sdk-base/src/client.rs | 11 +++++- crates/matrix-sdk-base/src/rooms/mod.rs | 38 +++++++++++++++++++ crates/matrix-sdk-base/src/rooms/normal.rs | 2 +- .../src/store/migration_helpers.rs | 3 +- 6 files changed, 54 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 627e5512f0d..03b31269b24 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -523,6 +523,9 @@ name = "bitflags" version = "2.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ed570934406eb16438a4e976b1b4500774099c13b8cb96eec99f620f05090ddf" +dependencies = [ + "serde", +] [[package]] name = "bitmaps" diff --git a/crates/matrix-sdk-base/Cargo.toml b/crates/matrix-sdk-base/Cargo.toml index 00718034f0a..7e3281d0e0e 100644 --- a/crates/matrix-sdk-base/Cargo.toml +++ b/crates/matrix-sdk-base/Cargo.toml @@ -39,7 +39,7 @@ as_variant = { workspace = true } assert_matches = { workspace = true, optional = true } assert_matches2 = { workspace = true, optional = true } async-trait = { workspace = true } -bitflags = "2.4.0" +bitflags = { version = "2.4.0", features = ["serde"] } eyeball = { workspace = true } eyeball-im = { workspace = true } futures-util = { workspace = true } diff --git a/crates/matrix-sdk-base/src/client.rs b/crates/matrix-sdk-base/src/client.rs index 38a572edfde..18b903b086b 100644 --- a/crates/matrix-sdk-base/src/client.rs +++ b/crates/matrix-sdk-base/src/client.rs @@ -542,7 +542,7 @@ impl BaseClient { // // It finds the appropriate `RoomInfo`, allowing the caller to modify it, and // save it in the correct place. - fn on_rooms_infos( + fn on_room_info( room_id: &RoomId, changes: &mut StateChanges, client: &BaseClient, @@ -568,17 +568,24 @@ impl BaseClient { } } + // Handle new events. for raw_event in events { if let Ok(event) = raw_event.deserialize() { changes.add_room_account_data(room_id, event.clone(), raw_event.clone()); match event { AnyRoomAccountDataEvent::MarkedUnread(event) => { - on_rooms_infos(room_id, changes, self, |room_info| { + on_room_info(room_id, changes, self, |room_info| { room_info.base_info.is_marked_unread = event.content.unread; }); } + AnyRoomAccountDataEvent::Tag(event) => { + on_room_info(room_id, changes, self, |room_info| { + room_info.base_info.handle_notable_tags(&event.content.tags); + }); + } + // Nothing. _ => {} } diff --git a/crates/matrix-sdk-base/src/rooms/mod.rs b/crates/matrix-sdk-base/src/rooms/mod.rs index 041b2c26968..20fb726877f 100644 --- a/crates/matrix-sdk-base/src/rooms/mod.rs +++ b/crates/matrix-sdk-base/src/rooms/mod.rs @@ -30,6 +30,7 @@ use ruma::{ tombstone::RoomTombstoneEventContent, topic::RoomTopicEventContent, }, + tag::{TagName, Tags}, AnyStrippedStateEvent, AnySyncStateEvent, EmptyStateKey, RedactContent, RedactedStateEventContent, StaticStateEventContent, SyncStateEvent, }, @@ -107,6 +108,12 @@ pub struct BaseRoomInfo { // Whether this room has been manually marked as unread #[serde(default)] pub(crate) is_marked_unread: bool, + /// Some notable tags. + /// + /// We are not interested by all the tags. Some tags are more important than + /// others, and this field collects them. + #[serde(skip_serializing_if = "NotableTags::is_empty", default)] + pub(crate) notable_tags: NotableTags, } impl BaseRoomInfo { @@ -288,6 +295,36 @@ impl BaseRoomInfo { self.rtc_member.retain(|_, member_event| member_event.event_id() != Some(redacts)); } } + + pub fn handle_notable_tags(&mut self, tags: &Tags) { + let mut notable_tags = NotableTags::empty(); + + if tags.contains_key(&TagName::Favorite) { + notable_tags.insert(NotableTags::FAVOURITE); + } + + if tags.contains_key(&TagName::LowPriority) { + notable_tags.insert(NotableTags::LOW_PRIORITY); + } + + self.notable_tags = notable_tags; + } +} + +bitflags! { + /// Notable tags, i.e. subset of tags that we are more interested by. + /// + /// We are not interested by all the tags. Some tags are more important than + /// others, and this struct describe them. + #[repr(transparent)] + #[derive(Debug, Default, Clone, Copy, Deserialize, Serialize)] + pub struct NotableTags: u8 { + /// The `m.favourite` tag. + const FAVOURITE = 0b0000_0001; + + /// THe `m.lowpriority` tag. + const LOW_PRIORITY = 0b0000_0010; + } } trait OptionExt { @@ -321,6 +358,7 @@ impl Default for BaseRoomInfo { topic: None, rtc_member: BTreeMap::new(), is_marked_unread: false, + notable_tags: NotableTags::empty(), } } } diff --git a/crates/matrix-sdk-base/src/rooms/normal.rs b/crates/matrix-sdk-base/src/rooms/normal.rs index 240382b040d..a99d76818da 100644 --- a/crates/matrix-sdk-base/src/rooms/normal.rs +++ b/crates/matrix-sdk-base/src/rooms/normal.rs @@ -58,7 +58,7 @@ use tracing::{debug, field::debug, info, instrument, trace, warn}; use super::{ members::{MemberInfo, MemberRoomInfo}, - BaseRoomInfo, DisplayName, RoomCreateWithCreatorEventContent, RoomMember, + BaseRoomInfo, DisplayName, NotableTags, RoomCreateWithCreatorEventContent, RoomMember, }; #[cfg(feature = "experimental-sliding-sync")] use crate::latest_event::LatestEvent; diff --git a/crates/matrix-sdk-base/src/store/migration_helpers.rs b/crates/matrix-sdk-base/src/store/migration_helpers.rs index 695f75d3a8a..9222ff1c49f 100644 --- a/crates/matrix-sdk-base/src/store/migration_helpers.rs +++ b/crates/matrix-sdk-base/src/store/migration_helpers.rs @@ -44,7 +44,7 @@ use crate::{ deserialized_responses::SyncOrStrippedState, rooms::{ normal::{RoomSummary, SyncInfo}, - BaseRoomInfo, + BaseRoomInfo, NotableTags, }, sync::UnreadNotificationsCount, MinimalStateEvent, OriginalMinimalStateEvent, RoomInfo, RoomState, @@ -205,6 +205,7 @@ impl BaseRoomInfoV1 { topic, rtc_member: BTreeMap::new(), is_marked_unread: false, + notable_tags: NotableTags::empty(), }) } } From 71f4af9cdd75e789d07c39c9b1a379f105b46c45 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 8 Feb 2024 15:17:17 +0100 Subject: [PATCH 04/13] feat(base,sdk,ffi): Remove `RoomNotableTags`. The previous patch has introduced the new `NotableTags` type to replace the `RoomNotableTags`. This patch removes the latter. This patch keeps the `Room::set_is_favorite` and `::set_is_low_priority` methods. However, this patch adds the `Room::is_favourite` and `::is_low_priority` methods, with the consequence of entirely hiding the notable tags type from the public API. --- bindings/matrix-sdk-ffi/src/room.rs | 22 +------- crates/matrix-sdk-base/src/client.rs | 21 +------- crates/matrix-sdk-base/src/lib.rs | 2 +- crates/matrix-sdk-base/src/rooms/mod.rs | 2 +- crates/matrix-sdk-base/src/rooms/normal.rs | 59 +++++----------------- crates/matrix-sdk/src/lib.rs | 4 +- crates/matrix-sdk/src/room/mod.rs | 8 ++- 7 files changed, 25 insertions(+), 93 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/room.rs b/bindings/matrix-sdk-ffi/src/room.rs index 79ef6b6fdd7..e2c3b236335 100644 --- a/bindings/matrix-sdk-ffi/src/room.rs +++ b/bindings/matrix-sdk-ffi/src/room.rs @@ -3,7 +3,7 @@ use std::{convert::TryFrom, sync::Arc}; use anyhow::{Context, Result}; use matrix_sdk::{ room::{power_levels::RoomPowerLevelChanges, Room as SdkRoom}, - RoomMemberships, RoomNotableTags, RoomState, + RoomMemberships, RoomState, }; use matrix_sdk_ui::timeline::RoomExt; use mime::Mime; @@ -256,21 +256,6 @@ impl Room { }))) } - pub fn subscribe_to_notable_tags( - self: Arc, - listener: Box, - ) -> Arc { - Arc::new(TaskHandle::new(RUNTIME.spawn(async move { - let (initial, mut subscriber) = self.inner.notable_tags_stream().await; - // Send the initial value - listener.call(initial); - // Then wait for changes - while let Some(notable_tags) = subscriber.next().await { - listener.call(notable_tags); - } - }))) - } - pub async fn set_is_favorite( &self, is_favorite: bool, @@ -626,11 +611,6 @@ pub trait TypingNotificationsListener: Sync + Send { fn call(&self, typing_user_ids: Vec); } -#[uniffi::export(callback_interface)] -pub trait RoomNotableTagsListener: Sync + Send { - fn call(&self, notable_tags: RoomNotableTags); -} - #[derive(uniffi::Object)] pub struct RoomMembersIterator { chunk_iterator: ChunkIterator, diff --git a/crates/matrix-sdk-base/src/client.rs b/crates/matrix-sdk-base/src/client.rs index 18b903b086b..b9462ba2e10 100644 --- a/crates/matrix-sdk-base/src/client.rs +++ b/crates/matrix-sdk-base/src/client.rs @@ -44,7 +44,7 @@ use ruma::{ }, AnyGlobalAccountDataEvent, AnyRoomAccountDataEvent, AnyStrippedStateEvent, AnySyncEphemeralRoomEvent, AnySyncMessageLikeEvent, AnySyncStateEvent, - AnySyncTimelineEvent, GlobalAccountDataEventType, RoomAccountDataEventType, StateEventType, + AnySyncTimelineEvent, GlobalAccountDataEventType, StateEventType, }, push::{Action, PushConditionRoomCtx, Ruleset}, serde::Raw, @@ -68,7 +68,7 @@ use crate::{ StateChanges, StateStoreDataKey, StateStoreDataValue, StateStoreExt, Store, StoreConfig, }, sync::{JoinedRoom, LeftRoom, Notification, Rooms, SyncResponse, Timeline}, - RoomNotableTags, RoomStateFilter, SessionMeta, + RoomStateFilter, SessionMeta, }; #[cfg(feature = "e2e-encryption")] use crate::{error::Error, RoomMemberships}; @@ -1035,23 +1035,6 @@ impl BaseClient { room.set_room_info(room_info.clone()) } } - - for (room_id, room_account_data) in &changes.room_account_data { - if let Some(room) = self.store.get_room(room_id) { - let tags = room_account_data.get(&RoomAccountDataEventType::Tag).and_then(|r| { - match r.deserialize() { - Ok(AnyRoomAccountDataEvent::Tag(event)) => Some(event.content.tags), - Err(e) => { - warn!("Room account data tag event failed to deserialize : {e}"); - None - } - Ok(_) => None, - } - }); - let notable_tags = RoomNotableTags::new(tags); - room.set_notable_tags(notable_tags) - } - } } /// Receive a get member events response and convert it to a deserialized diff --git a/crates/matrix-sdk-base/src/lib.rs b/crates/matrix-sdk-base/src/lib.rs index dd5218ef351..e770799ac82 100644 --- a/crates/matrix-sdk-base/src/lib.rs +++ b/crates/matrix-sdk-base/src/lib.rs @@ -51,7 +51,7 @@ pub use matrix_sdk_crypto as crypto; pub use once_cell; pub use rooms::{ DisplayName, Room, RoomCreateWithCreatorEventContent, RoomInfo, RoomMember, RoomMemberships, - RoomNotableTags, RoomState, RoomStateFilter, + RoomState, RoomStateFilter, }; pub use store::{StateChanges, StateStore, StateStoreDataKey, StateStoreDataValue, StoreError}; pub use utils::{ diff --git a/crates/matrix-sdk-base/src/rooms/mod.rs b/crates/matrix-sdk-base/src/rooms/mod.rs index 20fb726877f..71b05e73ce2 100644 --- a/crates/matrix-sdk-base/src/rooms/mod.rs +++ b/crates/matrix-sdk-base/src/rooms/mod.rs @@ -11,7 +11,7 @@ use std::{ use bitflags::bitflags; pub use members::RoomMember; -pub use normal::{Room, RoomInfo, RoomNotableTags, RoomState, RoomStateFilter}; +pub use normal::{Room, RoomInfo, RoomState, RoomStateFilter}; use ruma::{ assign, events::{ diff --git a/crates/matrix-sdk-base/src/rooms/normal.rs b/crates/matrix-sdk-base/src/rooms/normal.rs index a99d76818da..93e5e1d2d02 100644 --- a/crates/matrix-sdk-base/src/rooms/normal.rs +++ b/crates/matrix-sdk-base/src/rooms/normal.rs @@ -44,7 +44,7 @@ use ruma::{ redaction::SyncRoomRedactionEvent, tombstone::RoomTombstoneEventContent, }, - tag::{TagName, Tags}, + tag::Tags, AnyRoomAccountDataEvent, AnyStrippedStateEvent, AnySyncStateEvent, RoomAccountDataEventType, }, @@ -90,9 +90,6 @@ pub struct Room { /// to disk but held in memory. #[cfg(all(feature = "e2e-encryption", feature = "experimental-sliding-sync"))] pub latest_encrypted_events: Arc>>>, - /// Observable of when some notable tags are set or removed from the room - /// account data. - notable_tags: SharedObservable, } /// The room summary containing member counts and members that should be used to @@ -166,7 +163,6 @@ impl Room { latest_encrypted_events: Arc::new(SyncRwLock::new(RingBuffer::new( Self::MAX_ENCRYPTED_EVENTS, ))), - notable_tags: Default::default(), } } @@ -654,26 +650,6 @@ impl Room { self.inner.set(room_info); } - /// Update the inner observable with the given [`RoomNotableTags`], and - /// notify subscribers. - pub fn set_notable_tags(&self, notable_tags: RoomNotableTags) { - self.notable_tags.set(notable_tags); - } - - /// Returns the current [`RoomNotableTags`] and subscribe to changes. - pub async fn notable_tags_stream(&self) -> (RoomNotableTags, Subscriber) { - (self.current_notable_tags().await, self.notable_tags.subscribe()) - } - - /// Return the current [`RoomNotableTags`] extracted from store. - pub async fn current_notable_tags(&self) -> RoomNotableTags { - let current_tags = self.tags().await.unwrap_or_else(|e| { - warn!("Failed to get tags from store: {}", e); - None - }); - RoomNotableTags::new(current_tags) - } - /// Get the `RoomMember` with the given `user_id`. /// /// Returns `None` if the member was never part of this room, otherwise @@ -756,6 +732,14 @@ impl Room { } } + pub fn is_favourite(&self) -> bool { + self.inner.read().base_info.notable_tags.contains(NotableTags::FAVOURITE) + } + + pub fn is_low_priority(&self) -> bool { + self.inner.read().base_info.notable_tags.contains(NotableTags::LOW_PRIORITY) + } + /// Get the receipt as an `OwnedEventId` and `Receipt` tuple for the given /// `receipt_type`, `thread` and `user_id` in this room. pub async fn load_user_receipt( @@ -850,27 +834,6 @@ pub(crate) enum SyncInfo { FullySynced, } -/// Holds information computed from the room account data `m.tag` events. -#[derive(Clone, Debug, Default)] -#[cfg_attr(feature = "uniffi", derive(uniffi::Record))] -pub struct RoomNotableTags { - /// Whether or not the room is marked as favorite. - pub is_favorite: bool, - /// Whether or not the room is marked as low priority. - pub is_low_priority: bool, -} - -impl RoomNotableTags { - /// Computes the provided tags to create a [`RoomNotableTags`] instance. - pub fn new(tags: Option) -> Self { - let tags = tags.as_ref(); - RoomNotableTags { - is_favorite: tags.map_or(false, |tag| tag.contains_key(&TagName::Favorite)), - is_low_priority: tags.map_or(false, |tag| tag.contains_key(&TagName::LowPriority)), - } - } -} - impl RoomInfo { #[doc(hidden)] // used by store tests, otherwise it would be pub(crate) pub fn new(room_id: &RoomId, room_state: RoomState) -> Self { @@ -1347,7 +1310,7 @@ mod tests { use crate::latest_event::LatestEvent; use crate::{ store::{MemoryStore, StateChanges, StateStore}, - DisplayName, MinimalStateEvent, OriginalMinimalStateEvent, RoomNotableTags, + DisplayName, MinimalStateEvent, OriginalMinimalStateEvent, }; #[test] @@ -1501,6 +1464,7 @@ mod tests { assert!(info.base_info.topic.is_none()); } + /* #[async_test] async fn when_set_notable_tags_is_called_then_notable_tags_subscriber_is_updated() { let (_, room) = make_room(RoomState::Joined); @@ -1544,6 +1508,7 @@ mod tests { let (initial_notable_tags, _) = room.notable_tags_stream().await; assert!(initial_notable_tags.is_favorite); } + */ fn make_room(room_type: RoomState) -> (Arc, Room) { let store = Arc::new(MemoryStore::new()); diff --git a/crates/matrix-sdk/src/lib.rs b/crates/matrix-sdk/src/lib.rs index bb9b20b3c94..1087f837b07 100644 --- a/crates/matrix-sdk/src/lib.rs +++ b/crates/matrix-sdk/src/lib.rs @@ -24,8 +24,8 @@ pub use matrix_sdk_base::{ deserialized_responses, store::{DynStateStore, MemoryStore, StateStoreExt}, DisplayName, Room as BaseRoom, RoomCreateWithCreatorEventContent, RoomInfo, - RoomMember as BaseRoomMember, RoomMemberships, RoomNotableTags, RoomState, SessionMeta, - StateChanges, StateStore, StoreError, + RoomMember as BaseRoomMember, RoomMemberships, RoomState, SessionMeta, StateChanges, + StateStore, StoreError, }; pub use matrix_sdk_common::*; pub use reqwest; diff --git a/crates/matrix-sdk/src/room/mod.rs b/crates/matrix-sdk/src/room/mod.rs index dda067cb1db..d1972b912a0 100644 --- a/crates/matrix-sdk/src/room/mod.rs +++ b/crates/matrix-sdk/src/room/mod.rs @@ -979,8 +979,10 @@ impl Room { pub async fn set_is_favorite(&self, is_favorite: bool, tag_order: Option) -> Result<()> { if is_favorite { let tag_info = assign!(TagInfo::new(), { order: tag_order }); + self.set_tag(TagName::Favorite, tag_info).await?; - if self.current_notable_tags().await.is_low_priority { + + if self.is_low_priority() { self.remove_tag(TagName::LowPriority).await?; } } else { @@ -1004,8 +1006,10 @@ impl Room { ) -> Result<()> { if is_low_priority { let tag_info = assign!(TagInfo::new(), { order: tag_order }); + self.set_tag(TagName::LowPriority, tag_info).await?; - if self.current_notable_tags().await.is_favorite { + + if self.is_favourite() { self.remove_tag(TagName::Favorite).await?; } } else { From 31ba7b82d8bce3fef9b5e331c2bec6fa4d03ec2a Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 8 Feb 2024 15:25:41 +0100 Subject: [PATCH 05/13] doc(sdk,base,ffi): Improve documentation and rename `favorite` to `favourite`. The Matrix specification uses the `m.favourite` orthography. Let's use the same in our code. So `set_is_favorite` becomes `set_is_favourite`. This patch updates this in various places for the sake of consistency. --- bindings/matrix-sdk-ffi/src/room.rs | 4 ++-- crates/matrix-sdk-base/src/rooms/mod.rs | 4 ++-- crates/matrix-sdk/src/room/mod.rs | 22 ++++++++++++---------- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/room.rs b/bindings/matrix-sdk-ffi/src/room.rs index e2c3b236335..8b76f541925 100644 --- a/bindings/matrix-sdk-ffi/src/room.rs +++ b/bindings/matrix-sdk-ffi/src/room.rs @@ -256,12 +256,12 @@ impl Room { }))) } - pub async fn set_is_favorite( + pub async fn set_is_favourite( &self, is_favorite: bool, tag_order: Option, ) -> Result<(), ClientError> { - self.inner.set_is_favorite(is_favorite, tag_order).await?; + self.inner.set_is_favourite(is_favorite, tag_order).await?; Ok(()) } diff --git a/crates/matrix-sdk-base/src/rooms/mod.rs b/crates/matrix-sdk-base/src/rooms/mod.rs index 71b05e73ce2..9ee1ed2305d 100644 --- a/crates/matrix-sdk-base/src/rooms/mod.rs +++ b/crates/matrix-sdk-base/src/rooms/mod.rs @@ -105,7 +105,7 @@ 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 + /// Whether this room has been manually marked as unread. #[serde(default)] pub(crate) is_marked_unread: bool, /// Some notable tags. @@ -318,7 +318,7 @@ bitflags! { /// others, and this struct describe them. #[repr(transparent)] #[derive(Debug, Default, Clone, Copy, Deserialize, Serialize)] - pub struct NotableTags: u8 { + pub(crate) struct NotableTags: u8 { /// The `m.favourite` tag. const FAVOURITE = 0b0000_0001; diff --git a/crates/matrix-sdk/src/room/mod.rs b/crates/matrix-sdk/src/room/mod.rs index d1972b912a0..0548184e4fa 100644 --- a/crates/matrix-sdk/src/room/mod.rs +++ b/crates/matrix-sdk/src/room/mod.rs @@ -968,16 +968,17 @@ impl Room { self.client.send(request, None).await } - /// Update the is_favorite flag from the room by calling set_tag or - /// remove_tag method on `m.favourite` tag. - /// If `is_favorite` is `true`, and `m.low_priority` tag is set on the room, - /// the tag will be removed too. + /// Add or remove the `m.favourite` flag for this room. + /// + /// If `is_favourite` is `true`, and the `m.low_priority` tag is set on the + /// room, the tag will be removed too. + /// /// # Arguments /// - /// * `is_favorite` - Whether to mark this room as favorite or not. + /// * `is_favourite` - Whether to mark this room as favourite. /// * `tag_order` - The order of the tag if any. - pub async fn set_is_favorite(&self, is_favorite: bool, tag_order: Option) -> Result<()> { - if is_favorite { + pub async fn set_is_favourite(&self, is_favourite: bool, tag_order: Option) -> Result<()> { + if is_favourite { let tag_info = assign!(TagInfo::new(), { order: tag_order }); self.set_tag(TagName::Favorite, tag_info).await?; @@ -991,10 +992,11 @@ impl Room { Ok(()) } - /// Update the is_low_priority flag from the room by calling set_tag or - /// remove_tag method on `m.low_priority` tag. - /// If `is_low_priority` is `true`, and `m.favourite` tag is set on the + /// Add or remove the `m.lowpriority` flag for this room. + /// + /// If `is_low_priority` is `true`, and the `m.favourite` tag is set on the /// room, the tag will be removed too. + /// /// # Arguments /// /// * `is_low_priority` - Whether to mark this room as low_priority or not. From 8b298dfd2f4689d4167d18aeee95b6e02a196f81 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 8 Feb 2024 15:27:48 +0100 Subject: [PATCH 06/13] chore(base): Rename `NotableTags` to `RoomNotableTags`. Now that `NotableTags` has replaced `RoomNotableTags` entirely, this patch can rename `NotableTags` to `RoomNotableTags` as it's a better name for it. Note that this type is private to `matrix_sdk_base`, so it's fine to rename it. --- crates/matrix-sdk-base/src/rooms/mod.rs | 14 +++++++------- crates/matrix-sdk-base/src/rooms/normal.rs | 6 +++--- .../matrix-sdk-base/src/store/migration_helpers.rs | 4 ++-- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/crates/matrix-sdk-base/src/rooms/mod.rs b/crates/matrix-sdk-base/src/rooms/mod.rs index 9ee1ed2305d..eb7c0ce4305 100644 --- a/crates/matrix-sdk-base/src/rooms/mod.rs +++ b/crates/matrix-sdk-base/src/rooms/mod.rs @@ -112,8 +112,8 @@ pub struct BaseRoomInfo { /// /// We are not interested by all the tags. Some tags are more important than /// others, and this field collects them. - #[serde(skip_serializing_if = "NotableTags::is_empty", default)] - pub(crate) notable_tags: NotableTags, + #[serde(skip_serializing_if = "RoomNotableTags::is_empty", default)] + pub(crate) notable_tags: RoomNotableTags, } impl BaseRoomInfo { @@ -297,14 +297,14 @@ impl BaseRoomInfo { } pub fn handle_notable_tags(&mut self, tags: &Tags) { - let mut notable_tags = NotableTags::empty(); + let mut notable_tags = RoomNotableTags::empty(); if tags.contains_key(&TagName::Favorite) { - notable_tags.insert(NotableTags::FAVOURITE); + notable_tags.insert(RoomNotableTags::FAVOURITE); } if tags.contains_key(&TagName::LowPriority) { - notable_tags.insert(NotableTags::LOW_PRIORITY); + notable_tags.insert(RoomNotableTags::LOW_PRIORITY); } self.notable_tags = notable_tags; @@ -318,7 +318,7 @@ bitflags! { /// others, and this struct describe them. #[repr(transparent)] #[derive(Debug, Default, Clone, Copy, Deserialize, Serialize)] - pub(crate) struct NotableTags: u8 { + pub(crate) struct RoomNotableTags: u8 { /// The `m.favourite` tag. const FAVOURITE = 0b0000_0001; @@ -358,7 +358,7 @@ impl Default for BaseRoomInfo { topic: None, rtc_member: BTreeMap::new(), is_marked_unread: false, - notable_tags: NotableTags::empty(), + notable_tags: RoomNotableTags::empty(), } } } diff --git a/crates/matrix-sdk-base/src/rooms/normal.rs b/crates/matrix-sdk-base/src/rooms/normal.rs index 93e5e1d2d02..745d3039bbf 100644 --- a/crates/matrix-sdk-base/src/rooms/normal.rs +++ b/crates/matrix-sdk-base/src/rooms/normal.rs @@ -58,7 +58,7 @@ use tracing::{debug, field::debug, info, instrument, trace, warn}; use super::{ members::{MemberInfo, MemberRoomInfo}, - BaseRoomInfo, DisplayName, NotableTags, RoomCreateWithCreatorEventContent, RoomMember, + BaseRoomInfo, DisplayName, RoomCreateWithCreatorEventContent, RoomMember, RoomNotableTags, }; #[cfg(feature = "experimental-sliding-sync")] use crate::latest_event::LatestEvent; @@ -733,11 +733,11 @@ impl Room { } pub fn is_favourite(&self) -> bool { - self.inner.read().base_info.notable_tags.contains(NotableTags::FAVOURITE) + self.inner.read().base_info.notable_tags.contains(RoomNotableTags::FAVOURITE) } pub fn is_low_priority(&self) -> bool { - self.inner.read().base_info.notable_tags.contains(NotableTags::LOW_PRIORITY) + self.inner.read().base_info.notable_tags.contains(RoomNotableTags::LOW_PRIORITY) } /// Get the receipt as an `OwnedEventId` and `Receipt` tuple for the given diff --git a/crates/matrix-sdk-base/src/store/migration_helpers.rs b/crates/matrix-sdk-base/src/store/migration_helpers.rs index 9222ff1c49f..d86a1fada9f 100644 --- a/crates/matrix-sdk-base/src/store/migration_helpers.rs +++ b/crates/matrix-sdk-base/src/store/migration_helpers.rs @@ -44,7 +44,7 @@ use crate::{ deserialized_responses::SyncOrStrippedState, rooms::{ normal::{RoomSummary, SyncInfo}, - BaseRoomInfo, NotableTags, + BaseRoomInfo, RoomNotableTags, }, sync::UnreadNotificationsCount, MinimalStateEvent, OriginalMinimalStateEvent, RoomInfo, RoomState, @@ -205,7 +205,7 @@ impl BaseRoomInfoV1 { topic, rtc_member: BTreeMap::new(), is_marked_unread: false, - notable_tags: NotableTags::empty(), + notable_tags: RoomNotableTags::empty(), }) } } From 75454de284bd4e5a0dff33055cc9552bd7bfec40 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 8 Feb 2024 15:52:39 +0100 Subject: [PATCH 07/13] test(base): Test `Room::is_favourite` and `::is_low_priority`. This patch tests the `is_favourite` and `is_low_priority` methods. --- crates/matrix-sdk-base/src/rooms/mod.rs | 36 ++++- crates/matrix-sdk-base/src/rooms/normal.rs | 161 ++++++++++++++++----- 2 files changed, 162 insertions(+), 35 deletions(-) diff --git a/crates/matrix-sdk-base/src/rooms/mod.rs b/crates/matrix-sdk-base/src/rooms/mod.rs index eb7c0ce4305..d2e429e78e0 100644 --- a/crates/matrix-sdk-base/src/rooms/mod.rs +++ b/crates/matrix-sdk-base/src/rooms/mod.rs @@ -562,7 +562,11 @@ impl RoomMemberships { #[cfg(test)] mod tests { - use super::{calculate_room_name, DisplayName}; + use std::ops::Not; + + use ruma::events::tag::{TagInfo, TagName, Tags}; + + use super::{calculate_room_name, BaseRoomInfo, DisplayName, RoomNotableTags}; #[test] fn test_calculate_room_name() { @@ -596,4 +600,34 @@ mod tests { actual = calculate_room_name(1, 0, vec!["a", "b", "c"]); assert_eq!(DisplayName::EmptyWas("a, b, c".to_owned()), actual); } + + #[test] + fn test_handle_notable_tags_favourite() { + let mut base_room_info = BaseRoomInfo::default(); + + let mut tags = Tags::new(); + tags.insert(TagName::Favorite, TagInfo::default()); + + assert!(base_room_info.notable_tags.contains(RoomNotableTags::FAVOURITE).not()); + base_room_info.handle_notable_tags(&tags); + assert!(base_room_info.notable_tags.contains(RoomNotableTags::FAVOURITE)); + tags.clear(); + base_room_info.handle_notable_tags(&tags); + assert!(base_room_info.notable_tags.contains(RoomNotableTags::FAVOURITE).not()); + } + + #[test] + fn test_handle_notable_tags_low_priority() { + let mut base_room_info = BaseRoomInfo::default(); + + let mut tags = Tags::new(); + tags.insert(TagName::LowPriority, TagInfo::default()); + + assert!(base_room_info.notable_tags.contains(RoomNotableTags::LOW_PRIORITY).not()); + base_room_info.handle_notable_tags(&tags); + assert!(base_room_info.notable_tags.contains(RoomNotableTags::LOW_PRIORITY)); + tags.clear(); + base_room_info.handle_notable_tags(&tags); + assert!(base_room_info.notable_tags.contains(RoomNotableTags::LOW_PRIORITY).not()); + } } diff --git a/crates/matrix-sdk-base/src/rooms/normal.rs b/crates/matrix-sdk-base/src/rooms/normal.rs index 745d3039bbf..7d46e93b321 100644 --- a/crates/matrix-sdk-base/src/rooms/normal.rs +++ b/crates/matrix-sdk-base/src/rooms/normal.rs @@ -1268,8 +1268,7 @@ impl RoomStateFilter { #[cfg(test)] mod tests { use std::{ - collections::BTreeMap, - ops::Sub, + ops::{Not, Sub}, str::FromStr, sync::Arc, time::{Duration, SystemTime}, @@ -1464,51 +1463,145 @@ mod tests { assert!(info.base_info.topic.is_none()); } - /* #[async_test] - async fn when_set_notable_tags_is_called_then_notable_tags_subscriber_is_updated() { - let (_, room) = make_room(RoomState::Joined); - let (_, mut notable_tags_subscriber) = room.notable_tags_stream().await; + async fn test_is_favourite() { + // Given a room, + let client = BaseClient::new(); - stream_assert::assert_pending!(notable_tags_subscriber); + client + .set_session_meta(SessionMeta { + user_id: user_id!("@alice:example.org").into(), + device_id: ruma::device_id!("AYEAYEAYE").into(), + }) + .await + .unwrap(); - let notable_tags = RoomNotableTags::new(None); - room.set_notable_tags(notable_tags); + let room_id = room_id!("!test:localhost"); + let room = client.get_or_create_room(room_id, RoomState::Joined); - use futures_util::FutureExt as _; - assert!(notable_tags_subscriber.next().now_or_never().is_some()); - stream_assert::assert_pending!(notable_tags_subscriber); - } + // Sanity checks to ensure the room isn't marked as favourite. + assert!(room.is_favourite().not()); - #[test] - fn when_tags_has_favorite_tag_then_notable_tags_is_favorite_is_true() { - let tags = BTreeMap::from([(TagName::Favorite, TagInfo::new())]); - let notable_tags = RoomNotableTags::new(Some(tags)); - assert!(notable_tags.is_favorite); - } + // Subscribe to the `RoomInfo`. + let mut room_info_subscriber = room.subscribe_info(); - #[test] - fn when_tags_has_no_tags_then_notable_tags_is_favorite_is_false() { - let notable_tags = RoomNotableTags::new(None); - assert!(!notable_tags.is_favorite); + assert_pending!(room_info_subscriber); + + // Create the tag. + let tag_raw = Raw::new(&json!({ + "content": { + "tags": { + "m.favourite": { + "order": 0.0 + }, + }, + }, + "type": "m.tag", + })) + .unwrap() + .cast(); + + // When the new tag is handled and applied. + let mut changes = StateChanges::default(); + client.handle_room_account_data(room_id, &[tag_raw], &mut changes).await; + client.apply_changes(&changes); + + // The `RoomInfo` is getting notified. + assert_ready!(room_info_subscriber); + assert_pending!(room_info_subscriber); + + // The room is now marked as favourite. + assert!(room.is_favourite()); + + // Now, let's remove the tag. + let tag_raw = Raw::new(&json!({ + "content": { + "tags": {}, + }, + "type": "m.tag" + })) + .unwrap() + .cast(); + client.handle_room_account_data(room_id, &[tag_raw], &mut changes).await; + client.apply_changes(&changes); + + // The `RoomInfo` is getting notified. + assert_ready!(room_info_subscriber); + assert_pending!(room_info_subscriber); + + // The room is now marked as _not_ favourite. + assert!(room.is_favourite().not()); } #[async_test] - async fn when_tags_are_inserted_in_room_account_data_then_initial_notable_tags_is_updated() { - let (store, room) = make_room(RoomState::Joined); - let mut changes = StateChanges::new("".to_owned()); + async fn test_is_low_priority() { + // Given a room, + let client = BaseClient::new(); + + client + .set_session_meta(SessionMeta { + user_id: user_id!("@alice:example.org").into(), + device_id: ruma::device_id!("AYEAYEAYE").into(), + }) + .await + .unwrap(); + + let room_id = room_id!("!test:localhost"); + let room = client.get_or_create_room(room_id, RoomState::Joined); - let tag_json: &Value = &test_json::TAG; - let tag_raw = Raw::new(tag_json).unwrap().cast(); - let tag_event = tag_raw.deserialize().unwrap(); - changes.add_room_account_data(room.room_id(), tag_event, tag_raw); + // Sanity checks to ensure the room isn't marked as low priority. + assert!(!room.is_low_priority()); - store.save_changes(&changes).await.unwrap(); + // Subscribe to the `RoomInfo`. + let mut room_info_subscriber = room.subscribe_info(); + + assert_pending!(room_info_subscriber); + + // Create the tag. + let tag_raw = Raw::new(&json!({ + "content": { + "tags": { + "m.lowpriority": { + "order": 0.0 + }, + } + }, + "type": "m.tag" + })) + .unwrap() + .cast(); + + // When the new tag is handled and applied. + let mut changes = StateChanges::default(); + client.handle_room_account_data(room_id, &[tag_raw], &mut changes).await; + client.apply_changes(&changes); + + // The `RoomInfo` is getting notified. + assert_ready!(room_info_subscriber); + assert_pending!(room_info_subscriber); + + // The room is now marked as low priority. + assert!(room.is_low_priority()); + + // Now, let's remove the tag. + let tag_raw = Raw::new(&json!({ + "content": { + "tags": {}, + }, + "type": "m.tag" + })) + .unwrap() + .cast(); + client.handle_room_account_data(room_id, &[tag_raw], &mut changes).await; + client.apply_changes(&changes); + + // The `RoomInfo` is getting notified. + assert_ready!(room_info_subscriber); + assert_pending!(room_info_subscriber); - let (initial_notable_tags, _) = room.notable_tags_stream().await; - assert!(initial_notable_tags.is_favorite); + // The room is now marked as _not_ low priority. + assert!(room.is_low_priority().not()); } - */ fn make_room(room_type: RoomState) -> (Arc, Room) { let store = Arc::new(MemoryStore::new()); From e6dadf2b0def3e6c99bccddfc957a38cd4606c84 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 8 Feb 2024 15:53:11 +0100 Subject: [PATCH 08/13] chore(base): Clean up tests. --- crates/matrix-sdk-base/src/rooms/normal.rs | 20 +++++++------- .../matrix-sdk/tests/integration/room/tags.rs | 26 ++++++++----------- 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/crates/matrix-sdk-base/src/rooms/normal.rs b/crates/matrix-sdk-base/src/rooms/normal.rs index 7d46e93b321..4552774e1d5 100644 --- a/crates/matrix-sdk-base/src/rooms/normal.rs +++ b/crates/matrix-sdk-base/src/rooms/normal.rs @@ -1277,7 +1277,7 @@ mod tests { use assign::assign; #[cfg(feature = "experimental-sliding-sync")] use matrix_sdk_common::deserialized_responses::SyncTimelineEvent; - use matrix_sdk_test::{async_test, test_json, ALICE, BOB, CAROL}; + use matrix_sdk_test::{async_test, ALICE, BOB, CAROL}; use ruma::{ api::client::sync::sync_events::v3::RoomSummary as RumaSummary, events::{ @@ -1293,14 +1293,14 @@ mod tests { }, name::RoomNameEventContent, }, - tag::{TagInfo, TagName}, AnySyncStateEvent, StateEventType, StateUnsigned, SyncStateEvent, }, room_alias_id, room_id, serde::Raw, user_id, MilliSecondsSinceUnixEpoch, OwnedEventId, OwnedUserId, UserId, }; - use serde_json::{json, Value}; + use serde_json::json; + use stream_assert::{assert_pending, assert_ready}; #[cfg(feature = "experimental-sliding-sync")] use super::SyncInfo; @@ -1309,7 +1309,7 @@ mod tests { use crate::latest_event::LatestEvent; use crate::{ store::{MemoryStore, StateChanges, StateStore}, - DisplayName, MinimalStateEvent, OriginalMinimalStateEvent, + BaseClient, DisplayName, MinimalStateEvent, OriginalMinimalStateEvent, SessionMeta, }; #[test] @@ -1870,10 +1870,11 @@ mod tests { #[cfg(feature = "experimental-sliding-sync")] async fn test_setting_the_latest_event_doesnt_cause_a_room_info_update() { // Given a room, - let client = crate::BaseClient::new(); + + let client = BaseClient::new(); client - .set_session_meta(crate::SessionMeta { + .set_session_meta(SessionMeta { user_id: user_id!("@alice:example.org").into(), device_id: ruma::device_id!("AYEAYEAYE").into(), }) @@ -1898,16 +1899,15 @@ mod tests { room.on_latest_event_decrypted(event.clone(), 0, &mut changes); // The subscriber isn't notified at this point. - stream_assert::assert_pending!(room_info_subscriber); + assert_pending!(room_info_subscriber); // Then updating the room info will store the event, client.apply_changes(&changes); assert_eq!(room.latest_event().unwrap().event_id(), event.event_id()); // And wake up the subscriber. - use futures_util::FutureExt as _; - assert!(room_info_subscriber.next().now_or_never().is_some()); - stream_assert::assert_pending!(room_info_subscriber); + assert_ready!(room_info_subscriber); + assert_pending!(room_info_subscriber); } #[test] diff --git a/crates/matrix-sdk/tests/integration/room/tags.rs b/crates/matrix-sdk/tests/integration/room/tags.rs index 46d4460ca92..d6e7ba35c7c 100644 --- a/crates/matrix-sdk/tests/integration/room/tags.rs +++ b/crates/matrix-sdk/tests/integration/room/tags.rs @@ -81,20 +81,20 @@ async fn synced_client_with_room( } #[async_test] -async fn when_set_is_favorite_is_run_with_true_then_set_tag_api_is_called() { +async fn when_set_is_favourite_is_run_with_true_then_set_tag_api_is_called() { let room_id = room_id!("!test:example.org"); let mut ev_builder = SyncResponseBuilder::new(); let (_client, room, server) = synced_client_with_room(&mut ev_builder, room_id).await; mock_tag_api(&server, TagName::Favorite, TagOperation::Set, 1).await; - room.set_is_favorite(true, Option::default()).await.unwrap(); + room.set_is_favourite(true, Option::default()).await.unwrap(); server.verify().await; } #[async_test] -async fn when_set_is_favorite_is_run_with_true_and_low_priority_tag_was_set_then_set_tag_and_remove_tag_apis_are_called( +async fn when_set_is_favourite_is_run_with_true_and_low_priority_tag_was_set_then_set_tag_and_remove_tag_apis_are_called( ) { let room_id = room_id!("!test:example.org"); let mut ev_builder = SyncResponseBuilder::new(); @@ -106,20 +106,20 @@ async fn when_set_is_favorite_is_run_with_true_and_low_priority_tag_was_set_then mock_tag_api(&server, TagName::Favorite, TagOperation::Set, 1).await; mock_tag_api(&server, TagName::LowPriority, TagOperation::Remove, 1).await; - room.set_is_favorite(true, Option::default()).await.unwrap(); + room.set_is_favourite(true, Option::default()).await.unwrap(); server.verify().await; } #[async_test] -async fn when_set_is_favorite_is_run_with_false_then_delete_tag_api_is_called() { +async fn when_set_is_favourite_is_run_with_false_then_delete_tag_api_is_called() { let room_id = room_id!("!test:example.org"); let mut ev_builder = SyncResponseBuilder::new(); let (_client, room, server) = synced_client_with_room(&mut ev_builder, room_id).await; mock_tag_api(&server, TagName::Favorite, TagOperation::Remove, 1).await; - room.set_is_favorite(false, Option::default()).await.unwrap(); + room.set_is_favourite(false, Option::default()).await.unwrap(); server.verify().await; } @@ -169,14 +169,12 @@ async fn when_set_is_low_priority_is_run_with_false_then_delete_tag_api_is_calle } #[async_test] -async fn when_favorite_tag_is_set_in_sync_response_then_notable_tags_is_favorite_is_true() { +async fn when_favorite_tag_is_set_in_sync_response_then_notable_tags_is_favourite_is_true() { let room_id = room_id!("!test:example.org"); let mut ev_builder = SyncResponseBuilder::new(); let (client, room, server) = synced_client_with_room(&mut ev_builder, room_id).await; - let (initial_notable_tags, mut notable_tags_stream) = room.notable_tags_stream().await; - - assert!(!initial_notable_tags.is_favorite); + assert!(!room.is_favourite()); // Ensure the notable tags stream is updated when the favorite tag is set on // sync @@ -184,7 +182,7 @@ async fn when_favorite_tag_is_set_in_sync_response_then_notable_tags_is_favorite mock_sync_with_tags(&server, &mut ev_builder, room_id, tags).await; sync_once(&client, &server).await; - assert!(notable_tags_stream.next().await.unwrap().is_favorite); + assert!(room.is_favourite()); } #[async_test] @@ -193,9 +191,7 @@ async fn when_low_priority_tag_is_set_in_sync_response_then_notable_tags_is_low_ let mut ev_builder = SyncResponseBuilder::new(); let (client, room, server) = synced_client_with_room(&mut ev_builder, room_id).await; - let (initial_notable_tags, mut notable_tags_stream) = room.notable_tags_stream().await; - - assert!(!initial_notable_tags.is_low_priority); + assert!(!room.is_low_priority()); // Ensure the notable tags stream is updated when the low_priority tag is set on // sync @@ -203,5 +199,5 @@ async fn when_low_priority_tag_is_set_in_sync_response_then_notable_tags_is_low_ mock_sync_with_tags(&server, &mut ev_builder, room_id, tags).await; sync_once(&client, &server).await; - assert!(notable_tags_stream.next().await.unwrap().is_low_priority); + assert!(room.is_low_priority()); } From 61c7a96e36a565d2aa2cdb9687e988695c204e54 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 8 Feb 2024 16:43:34 +0100 Subject: [PATCH 09/13] doc(base): Add missing documentation. --- crates/matrix-sdk-base/src/rooms/normal.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/crates/matrix-sdk-base/src/rooms/normal.rs b/crates/matrix-sdk-base/src/rooms/normal.rs index 4552774e1d5..8f5839ee29b 100644 --- a/crates/matrix-sdk-base/src/rooms/normal.rs +++ b/crates/matrix-sdk-base/src/rooms/normal.rs @@ -732,10 +732,17 @@ impl Room { } } + /// Check whether the room is marked as favourite. + /// + /// A room is considered favourite if it has received the `m.favourite` tag. pub fn is_favourite(&self) -> bool { self.inner.read().base_info.notable_tags.contains(RoomNotableTags::FAVOURITE) } + /// Check whether the room is marked as low priority. + /// + /// A room is considered low priority if it has received the `m.lowpriority` + /// tag. pub fn is_low_priority(&self) -> bool { self.inner.read().base_info.notable_tags.contains(RoomNotableTags::LOW_PRIORITY) } From 868bb9a8d9e567142f7c32b3ae9f87b2974cf1eb Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 8 Feb 2024 18:49:32 +0100 Subject: [PATCH 10/13] fix(base) `Client::receive_sync_response` overwrites `RoomInfo`. The workflow inside `Client::receive_sync_response` is a little bit fragile. When `Client::handle_room_account_data` is called, it modifies `RoomInfo`. The problem is that a current `RoomInfo` is being computed before `handle_room_account_data` is called, and saved a couple lines after, resulting in overwriting the modification made by `handle_room_account_data`. This patch ensures that the new `RoomInfo` is saved before `handle_room_account_data` is called. --- crates/matrix-sdk-base/src/client.rs | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/crates/matrix-sdk-base/src/client.rs b/crates/matrix-sdk-base/src/client.rs index b9462ba2e10..e357cc58a74 100644 --- a/crates/matrix-sdk-base/src/client.rs +++ b/crates/matrix-sdk-base/src/client.rs @@ -551,9 +551,9 @@ impl BaseClient { F: Fn(&mut RoomInfo), { // `StateChanges` has the `RoomInfo`. - if let Some(room) = changes.room_infos.get_mut(room_id) { + if let Some(room_info) = changes.room_infos.get_mut(room_id) { // Show time. - on_room_info(room); + on_room_info(room_info); } // The `BaseClient` has the `Room`, which has the `RoomInfo`. else if let Some(room) = client.store.get_room(room_id) { @@ -819,8 +819,8 @@ impl BaseClient { for (room_id, new_info) in response.rooms.join { let room = self.store.get_or_create_room(&room_id, RoomState::Joined); let mut room_info = room.clone_info(); - room_info.mark_as_joined(); + room_info.mark_as_joined(); room_info.update_summary(&new_info.summary); room_info.set_prev_batch(new_info.timeline.prev_batch.as_deref()); room_info.mark_state_fully_synced(); @@ -875,9 +875,19 @@ impl BaseClient { ) .await?; + // Save the new `RoomInfo`. + changes.add_room(room_info); + self.handle_room_account_data(&room_id, &new_info.account_data.events, &mut changes) .await; + // `Self::handle_room_account_data` might have updated the `RoomInfo`. Let's + // fetch it again. + // + // SAFETY: `unwrap` is safe because the `RoomInfo` has been inserted 2 lines + // above. + let mut room_info = changes.room_infos.get(&room_id).unwrap().clone(); + #[cfg(feature = "e2e-encryption")] if room_info.is_encrypted() { if let Some(o) = self.olm_machine().await.as_ref() { @@ -949,12 +959,14 @@ impl BaseClient { ) .await?; + // Save the new `RoomInfo`. + changes.add_room(room_info); + self.handle_room_account_data(&room_id, &new_info.account_data.events, &mut changes) .await; let ambiguity_changes = ambiguity_cache.changes.remove(&room_id).unwrap_or_default(); - changes.add_room(room_info); new_rooms.leave.insert( room_id, LeftRoom::new( @@ -1032,7 +1044,7 @@ impl BaseClient { for (room_id, room_info) in &changes.room_infos { if let Some(room) = self.store.get_room(room_id) { - room.set_room_info(room_info.clone()) + room.set_room_info(room_info.clone()); } } } From 65774aa90ba8c306095ca60dfde8adf122977d0d Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Fri, 9 Feb 2024 09:27:02 +0100 Subject: [PATCH 11/13] chore(base): Remove warnings when `e2e-encryption` is disabled. --- crates/matrix-sdk-base/src/client.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/matrix-sdk-base/src/client.rs b/crates/matrix-sdk-base/src/client.rs index e357cc58a74..7f6943455f1 100644 --- a/crates/matrix-sdk-base/src/client.rs +++ b/crates/matrix-sdk-base/src/client.rs @@ -1407,17 +1407,17 @@ impl Default for BaseClient { #[cfg(test)] mod tests { use matrix_sdk_test::{ - async_test, response_from_file, sync_timeline_event, InvitedRoomBuilder, JoinedRoomBuilder, - LeftRoomBuilder, StrippedStateTestEvent, SyncResponseBuilder, + async_test, response_from_file, sync_timeline_event, InvitedRoomBuilder, LeftRoomBuilder, + StrippedStateTestEvent, SyncResponseBuilder, }; use ruma::{ api::{client as api, IncomingResponse}, - room_id, user_id, RoomId, UserId, + room_id, user_id, UserId, }; use serde_json::json; use super::BaseClient; - use crate::{store::StateStoreExt, DisplayName, Room, RoomState, SessionMeta, StateChanges}; + use crate::{store::StateStoreExt, DisplayName, RoomState, SessionMeta}; #[async_test] async fn invite_after_leaving() { @@ -1565,7 +1565,7 @@ mod tests { assert!(room.latest_event().is_none()); // When I tell it to do some decryption - let mut changes = StateChanges::default(); + let mut changes = crate::StateChanges::default(); client.decrypt_latest_events(&room, &mut changes).await; // Then nothing changed @@ -1594,13 +1594,13 @@ mod tests { #[cfg(feature = "e2e-encryption")] async fn process_room_join( client: &BaseClient, - room_id: &RoomId, + room_id: &ruma::RoomId, event_id: &str, user_id: &UserId, - ) -> Room { + ) -> crate::Room { let mut ev_builder = SyncResponseBuilder::new(); let response = ev_builder - .add_joined_room(JoinedRoomBuilder::new(room_id).add_timeline_event( + .add_joined_room(matrix_sdk_test::JoinedRoomBuilder::new(room_id).add_timeline_event( sync_timeline_event!({ "content": { "displayname": "Alice", From bcb125a09bd7399d40c0044e193c2609b4071bdf Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Fri, 9 Feb 2024 09:44:40 +0100 Subject: [PATCH 12/13] test(sdk): Revisit the tests for notable tags. This patch rewrites all the integration tests for the notable tags. The tests were good, but we could merge some together. The names were too long, they have been shortened. --- .../matrix-sdk/tests/integration/room/tags.rs | 114 +++++++++++------- 1 file changed, 69 insertions(+), 45 deletions(-) diff --git a/crates/matrix-sdk/tests/integration/room/tags.rs b/crates/matrix-sdk/tests/integration/room/tags.rs index d6e7ba35c7c..994102329ec 100644 --- a/crates/matrix-sdk/tests/integration/room/tags.rs +++ b/crates/matrix-sdk/tests/integration/room/tags.rs @@ -1,4 +1,4 @@ -use std::{collections::BTreeMap, time::Duration}; +use std::{collections::BTreeMap, ops::Not, time::Duration}; use matrix_sdk::{config::SyncSettings, Client, Room}; use matrix_sdk_test::{ @@ -81,42 +81,71 @@ async fn synced_client_with_room( } #[async_test] -async fn when_set_is_favourite_is_run_with_true_then_set_tag_api_is_called() { +async fn test_set_favourite() { let room_id = room_id!("!test:example.org"); let mut ev_builder = SyncResponseBuilder::new(); - let (_client, room, server) = synced_client_with_room(&mut ev_builder, room_id).await; + let (client, room, server) = synced_client_with_room(&mut ev_builder, room_id).await; + // Not favourite. + assert!(room.is_favourite().not()); + + // Server will be called to set the room as favourite. mock_tag_api(&server, TagName::Favorite, TagOperation::Set, 1).await; room.set_is_favourite(true, Option::default()).await.unwrap(); + // Mock the response from the server. + let tags = BTreeMap::from([(TagName::Favorite, TagInfo::default())]); + mock_sync_with_tags(&server, &mut ev_builder, room_id, tags).await; + sync_once(&client, &server).await; + + // Favourite! + assert!(room.is_favourite()); + server.verify().await; } #[async_test] -async fn when_set_is_favourite_is_run_with_true_and_low_priority_tag_was_set_then_set_tag_and_remove_tag_apis_are_called( -) { +async fn test_set_favourite_on_low_priority_room() { let room_id = room_id!("!test:example.org"); let mut ev_builder = SyncResponseBuilder::new(); let (client, room, server) = synced_client_with_room(&mut ev_builder, room_id).await; + + // Mock a response from the server setting the room as low priority. let tags = BTreeMap::from([(TagName::LowPriority, TagInfo::default())]); mock_sync_with_tags(&server, &mut ev_builder, room_id, tags).await; sync_once(&client, &server).await; + // Not favourite, but low priority! + assert!(room.is_favourite().not()); + assert!(room.is_low_priority()); + + // Server will be called to set the room as favourite, and to unset the room as + // low priority. mock_tag_api(&server, TagName::Favorite, TagOperation::Set, 1).await; mock_tag_api(&server, TagName::LowPriority, TagOperation::Remove, 1).await; room.set_is_favourite(true, Option::default()).await.unwrap(); + // Mock the response from the server. + let tags = BTreeMap::from([(TagName::Favorite, TagInfo::default())]); + mock_sync_with_tags(&server, &mut ev_builder, room_id, tags).await; + sync_once(&client, &server).await; + + // Favourite, and not low priority! + assert!(room.is_favourite()); + assert!(room.is_low_priority().not()); + server.verify().await; } #[async_test] -async fn when_set_is_favourite_is_run_with_false_then_delete_tag_api_is_called() { +async fn test_unset_favourite() { let room_id = room_id!("!test:example.org"); let mut ev_builder = SyncResponseBuilder::new(); let (_client, room, server) = synced_client_with_room(&mut ev_builder, room_id).await; + // Server will be called to unset the room as favourite. mock_tag_api(&server, TagName::Favorite, TagOperation::Remove, 1).await; room.set_is_favourite(false, Option::default()).await.unwrap(); @@ -125,79 +154,74 @@ async fn when_set_is_favourite_is_run_with_false_then_delete_tag_api_is_called() } #[async_test] -async fn when_set_is_low_priority_is_run_with_true_then_set_tag_api_is_called() { +async fn test_set_low_priority() { let room_id = room_id!("!test:example.org"); let mut ev_builder = SyncResponseBuilder::new(); - let (_client, room, server) = synced_client_with_room(&mut ev_builder, room_id).await; + let (client, room, server) = synced_client_with_room(&mut ev_builder, room_id).await; + + // Not low prioriry. + assert!(room.is_low_priority().not()); + // Server will be called to set the room as favourite. mock_tag_api(&server, TagName::LowPriority, TagOperation::Set, 1).await; room.set_is_low_priority(true, Option::default()).await.unwrap(); + // Mock the response from the server. + let tags = BTreeMap::from([(TagName::LowPriority, TagInfo::default())]); + mock_sync_with_tags(&server, &mut ev_builder, room_id, tags).await; + sync_once(&client, &server).await; + + // Low priority! + assert!(room.is_low_priority()); + server.verify().await; } #[async_test] -async fn when_set_is_low_priority_is_run_with_true_and_favorite_tag_was_set_then_set_tag_and_remove_tag_apis_are_called( -) { +async fn test_set_low_priority_on_favourite_room() { let room_id = room_id!("!test:example.org"); let mut ev_builder = SyncResponseBuilder::new(); let (client, room, server) = synced_client_with_room(&mut ev_builder, room_id).await; + + // Mock a response from the server setting the room as favourite. let tags = BTreeMap::from([(TagName::Favorite, TagInfo::default())]); mock_sync_with_tags(&server, &mut ev_builder, room_id, tags).await; sync_once(&client, &server).await; + // Favourite, but not low priority! + assert!(room.is_favourite()); + assert!(room.is_low_priority().not()); + + // Server will be called to set the room as favourite, and to unset the room as + // low priority. mock_tag_api(&server, TagName::LowPriority, TagOperation::Set, 1).await; mock_tag_api(&server, TagName::Favorite, TagOperation::Remove, 1).await; room.set_is_low_priority(true, Option::default()).await.unwrap(); + // Mock the response from the server. + let tags = BTreeMap::from([(TagName::LowPriority, TagInfo::default())]); + mock_sync_with_tags(&server, &mut ev_builder, room_id, tags).await; + sync_once(&client, &server).await; + + // Not favourite, and low priority! + assert!(room.is_favourite().not()); + assert!(room.is_low_priority()); + server.verify().await; } #[async_test] -async fn when_set_is_low_priority_is_run_with_false_then_delete_tag_api_is_called() { +async fn test_unset_low_priority() { let room_id = room_id!("!test:example.org"); let mut ev_builder = SyncResponseBuilder::new(); let (_client, room, server) = synced_client_with_room(&mut ev_builder, room_id).await; + // Server will be called to unset the room as favourite. mock_tag_api(&server, TagName::LowPriority, TagOperation::Remove, 1).await; room.set_is_low_priority(false, Option::default()).await.unwrap(); server.verify().await; } - -#[async_test] -async fn when_favorite_tag_is_set_in_sync_response_then_notable_tags_is_favourite_is_true() { - let room_id = room_id!("!test:example.org"); - let mut ev_builder = SyncResponseBuilder::new(); - let (client, room, server) = synced_client_with_room(&mut ev_builder, room_id).await; - - assert!(!room.is_favourite()); - - // Ensure the notable tags stream is updated when the favorite tag is set on - // sync - let tags = BTreeMap::from([(TagName::Favorite, TagInfo::default())]); - mock_sync_with_tags(&server, &mut ev_builder, room_id, tags).await; - sync_once(&client, &server).await; - - assert!(room.is_favourite()); -} - -#[async_test] -async fn when_low_priority_tag_is_set_in_sync_response_then_notable_tags_is_low_priority_is_true() { - let room_id = room_id!("!test:example.org"); - let mut ev_builder = SyncResponseBuilder::new(); - let (client, room, server) = synced_client_with_room(&mut ev_builder, room_id).await; - - assert!(!room.is_low_priority()); - - // Ensure the notable tags stream is updated when the low_priority tag is set on - // sync - let tags = BTreeMap::from([(TagName::LowPriority, TagInfo::default())]); - mock_sync_with_tags(&server, &mut ev_builder, room_id, tags).await; - sync_once(&client, &server).await; - - assert!(room.is_low_priority()); -} From bebb7336078650d11333db45622389b4cb570c25 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Fri, 9 Feb 2024 11:37:50 +0100 Subject: [PATCH 13/13] nit: change favorite to favourite in a few places Signed-off-by: Benjamin Bouvier --- bindings/matrix-sdk-ffi/src/room.rs | 4 ++-- crates/matrix-sdk-base/src/rooms/mod.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/room.rs b/bindings/matrix-sdk-ffi/src/room.rs index 8b76f541925..67d8aef9153 100644 --- a/bindings/matrix-sdk-ffi/src/room.rs +++ b/bindings/matrix-sdk-ffi/src/room.rs @@ -258,10 +258,10 @@ impl Room { pub async fn set_is_favourite( &self, - is_favorite: bool, + is_favourite: bool, tag_order: Option, ) -> Result<(), ClientError> { - self.inner.set_is_favourite(is_favorite, tag_order).await?; + self.inner.set_is_favourite(is_favourite, tag_order).await?; Ok(()) } diff --git a/crates/matrix-sdk-base/src/rooms/mod.rs b/crates/matrix-sdk-base/src/rooms/mod.rs index d2e429e78e0..efcccb71994 100644 --- a/crates/matrix-sdk-base/src/rooms/mod.rs +++ b/crates/matrix-sdk-base/src/rooms/mod.rs @@ -315,7 +315,7 @@ bitflags! { /// Notable tags, i.e. subset of tags that we are more interested by. /// /// We are not interested by all the tags. Some tags are more important than - /// others, and this struct describe them. + /// others, and this struct describes them. #[repr(transparent)] #[derive(Debug, Default, Clone, Copy, Deserialize, Serialize)] pub(crate) struct RoomNotableTags: u8 {